diff --git a/src/__tests__/renderer/components/LightboxModal.test.tsx b/src/__tests__/renderer/components/LightboxModal.test.tsx index e0f4fb4c..1e92211a 100644 --- a/src/__tests__/renderer/components/LightboxModal.test.tsx +++ b/src/__tests__/renderer/components/LightboxModal.test.tsx @@ -1124,4 +1124,135 @@ describe('LightboxModal', () => { ).not.toBeInTheDocument(); }); }); + + describe('integration: parent state synchronization', () => { + /** + * This test simulates the App.tsx pattern where lightboxImages is a separate + * snapshot from stagedImages. The bug we're testing for: when deleting an image, + * both stagedImages AND lightboxImages must be updated, otherwise the lightbox + * continues showing the "deleted" image from the stale lightboxImages snapshot. + */ + it('parent must update lightboxImages when deleting (regression test)', () => { + // Simulate the parent component pattern from App.tsx + let lightboxImages = [...mockImages]; // Snapshot taken when lightbox opened + let stagedImages = [...mockImages]; // Actual staged images state + let currentImage = mockImages[2]; // Viewing last image + + const onClose = vi.fn(); + const onNavigate = vi.fn((img: string) => { + currentImage = img; + }); + + // This simulates the FIXED handleDeleteLightboxImage from App.tsx + // The bug was that it only updated stagedImages, not lightboxImages + const onDelete = vi.fn((img: string) => { + stagedImages = stagedImages.filter((i) => i !== img); + // CRITICAL: Also update lightboxImages (this was the bug fix) + lightboxImages = lightboxImages.filter((i) => i !== img); + }); + + const { rerender } = renderWithLayerStack( + + ); + + // Verify initial state: viewing image 3 of 3 + expect(screen.getByText(/Image 3 of 3/)).toBeInTheDocument(); + + // Delete the image + const deleteButton = screen.getByTitle('Delete image (Delete key)'); + fireEvent.click(deleteButton); + const confirmButton = screen.getByRole('button', { name: /confirm/i }); + fireEvent.click(confirmButton); + + // Verify onDelete was called with the correct image + expect(onDelete).toHaveBeenCalledWith(mockImages[2]); + + // Verify navigation happened (should go to previous image since we deleted last) + expect(onNavigate).toHaveBeenCalledWith(mockImages[1]); + + // CRITICAL: After deletion, lightboxImages should be updated too + // This is what the bug fix ensures + expect(lightboxImages).toHaveLength(2); + expect(lightboxImages).not.toContain(mockImages[2]); + + // Rerender with updated state (simulating React re-render) + rerender( + + + + ); + + // Now should show image 2 of 2 (not 2 of 3) + expect(screen.getByText(/Image 2 of 2/)).toBeInTheDocument(); + }); + + it('demonstrates the bug when lightboxImages is not updated (negative test)', () => { + // This test demonstrates what happens when the bug exists: + // Only stagedImages is updated, lightboxImages remains stale + let lightboxImages = [...mockImages]; // Snapshot - NOT updated on delete (bug!) + let stagedImages = [...mockImages]; + let currentImage = mockImages[2]; + + const onClose = vi.fn(); + const onNavigate = vi.fn((img: string) => { + currentImage = img; + }); + + // BUGGY version: only updates stagedImages + const onDeleteBuggy = vi.fn((img: string) => { + stagedImages = stagedImages.filter((i) => i !== img); + // BUG: lightboxImages is NOT updated! + }); + + const { rerender } = renderWithLayerStack( + + ); + + // Delete the image + const deleteButton = screen.getByTitle('Delete image (Delete key)'); + fireEvent.click(deleteButton); + const confirmButton = screen.getByRole('button', { name: /confirm/i }); + fireEvent.click(confirmButton); + + expect(onNavigate).toHaveBeenCalledWith(mockImages[1]); + + // BUG: lightboxImages still has 3 images because it wasn't updated + expect(lightboxImages).toHaveLength(3); // This is the bug! + + // Rerender with the buggy state + rerender( + + + + ); + + // BUG: Shows "Image 2 of 3" instead of "Image 2 of 2" + // because lightboxImages wasn't updated + expect(screen.getByText(/Image 2 of 3/)).toBeInTheDocument(); + }); + }); }); diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index 5eb7f7b8..a85540f2 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -550,6 +550,9 @@ function MaestroConsoleInner() { // Rendering settings disableConfetti, + + // Tab naming settings + automaticTabNamingEnabled, } = settings; // --- KEYBOARD SHORTCUT HELPERS --- @@ -2600,6 +2603,60 @@ function MaestroConsoleInner() { const actualSessionId = parsed.actualSessionId; const tabId = parsed.tabId ?? undefined; + // First, check if we need to trigger automatic tab naming BEFORE updating state + // We need the current session state to determine this + const currentSessions = sessionsRef.current; + const currentSession = currentSessions.find((s) => s.id === actualSessionId); + + // Capture info for tab naming (must be done before state update modifies awaitingSessionId) + interface TabNamingInfo { + tabId: string; + userMessage: string; + agentType: string; + cwd: string; + sessionSshRemoteConfig?: { + enabled: boolean; + remoteId: string | null; + workingDirOverride?: string; + }; + } + let tabNamingInfo: TabNamingInfo | null = null; + + if (currentSession) { + // Find the target tab (same logic as in setSessions below) + let targetTab = tabId + ? currentSession.aiTabs?.find((tab) => tab.id === tabId) + : undefined; + + if (!targetTab) { + const awaitingTab = currentSession.aiTabs?.find( + (tab) => tab.awaitingSessionId && !tab.agentSessionId + ); + targetTab = awaitingTab || getActiveTab(currentSession); + } + + // Check if we should trigger automatic tab naming: + // 1. Tab was awaiting session ID (this is a new session, not a resume) + // 2. Tab doesn't already have a custom name + // 3. Tab has at least one user message in logs + if (targetTab?.awaitingSessionId && !targetTab.name) { + const userMessages = targetTab.logs.filter((log) => log.source === 'user'); + if (userMessages.length > 0) { + const firstUserMessage = userMessages[0]; + // Only use text messages for naming (skip image-only messages) + if (firstUserMessage.text?.trim()) { + tabNamingInfo = { + tabId: targetTab.id, + userMessage: firstUserMessage.text, + agentType: currentSession.toolType, + cwd: currentSession.cwd, + sessionSshRemoteConfig: currentSession.sessionSshRemoteConfig, + }; + } + } + } + } + // Store Claude session ID in session state // Note: slash commands are now received via onSlashCommands from Claude Code's init message setSessions((prev) => { @@ -2663,6 +2720,61 @@ function MaestroConsoleInner() { return { ...s, aiTabs: updatedAiTabs, agentSessionId }; // Also keep session-level for backwards compatibility }); }); + + // Trigger automatic tab naming if conditions are met + // This runs after the state update so it happens in parallel with the main AI processing + // Use ref to access the latest setting value (useEffect has [] deps so we'd get stale value otherwise) + if (tabNamingInfo && automaticTabNamingEnabledRef.current) { + console.log('[onSessionId] Triggering automatic tab naming', { + tabId: tabNamingInfo.tabId, + messageLength: tabNamingInfo.userMessage.length, + agentType: tabNamingInfo.agentType, + }); + + // Call the tab naming API (async, don't await) + window.maestro.tabNaming + .generateTabName({ + userMessage: tabNamingInfo.userMessage, + agentType: tabNamingInfo.agentType, + cwd: tabNamingInfo.cwd, + sessionSshRemoteConfig: tabNamingInfo.sessionSshRemoteConfig, + }) + .then((generatedName) => { + if (!generatedName) { + console.log('[onSessionId] Tab naming returned null (timeout or error)'); + return; + } + + console.log('[onSessionId] Tab naming generated:', generatedName); + + // Update the tab name only if it's still null (user hasn't renamed it) + setSessions((prev) => + prev.map((s) => { + if (s.id !== actualSessionId) return s; + + const tab = s.aiTabs.find((t) => t.id === tabNamingInfo!.tabId); + // Only update if tab exists and name is still null (UUID display) + if (!tab || tab.name !== null) { + console.log('[onSessionId] Skipping tab name update (already named)', { + tabExists: !!tab, + currentName: tab?.name, + }); + return s; + } + + return { + ...s, + aiTabs: s.aiTabs.map((t) => + t.id === tabNamingInfo!.tabId ? { ...t, name: generatedName } : t + ), + }; + }) + ); + }) + .catch((error) => { + console.error('[onSessionId] Tab naming failed:', error); + }); + } } ); @@ -3482,11 +3594,13 @@ function MaestroConsoleInner() { const customAICommandsRef = useRef(customAICommands); const speckitCommandsRef = useRef(speckitCommands); const openspecCommandsRef = useRef(openspecCommands); + const automaticTabNamingEnabledRef = useRef(automaticTabNamingEnabled); addToastRef.current = addToast; updateGlobalStatsRef.current = updateGlobalStats; customAICommandsRef.current = customAICommands; speckitCommandsRef.current = speckitCommands; openspecCommandsRef.current = openspecCommands; + automaticTabNamingEnabledRef.current = automaticTabNamingEnabled; // Note: spawnBackgroundSynopsisRef and spawnAgentWithPromptRef are now provided by useAgentExecution hook // Note: addHistoryEntryRef is now provided by useAgentSessionManagement hook @@ -11248,8 +11362,11 @@ You are taking over this conversation. Based on the context above, provide a bri } else { setStagedImages((prev) => prev.filter((i) => i !== img)); } + // Also update lightboxImages so the lightbox navigation stays in sync + // (lightboxImages is a snapshot taken when the lightbox opened, so it gets stale on deletion) + setLightboxImages(lightboxImages.filter((i) => i !== img)); }, - [setStagedImages] + [setStagedImages, setLightboxImages, lightboxImages] ); const handleCloseAutoRunSetup = useCallback(() => setAutoRunSetupModalOpen(false), []); const handleCloseBatchRunner = useCallback(() => setBatchRunnerModalOpen(false), []);