diff --git a/src/renderer/hooks/use-editor-persistence.test.ts b/src/renderer/hooks/use-editor-persistence.test.ts index e66c3cd..b88059b 100644 --- a/src/renderer/hooks/use-editor-persistence.test.ts +++ b/src/renderer/hooks/use-editor-persistence.test.ts @@ -4,6 +4,10 @@ import { useEditorPersistence, persistState } from './use-editor-persistence' import { useWorkspaceStore } from '@/stores/workspace-store' import type { PaneNode, SplitNode, LeafNode } from '@/types/workspace.types' +const { mockLoadPersistedTerminals } = vi.hoisted(() => ({ + mockLoadPersistedTerminals: vi.fn() +})) + const { mockPersistenceRead, mockPersistenceWriteDebounced } = vi.hoisted(() => ({ mockPersistenceRead: vi.fn(), mockPersistenceWriteDebounced: vi.fn() @@ -69,6 +73,7 @@ const mockWorkspaceState: { syncTerminalTabs: ReturnType clearEditorTabs: ReturnType resetLayout: ReturnType + loadProjectWorkspace: ReturnType } = { root: { type: 'leaf', @@ -82,7 +87,8 @@ const mockWorkspaceState: { remapTerminalTabs: vi.fn(), syncTerminalTabs: vi.fn(), clearEditorTabs: vi.fn(), - resetLayout: vi.fn() + resetLayout: vi.fn(), + loadProjectWorkspace: vi.fn() } const mockProjectState = { @@ -126,9 +132,32 @@ vi.mock('@/stores/project-store', () => ({ } })) +const mockTerminalState = { + terminals: [] as Array<{ + id: string + name: string + projectId: string + shell: string + cwd?: string + ptyId?: string + }> +} + +vi.mock('@/stores/terminal-store', () => ({ + useTerminalStore: { + getState: vi.fn(() => mockTerminalState) + } +})) + +vi.mock('./useTerminalAutoSave', () => ({ + loadPersistedTerminals: mockLoadPersistedTerminals +})) + beforeEach(() => { mockPersistenceRead.mockReset() mockPersistenceWriteDebounced.mockReset() + mockLoadPersistedTerminals.mockReset() + mockLoadPersistedTerminals.mockResolvedValue(null) mockEditorState.openFiles = new Map>() mockEditorState.activeFilePath = null @@ -167,7 +196,9 @@ beforeEach(() => { mockWorkspaceState.syncTerminalTabs.mockReset() mockWorkspaceState.clearEditorTabs.mockReset() mockWorkspaceState.resetLayout.mockReset() + mockWorkspaceState.loadProjectWorkspace.mockReset() + mockTerminalState.terminals = [] }) afterEach(() => { @@ -228,13 +259,13 @@ describe('useEditorPersistence', () => { }) await waitFor(() => { - expect(mockExplorerState.restoreExpandedDirs).toHaveBeenCalledWith(['/projects/a/src']) + expect(mockExplorerState.restoreExpandedDirs).toHaveBeenLastCalledWith(['/projects/a/src']) }) rerender({ projectId: 'project-b' }) await waitFor(() => { - expect(mockExplorerState.restoreExpandedDirs).toHaveBeenCalledWith(['/projects/b/docs']) + expect(mockExplorerState.restoreExpandedDirs).toHaveBeenLastCalledWith(['/projects/b/docs']) }) }) @@ -302,7 +333,7 @@ describe('useEditorPersistence', () => { ]) }) - it('restores pane layout, keeps terminal tabs, and prunes missing editor tabs', async () => { + it('restores pane layout, remaps terminal tabs to live terminals, and prunes missing editor tabs', async () => { mockPersistenceRead.mockResolvedValue({ success: true, data: { @@ -347,24 +378,41 @@ describe('useEditorPersistence', () => { } }) - renderHook(() => useEditorPersistence('project-a')) + mockTerminalState.terminals = [ + { + id: 'live-1', + name: 'Claude', + projectId: 'project-a', + shell: 'bash', + cwd: '/projects/a', + ptyId: 'pty-live-1' + } + ] + mockLoadPersistedTerminals.mockResolvedValue({ + activeTerminalId: 'old-1', + terminals: [ + { + id: 'old-1', + name: 'Claude', + shell: 'bash', + cwd: '/projects/a', + scrollback: [] + } + ], + updatedAt: '2026-03-09T00:00:00.000Z' + }) - const workspaceStoreSetState = vi.mocked(useWorkspaceStore.setState) + renderHook(() => useEditorPersistence('project-a')) await waitFor(() => { - expect(workspaceStoreSetState).toHaveBeenCalled() + expect(mockWorkspaceState.loadProjectWorkspace).toHaveBeenCalled() }) - const workspaceStateUpdate = workspaceStoreSetState.mock.calls - .map((call) => call[0]) - .find((arg) => arg && typeof arg === 'object' && 'root' in arg) + const [restoredRootArg, activePaneIdArg] = mockWorkspaceState.loadProjectWorkspace.mock.calls[0] - expect(workspaceStateUpdate).toBeTruthy() - if (!workspaceStateUpdate) throw new Error('workspaceStateUpdate is undefined') + expect(activePaneIdArg).toBe('pane-drop') - expect(workspaceStateUpdate.activePaneId).toBe('pane-drop') - - const restoredRoot = workspaceStateUpdate.root as SplitNode + const restoredRoot = restoredRootArg as SplitNode expect(restoredRoot.type).toBe('split') const leftLeaf = restoredRoot.children[0] as LeafNode @@ -373,19 +421,159 @@ describe('useEditorPersistence', () => { expect(leftLeaf.type).toBe('leaf') expect(leftLeaf.id).toBe('pane-keep') expect(leftLeaf.tabs).toEqual([ - { type: 'terminal', id: 'term-old-1', terminalId: 'old-1' }, + { type: 'terminal', id: 'term-live-1', terminalId: 'live-1' }, { type: 'editor', id: 'edit-/projects/a/src/existing.ts', filePath: '/projects/a/src/existing.ts' } ]) + expect(leftLeaf.activeTabId).toBe('term-live-1') expect(rightLeaf.type).toBe('leaf') expect(rightLeaf.id).toBe('pane-drop') expect(rightLeaf.tabs).toEqual([]) }) + it('keeps persisted terminal tabs during recovery-only restore when no live terminal exists', async () => { + mockPersistenceRead.mockResolvedValue({ + success: true, + data: { + openFiles: [], + activeFilePath: null, + expandedDirs: ['/projects/a'], + fileExplorerVisible: true, + activeTabId: null, + activePaneId: 'pane-recovery', + paneLayout: { + type: 'leaf', + id: 'pane-recovery', + tabs: [{ type: 'terminal', terminalId: 'persisted-only' }], + activeTabId: 'term-persisted-only' + } + } + }) + mockLoadPersistedTerminals.mockResolvedValue({ + activeTerminalId: 'persisted-only', + terminals: [ + { + id: 'persisted-only', + name: 'Recovered terminal', + shell: 'bash', + cwd: '/projects/a', + scrollback: [] + } + ], + updatedAt: '2026-03-09T00:00:00.000Z' + }) + + renderHook(() => useEditorPersistence('project-a')) + + await waitFor(() => { + expect(mockWorkspaceState.loadProjectWorkspace).toHaveBeenCalled() + }) + + const [restoredRootArg, activePaneIdArg] = mockWorkspaceState.loadProjectWorkspace.mock.calls[0] + expect(activePaneIdArg).toBe('pane-recovery') + expect(restoredRootArg).toEqual({ + type: 'leaf', + id: 'pane-recovery', + tabs: [ + { + type: 'terminal', + id: 'term-persisted-only', + terminalId: 'persisted-only' + } + ], + activeTabId: 'term-persisted-only' + }) + }) + + it('ignores stale restore results after switching projects', async () => { + const projectARead = { + resolve: + undefined as + | ((value: { + success: true + data: { + openFiles: never[] + activeFilePath: null + expandedDirs: string[] + fileExplorerVisible: boolean + activeTabId: null + activePaneId: string + paneLayout: { + type: 'leaf' + id: string + tabs: { type: 'editor'; filePath: string }[] + activeTabId: string + } + } + }) => void) + | undefined + } + + mockPersistenceRead + .mockImplementationOnce( + () => + new Promise((resolve) => { + projectARead.resolve = resolve + }) + ) + .mockResolvedValueOnce({ success: true, data: null }) + .mockResolvedValueOnce({ + success: true, + data: { + openFiles: [], + activeFilePath: null, + expandedDirs: ['/projects/b'], + fileExplorerVisible: true, + activeTabId: null, + activePaneId: 'pane-b', + paneLayout: { + type: 'leaf', + id: 'pane-b', + tabs: [{ type: 'editor', filePath: '/projects/b/src/index.ts' }], + activeTabId: 'edit-/projects/b/src/index.ts' + } + } + }) + + const { rerender } = renderHook(({ currentProjectId }) => useEditorPersistence(currentProjectId), { + initialProps: { currentProjectId: 'project-a' } + }) + + rerender({ currentProjectId: 'project-b' }) + + await waitFor(() => { + expect(mockWorkspaceState.resetLayout).toHaveBeenCalledTimes(1) + }) + + projectARead.resolve?.({ + success: true, + data: { + openFiles: [], + activeFilePath: null, + expandedDirs: ['/projects/a'], + fileExplorerVisible: true, + activeTabId: null, + activePaneId: 'pane-a', + paneLayout: { + type: 'leaf', + id: 'pane-a', + tabs: [{ type: 'editor', filePath: '/projects/a/src/index.ts' }], + activeTabId: 'edit-/projects/a/src/index.ts' + } + } + }) + + await waitFor(() => { + expect(mockPersistenceRead).toHaveBeenCalledTimes(2) + }) + + expect(mockWorkspaceState.loadProjectWorkspace).not.toHaveBeenCalled() + }) + it('restores legacy pane layout entries that use editorFilePaths', async () => { mockPersistenceRead.mockResolvedValue({ success: true, @@ -416,21 +604,14 @@ describe('useEditorPersistence', () => { renderHook(() => useEditorPersistence('project-a')) - const workspaceStoreSetState = vi.mocked(useWorkspaceStore.setState) - await waitFor(() => { - expect(workspaceStoreSetState).toHaveBeenCalled() + expect(mockWorkspaceState.loadProjectWorkspace).toHaveBeenCalled() }) - const workspaceStateUpdate = workspaceStoreSetState.mock.calls - .map((call) => call[0]) - .find((arg) => arg && typeof arg === 'object' && 'root' in arg) - - expect(workspaceStateUpdate).toBeTruthy() - if (!workspaceStateUpdate) throw new Error('workspaceStateUpdate is undefined') + const [restoredRootArg, activePaneIdArg] = mockWorkspaceState.loadProjectWorkspace.mock.calls[0] - expect(workspaceStateUpdate.activePaneId).toBe('pane-legacy') - expect(workspaceStateUpdate.root).toEqual({ + expect(activePaneIdArg).toBe('pane-legacy') + expect(restoredRootArg).toEqual({ type: 'leaf', id: 'pane-legacy', tabs: [ diff --git a/src/renderer/hooks/use-editor-persistence.ts b/src/renderer/hooks/use-editor-persistence.ts index 556c079..bf8ffab 100644 --- a/src/renderer/hooks/use-editor-persistence.ts +++ b/src/renderer/hooks/use-editor-persistence.ts @@ -3,6 +3,7 @@ import { useEditorStore } from '@/stores/editor-store' import { persistenceApi } from '@/lib/api' import { useFileExplorerStore } from '@/stores/file-explorer-store' import { useProjectStore } from '@/stores/project-store' +import { useTerminalStore } from '@/stores/terminal-store' import { useWorkspaceStore, findPaneById, @@ -10,9 +11,12 @@ import { editorTabId, terminalTabId } from '@/stores/workspace-store' +import { loadPersistedTerminals } from './useTerminalAutoSave' import type { EditorFileState } from '@/stores/editor-store' import type { PaneNode, SplitNode, PaneDirection } from '@/types/workspace.types' import type { WorkspaceTab } from '@/stores/workspace-store' +import type { Terminal } from '@/types/project' +import type { PersistedTerminalLayout } from '../../shared/types/persistence.types' interface PersistedEditorFile { filePath: string @@ -174,6 +178,138 @@ function normalizePaneTree(root: PaneNode): PaneNode { } } +function createTerminalMatcher( + liveTerminals: Terminal[], + layout: PersistedTerminalLayout | null +): { + hasLiveTerminals: boolean + matchTerminalId: (persistedTerminalId: string) => string | null +} { + const liveTerminalsById = new Map(liveTerminals.map((terminal) => [terminal.id, terminal])) + const layoutTerminalsById = new Map(layout?.terminals.map((terminal) => [terminal.id, terminal]) ?? []) + const unusedLiveTerminals = [...liveTerminals] + + const consumeLiveTerminal = (terminalId: string): string | null => { + const match = liveTerminalsById.get(terminalId) + if (!match) { + return null + } + + const index = unusedLiveTerminals.findIndex((terminal) => terminal.id === terminalId) + if (index >= 0) { + unusedLiveTerminals.splice(index, 1) + } + return match.id + } + + return { + hasLiveTerminals: liveTerminals.length > 0, + matchTerminalId: (persistedTerminalId: string): string | null => { + const directMatch = consumeLiveTerminal(persistedTerminalId) + if (directMatch) { + return directMatch + } + + const persistedTerminal = layoutTerminalsById.get(persistedTerminalId) + if (!persistedTerminal) { + return null + } + + const exactIndex = unusedLiveTerminals.findIndex((terminal) => { + return ( + terminal.name === persistedTerminal.name && + terminal.shell === persistedTerminal.shell && + terminal.cwd === persistedTerminal.cwd + ) + }) + if (exactIndex >= 0) { + const [match] = unusedLiveTerminals.splice(exactIndex, 1) + return match.id + } + + const nameAndShellIndex = unusedLiveTerminals.findIndex((terminal) => { + return terminal.name === persistedTerminal.name && terminal.shell === persistedTerminal.shell + }) + if (nameAndShellIndex >= 0) { + const [match] = unusedLiveTerminals.splice(nameAndShellIndex, 1) + return match.id + } + + const nameOnlyIndex = unusedLiveTerminals.findIndex( + (terminal) => terminal.name === persistedTerminal.name + ) + if (nameOnlyIndex >= 0) { + const [match] = unusedLiveTerminals.splice(nameOnlyIndex, 1) + return match.id + } + + return null + } + } +} + +function reconcileTerminalTabs( + root: PaneNode, + openFilePaths: Set, + liveTerminals: Terminal[], + layout: PersistedTerminalLayout | null +): PaneNode { + const { hasLiveTerminals, matchTerminalId } = createTerminalMatcher(liveTerminals, layout) + const shouldKeepPersistedTerminalTabs = !hasLiveTerminals && !!layout?.terminals.length + + const visit = (node: PaneNode): PaneNode => { + if (node.type === 'leaf') { + const terminalTabIdMap = new Map() + const validTabs = node.tabs.flatMap((tab): WorkspaceTab[] => { + if (tab.type === 'editor') { + return openFilePaths.has(tab.filePath) ? [tab] : [] + } + + if (shouldKeepPersistedTerminalTabs) { + return [tab] + } + + const mappedTerminalId = matchTerminalId(tab.terminalId) + if (!mappedTerminalId) { + return [] + } + + const mappedTabId = terminalTabId(mappedTerminalId) + terminalTabIdMap.set(tab.id, mappedTabId) + + return [ + { + type: 'terminal', + id: mappedTabId, + terminalId: mappedTerminalId + } + ] + }) + + let activeTabId = node.activeTabId + if (activeTabId && terminalTabIdMap.has(activeTabId)) { + activeTabId = terminalTabIdMap.get(activeTabId) ?? activeTabId + } + if (activeTabId && !validTabs.some((tab) => tab.id === activeTabId)) { + activeTabId = validTabs.length > 0 ? validTabs[0].id : null + } + + return { + ...node, + tabs: validTabs, + activeTabId + } + } + + return { + ...node, + children: node.children.map(visit) + } as SplitNode + } + + return normalizePaneTree(visit(root)) +} + // Deserialize pane tree with full tab mapping function deserializePaneTree(persisted: PersistedPaneNodeInput): PaneNode { if (persisted.type === 'leaf') { @@ -230,6 +366,7 @@ function deserializePaneTree(persisted: PersistedPaneNodeInput): PaneNode { export function useEditorPersistence(projectId: string): void { const isRestoringRef = useRef(false) const prevProjectIdRef = useRef('') + const restoreRunIdRef = useRef(0) // Restore state when project changes useEffect(() => { @@ -237,6 +374,12 @@ export function useEditorPersistence(projectId: string): void { const oldProjectId = prevProjectIdRef.current prevProjectIdRef.current = projectId + const restoreRunId = ++restoreRunIdRef.current + let cancelled = false + const isStale = (): boolean => { + return cancelled || restoreRunIdRef.current !== restoreRunId || prevProjectIdRef.current !== projectId + } + async function restore(): Promise { isRestoringRef.current = true try { @@ -254,6 +397,10 @@ export function useEditorPersistence(projectId: string): void { editorStateKey(projectId) ) + if (isStale()) { + return + } + if (!result.success || !result.data) { // No persisted state — reset to single empty pane useWorkspaceStore.getState().resetLayout() @@ -276,8 +423,16 @@ export function useEditorPersistence(projectId: string): void { // Restore open files const editorStore = useEditorStore.getState() for (const file of persisted.openFiles) { + if (isStale()) { + return + } + try { await editorStore.openFile(file.filePath) + if (isStale()) { + return + } + editorStore.updateCursorPosition(file.filePath, file.cursorPosition.line, file.cursorPosition.col) editorStore.updateScrollTop(file.filePath, file.scrollTop) if (file.viewMode !== 'code') { @@ -297,6 +452,10 @@ export function useEditorPersistence(projectId: string): void { } } + if (isStale()) { + return + } + // Restore active file if (persisted.activeFilePath) { editorStore.setActiveFilePath(persisted.activeFilePath) @@ -306,43 +465,21 @@ export function useEditorPersistence(projectId: string): void { if (persisted.paneLayout) { const restoredTree = deserializePaneTree(persisted.paneLayout) const openFilePaths = new Set(useEditorStore.getState().openFiles.keys()) - const filterUnavailableTabs = (node: PaneNode): PaneNode => { - if (node.type === 'leaf') { - const validTabs = node.tabs.filter((tab) => { - if (tab.type === 'editor') { - return openFilePaths.has(tab.filePath) - } - - return true - }) - - let activeTabId = node.activeTabId - if (activeTabId && !validTabs.some((tab) => tab.id === activeTabId)) { - activeTabId = validTabs.length > 0 ? validTabs[0].id : null - } - - return { - ...node, - tabs: validTabs, - activeTabId - } - } - - return { - ...node, - children: node.children.map(filterUnavailableTabs) - } as SplitNode + const liveProjectTerminals = useTerminalStore + .getState() + .terminals.filter((terminal) => terminal.projectId === projectId && !!terminal.ptyId) + const persistedTerminalLayout = await loadPersistedTerminals(projectId) + if (isStale()) { + return } - const cleanTree = normalizePaneTree(filterUnavailableTabs(restoredTree)) - const leaves = getAllLeafPanes(cleanTree) - const persistedActivePaneId = persisted.activePaneId - const resolvedActivePaneId = - persistedActivePaneId && leaves.some((leaf) => leaf.id === persistedActivePaneId) - ? persistedActivePaneId - : leaves[0]?.id ?? cleanTree.id - - useWorkspaceStore.setState({ root: cleanTree, activePaneId: resolvedActivePaneId }) + const cleanTree = reconcileTerminalTabs( + restoredTree, + openFilePaths, + liveProjectTerminals, + persistedTerminalLayout + ) + useWorkspaceStore.getState().loadProjectWorkspace(cleanTree, persisted.activePaneId) } else { // Legacy fallback: build a fresh layout with editor tabs useWorkspaceStore.getState().resetLayout() @@ -352,12 +489,21 @@ export function useEditorPersistence(projectId: string): void { // Restore expanded directory tree after root initialization. await explorerStore.restoreExpandedDirs(filteredExpandedDirs) + if (isStale()) { + return + } } finally { - isRestoringRef.current = false + if (restoreRunIdRef.current === restoreRunId) { + isRestoringRef.current = false + } } } - restore() + void restore() + + return () => { + cancelled = true + } }, [projectId]) // Save state on changes (debounced) - coalesced across all store subscriptions diff --git a/src/renderer/hooks/use-terminal-restore.test.ts b/src/renderer/hooks/use-terminal-restore.test.ts index 04111a8..1080064 100644 --- a/src/renderer/hooks/use-terminal-restore.test.ts +++ b/src/renderer/hooks/use-terminal-restore.test.ts @@ -1,5 +1,125 @@ -import { describe, it, expect } from 'vitest' -import { normalizeShellForStartup } from './use-terminal-restore' +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { renderHook, waitFor } from '@testing-library/react' +import type { PaneNode } from '@/types/workspace.types' +import { normalizeShellForStartup, useTerminalRestore } from './use-terminal-restore' + +const { + mockLoadPersistedTerminals, + mockSaveTerminalLayout, + mockSetTerminalRestoreInProgress, + mockTerminalSpawn, + mockTerminalKill +} = vi.hoisted(() => ({ + mockLoadPersistedTerminals: vi.fn(), + mockSaveTerminalLayout: vi.fn(), + mockSetTerminalRestoreInProgress: vi.fn(), + mockTerminalSpawn: vi.fn(), + mockTerminalKill: vi.fn() +})) + +vi.mock('./useTerminalAutoSave', () => ({ + loadPersistedTerminals: mockLoadPersistedTerminals, + saveTerminalLayout: mockSaveTerminalLayout, + setTerminalRestoreInProgress: mockSetTerminalRestoreInProgress +})) + +vi.mock('@/lib/api', () => ({ + terminalApi: { + spawn: mockTerminalSpawn, + kill: mockTerminalKill + } +})) + +vi.mock('@/lib/shell-api', () => ({ + shellApi: { + getAvailableShells: vi.fn().mockResolvedValue({ success: true, data: { available: [] } }) + } +})) + +const mockProjectState = { + activeProjectId: '', + projects: [ + { id: 'project-a', path: '/projects/a' }, + { id: 'project-b', path: '/projects/b' } + ] +} + +vi.mock('../stores/project-store', () => ({ + useProjectStore: Object.assign( + (selector?: (state: typeof mockProjectState) => unknown) => + selector ? selector(mockProjectState) : mockProjectState, + { + getState: vi.fn(() => mockProjectState) + } + ) +})) + +const mockTerminalStoreState = { + terminals: [] as Array<{ id: string; projectId: string; name: string; shell: string; ptyId?: string }>, + activeTerminalId: '', + selectTerminal: vi.fn(), + setTerminals: vi.fn(), + addTerminal: vi.fn(), + setTerminalPtyId: vi.fn() +} + +vi.mock('../stores/terminal-store', () => ({ + useTerminalStore: { + getState: vi.fn(() => mockTerminalStoreState) + } +})) + +const mockWorkspaceStore = { + ensureTerminalTab: vi.fn(), + getActivePaneLeaf: vi.fn(() => ({ id: 'pane-active', type: 'leaf', tabs: [], activeTabId: null })), + setActiveTab: vi.fn(), + root: { + type: 'leaf', + id: 'pane-active', + tabs: [], + activeTabId: null + } as PaneNode +} + +vi.mock('../stores/workspace-store', async () => { + const actual = await vi.importActual('../stores/workspace-store') + return { + ...actual, + useWorkspaceStore: { + getState: vi.fn(() => mockWorkspaceStore) + } + } +}) + +vi.mock('../stores/app-settings-store', () => ({ + useAppSettingsStore: { + getState: vi.fn(() => ({ settings: { defaultShell: 'bash' } })) + } +})) + +beforeEach(() => { + vi.clearAllMocks() + mockProjectState.activeProjectId = '' + mockTerminalStoreState.terminals = [] + mockTerminalStoreState.activeTerminalId = '' + mockWorkspaceStore.root = { + type: 'leaf', + id: 'pane-active', + tabs: [], + activeTabId: null + } as PaneNode + mockWorkspaceStore.getActivePaneLeaf.mockReturnValue({ + id: 'pane-active', + type: 'leaf', + tabs: [], + activeTabId: null + }) + mockLoadPersistedTerminals.mockResolvedValue(null) + mockSaveTerminalLayout.mockResolvedValue(undefined) + mockTerminalSpawn.mockResolvedValue({ success: true, data: { id: 'pty-1' } }) + mockTerminalKill.mockResolvedValue({ success: true, data: undefined }) + mockTerminalStoreState.addTerminal.mockImplementation(() => ({ id: 'new-terminal' })) +}) describe('normalizeShellForStartup', () => { it('returns powershell when shell is empty', () => { @@ -37,3 +157,293 @@ describe('normalizeShellForStartup', () => { expect(normalizeShellForStartup('cmd')).toBe('cmd') }) }) + +describe('useTerminalRestore', () => { + it('uses the pane containing the restored terminal tab when selecting a live terminal', async () => { + mockTerminalStoreState.terminals = [ + { id: 'a-live', projectId: 'project-a', name: 'A', shell: 'bash', ptyId: 'pty-a' } + ] + mockLoadPersistedTerminals.mockResolvedValue({ + activeTerminalId: 'a-live', + terminals: [ + { + id: 'a-live', + name: 'A', + shell: 'bash', + cwd: '/projects/a', + scrollback: [] + } + ], + updatedAt: '2026-03-09T00:00:00.000Z' + }) + mockWorkspaceStore.root = { + type: 'split', + id: 'split-root', + direction: 'horizontal', + sizes: [50, 50], + children: [ + { type: 'leaf', id: 'pane-other', tabs: [], activeTabId: null }, + { + type: 'leaf', + id: 'pane-restored', + tabs: [{ type: 'terminal', id: 'term-a-live', terminalId: 'a-live' }], + activeTabId: null + } + ] + } as PaneNode + mockWorkspaceStore.getActivePaneLeaf.mockReturnValue({ + id: 'pane-active', + type: 'leaf', + tabs: [], + activeTabId: null + }) + + renderHook(() => { + mockProjectState.activeProjectId = 'project-a' + useTerminalRestore() + }) + + await waitFor(() => { + expect(mockWorkspaceStore.setActiveTab).toHaveBeenCalledWith('pane-restored', 'term-a-live') + expect(mockTerminalStoreState.selectTerminal).toHaveBeenCalledWith('a-live') + }) + }) + + it('does not apply cancelled live-terminal restore state after a project switch', async () => { + const projectALayout = { + resolve: undefined as ((value: null) => void) | undefined + } + + mockTerminalStoreState.terminals = [ + { id: 'a-live', projectId: 'project-a', name: 'A', shell: 'bash', ptyId: 'pty-a' }, + { id: 'b-live', projectId: 'project-b', name: 'B', shell: 'bash', ptyId: 'pty-b' } + ] + + mockLoadPersistedTerminals + .mockImplementationOnce( + () => + new Promise((resolve) => { + projectALayout.resolve = resolve + }) + ) + .mockResolvedValueOnce(null) + + const { rerender } = renderHook(({ projectId }) => { + mockProjectState.activeProjectId = projectId + useTerminalRestore() + }, { + initialProps: { projectId: 'project-a' } + }) + + rerender({ projectId: 'project-b' }) + + await waitFor(() => { + expect(mockWorkspaceStore.setActiveTab).toHaveBeenCalledWith('pane-active', 'term-b-live') + }) + + projectALayout.resolve?.(null) + + await waitFor(() => { + expect(mockLoadPersistedTerminals).toHaveBeenCalledTimes(2) + }) + + expect(mockWorkspaceStore.setActiveTab).toHaveBeenCalledTimes(1) + expect(mockWorkspaceStore.ensureTerminalTab).toHaveBeenCalledWith('b-live', undefined, true) + expect(mockWorkspaceStore.ensureTerminalTab).not.toHaveBeenCalledWith('a-live', undefined, true) + expect(mockTerminalStoreState.selectTerminal).toHaveBeenCalledWith('b-live') + }) + + it('does not spawn a fallback terminal after cancelled restore errors', async () => { + const projectALayout = { + reject: undefined as ((reason?: unknown) => void) | undefined + } + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + mockTerminalStoreState.terminals = [ + { id: 'b-live', projectId: 'project-b', name: 'B', shell: 'bash', ptyId: 'pty-b' } + ] + + mockLoadPersistedTerminals + .mockImplementationOnce( + () => + new Promise((_, reject) => { + projectALayout.reject = reject + }) + ) + .mockResolvedValueOnce(null) + + try { + const { rerender } = renderHook(({ projectId }) => { + mockProjectState.activeProjectId = projectId + useTerminalRestore() + }, { + initialProps: { projectId: 'project-a' } + }) + + rerender({ projectId: 'project-b' }) + + await waitFor(() => { + expect(mockWorkspaceStore.setActiveTab).toHaveBeenCalledWith('pane-active', 'term-b-live') + expect(mockTerminalStoreState.selectTerminal).toHaveBeenCalledWith('b-live') + }) + + projectALayout.reject?.(new Error('restore failed')) + + await waitFor(() => { + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to restore terminals:', + expect.any(Error) + ) + }) + + await waitFor(() => { + expect(mockTerminalSpawn).not.toHaveBeenCalled() + expect(mockTerminalStoreState.addTerminal).not.toHaveBeenCalled() + }) + } finally { + consoleErrorSpy.mockRestore() + } + }) + + it('kills a spawned pty when restore is cancelled after spawn succeeds', async () => { + const spawnGate = { + resolve: undefined as ((value: { success: true; data: { id: string } }) => void) | undefined + } + + mockTerminalStoreState.terminals = [ + { id: 'b-live', projectId: 'project-b', name: 'B', shell: 'bash', ptyId: 'pty-b' } + ] + + mockLoadPersistedTerminals + .mockImplementationOnce( + () => + Promise.resolve({ + activeTerminalId: 'old-a', + terminals: [ + { + id: 'old-a', + name: 'A', + shell: 'bash', + cwd: '/projects/a', + scrollback: [] + } + ], + updatedAt: '2026-03-09T00:00:00.000Z' + }) + ) + .mockResolvedValueOnce(null) + + mockTerminalSpawn.mockImplementationOnce( + () => + new Promise((resolve) => { + spawnGate.resolve = resolve as (value: { success: true; data: { id: string } }) => void + }) + ) + + const { rerender } = renderHook(({ projectId }) => { + mockProjectState.activeProjectId = projectId + useTerminalRestore() + }, { + initialProps: { projectId: 'project-a' } + }) + + await waitFor(() => { + expect(mockTerminalSpawn).toHaveBeenCalledTimes(1) + }) + + rerender({ projectId: 'project-b' }) + mockTerminalStoreState.terminals = [ + { id: 'b-live', projectId: 'project-b', name: 'B', shell: 'bash', ptyId: 'pty-b' } + ] + + await waitFor(() => { + expect(mockWorkspaceStore.setActiveTab).toHaveBeenCalledWith('pane-active', 'term-b-live') + expect(mockTerminalStoreState.selectTerminal).toHaveBeenCalledWith('b-live') + }) + + spawnGate.resolve?.({ success: true, data: { id: 'pty-orphan' } }) + + await waitFor(() => { + expect(mockTerminalKill).toHaveBeenCalledWith('pty-orphan') + }) + + expect(mockTerminalStoreState.setTerminals).not.toHaveBeenCalled() + expect(mockTerminalStoreState.selectTerminal).not.toHaveBeenCalledWith('new-terminal') + }) + + it('passes a stable owner token when marking restore progress', async () => { + mockTerminalStoreState.terminals = [ + { id: 'a-live', projectId: 'project-a', name: 'A', shell: 'bash', ptyId: 'pty-a' } + ] + mockLoadPersistedTerminals.mockResolvedValue(null) + + const { unmount } = renderHook(() => { + mockProjectState.activeProjectId = 'project-a' + useTerminalRestore() + }) + + await waitFor(() => { + expect(mockSetTerminalRestoreInProgress).toHaveBeenCalledWith( + 'project-a', + true, + expect.any(String) + ) + }) + + const ownerToken = mockSetTerminalRestoreInProgress.mock.calls[0][2] + unmount() + + await waitFor(() => { + expect(mockSetTerminalRestoreInProgress).toHaveBeenCalledWith('project-a', false, ownerToken) + }) + }) + + it('kills a spawned default terminal pty when restore is cancelled after spawn succeeds', async () => { + const spawnGate = { + resolve: undefined as ((value: { success: true; data: { id: string } }) => void) | undefined + } + + mockTerminalStoreState.terminals = [] + mockTerminalStoreState.addTerminal.mockImplementation(() => ({ id: 'new-terminal' })) + mockLoadPersistedTerminals + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + mockTerminalSpawn.mockImplementationOnce( + () => + new Promise((resolve) => { + spawnGate.resolve = resolve as (value: { success: true; data: { id: string } }) => void + }) + ) + + const { rerender } = renderHook(({ projectId }) => { + mockProjectState.activeProjectId = projectId + useTerminalRestore() + }, { + initialProps: { projectId: 'project-a' } + }) + + await waitFor(() => { + expect(mockTerminalSpawn).toHaveBeenCalledTimes(1) + }) + + mockTerminalStoreState.terminals = [ + { id: 'b-live', projectId: 'project-b', name: 'B', shell: 'bash', ptyId: 'pty-b' } + ] + rerender({ projectId: 'project-b' }) + + await waitFor(() => { + expect(mockWorkspaceStore.setActiveTab).toHaveBeenCalledWith('pane-active', 'term-b-live') + expect(mockTerminalStoreState.selectTerminal).toHaveBeenCalledWith('b-live') + }) + + spawnGate.resolve?.({ success: true, data: { id: 'pty-default-orphan' } }) + + await waitFor(() => { + expect(mockTerminalKill).toHaveBeenCalledWith('pty-default-orphan') + }) + + expect(mockTerminalStoreState.addTerminal).not.toHaveBeenCalled() + expect(mockTerminalStoreState.setTerminalPtyId).not.toHaveBeenCalled() + expect(mockTerminalStoreState.selectTerminal).not.toHaveBeenCalledWith('new-terminal') + }) +}) diff --git a/src/renderer/hooks/use-terminal-restore.ts b/src/renderer/hooks/use-terminal-restore.ts index 2ba9b40..387bed4 100644 --- a/src/renderer/hooks/use-terminal-restore.ts +++ b/src/renderer/hooks/use-terminal-restore.ts @@ -2,7 +2,7 @@ import { useEffect, useRef } from 'react' import { useProjectStore } from '../stores/project-store' import { useTerminalStore } from '../stores/terminal-store' import { useAppSettingsStore } from '../stores/app-settings-store' -import { useWorkspaceStore } from '../stores/workspace-store' +import { useWorkspaceStore, terminalTabId, findPaneContainingTab } from '../stores/workspace-store' import { terminalApi } from '@/lib/api' import { shellApi } from '@/lib/shell-api' import { @@ -57,6 +57,41 @@ function releaseGlobalSpawnLock(ownerId: string): void { } } +function resetSpawnCallCount(sessionId: string): void { + SPAWN_CALL_COUNT = 0 + debugLog('SPAWN_LOCK', `RESET SPAWN COUNT [${sessionId}]`) +} + +async function cleanupSpawnedPtys( + terminals: Array<{ ptyId?: string }>, + restoreId: string, + phase: string +): Promise { + const ptyIds = Array.from( + new Set( + terminals + .map((terminal) => terminal.ptyId) + .filter((ptyId): ptyId is string => !!ptyId) + ) + ) + + if (ptyIds.length === 0) { + return + } + + debugLog('restoreFromLayout', `CLEANING UP SPAWNED PTYS [${restoreId}]`, { + phase, + ptyIds + }) + + for (const ptyId of ptyIds) { + const killResult = await terminalApi.kill(ptyId) + if (!killResult.success) { + console.error('Failed to kill cancelled restore PTY:', killResult.error) + } + } +} + const IS_DEV = import.meta.env.DEV function debugLog(category: string, message: string, data?: unknown) { @@ -204,6 +239,8 @@ export function useTerminalRestore(): void { // Skip if no project selected or same project if (!activeProjectId || activeProjectId === previousProjectIdRef.current) { debugLog('useTerminalRestore', `SKIPPED [${callId}]: no project or same project`) + const idx = RESTORE_CALL_STACK.indexOf(callId) + if (idx > -1) RESTORE_CALL_STACK.splice(idx, 1) return } @@ -213,14 +250,17 @@ export function useTerminalRestore(): void { isRestoring: Array.from(isRestoringRef.current), hasLock: PROJECT_RESTORE_LOCKS.has(activeProjectId) }) + const idx = RESTORE_CALL_STACK.indexOf(callId) + if (idx > -1) RESTORE_CALL_STACK.splice(idx, 1) return } // Set flag immediately to prevent race condition isRestoringRef.current.add(activeProjectId) PROJECT_RESTORE_LOCKS.add(activeProjectId) - setTerminalRestoreInProgress(true) const projectIdToRestore = activeProjectId + const restoreOwnerId = `${projectIdToRestore}:${callId}` + setTerminalRestoreInProgress(projectIdToRestore, true, restoreOwnerId) debugLog('useTerminalRestore', `STARTING RESTORE [${callId}]`, { projectId: projectIdToRestore @@ -236,72 +276,100 @@ export function useTerminalRestore(): void { // FIX #5: Add cancellation token to handle cleanup properly let cancelled = false const cancelRestore = () => { cancelled = true } + const isCancelled = (): boolean => cancelled || previousProjectIdRef.current !== projectIdToRestore const restoreTerminals = async (): Promise => { try { // Check for cancellation before starting - if (cancelled) { + if (isCancelled()) { debugLog('useTerminalRestore', `CANCELLED [${callId}] before restore`) return } - // CRITICAL FIX: Kill all PTYs from previous project before switching - // This prevents orphaned PTYs from accumulating and causing "terminal spam" if (actualPreviousProjectId && actualPreviousProjectId !== projectIdToRestore) { - // First kill all PTYs from previous project - const terminalStore = useTerminalStore.getState() - const previousTerminals = terminalStore.terminals.filter( - (t) => t.projectId === actualPreviousProjectId && t.ptyId - ) - - debugLog('useTerminalRestore', `KILLING ${previousTerminals.length} PTYs from previous project [${actualPreviousProjectId}]`) - - for (const terminal of previousTerminals) { - if (!terminal.ptyId) { - continue - } - try { - await terminalApi.kill(terminal.ptyId) - debugLog('useTerminalRestore', `KILLED PTY [${terminal.ptyId}]`) - } catch (err) { - debugLog('useTerminalRestore', `FAILED TO KILL PTY [${terminal.ptyId}]`, { - error: err instanceof Error ? err.message : String(err) - }) - } - } - - // Then save the layout await saveTerminalLayout(actualPreviousProjectId) } - // Get fresh state after save completes + if (isCancelled()) { + debugLog('useTerminalRestore', `CANCELLED [${callId}] after save`) + return + } + const terminalStore = useTerminalStore.getState() const existingTerminals = terminalStore.terminals.filter( (t) => t.projectId === projectIdToRestore ) + const liveProjectTerminals = existingTerminals.filter((terminal) => !!terminal.ptyId) - // If terminals already exist in memory, just restore selection - if (existingTerminals.length > 0) { + if (liveProjectTerminals.length > 0) { const layout = await loadPersistedTerminals(projectIdToRestore) - selectTerminalForProject(projectIdToRestore, existingTerminals, layout) + if (isCancelled()) { + debugLog('useTerminalRestore', `CANCELLED [${callId}] after live layout load`) + return + } + + const workspaceStore = useWorkspaceStore.getState() + const terminalIdToSelect = selectTerminalForProject( + liveProjectTerminals, + layout + ) + + for (const terminal of liveProjectTerminals) { + workspaceStore.ensureTerminalTab( + terminal.id, + undefined, + terminal.id === terminalIdToSelect + ) + } + + if (isCancelled()) { + debugLog('useTerminalRestore', `CANCELLED [${callId}] before workspace selection`) + return + } + + const terminalTab = terminalIdToSelect ? terminalTabId(terminalIdToSelect) : null + const containingPane = terminalTab + ? findPaneContainingTab(useWorkspaceStore.getState().root, terminalTab) + : null + const activePane = containingPane ?? workspaceStore.getActivePaneLeaf() + if (terminalTab && activePane) { + workspaceStore.setActiveTab(activePane.id, terminalTab) + } + + if (terminalIdToSelect) { + if (isCancelled()) { + debugLog('useTerminalRestore', `CANCELLED [${callId}] before terminal selection`) + return + } + + useTerminalStore.getState().selectTerminal(terminalIdToSelect) + } return } // No terminals in memory - load from disk or create default const layout = await loadPersistedTerminals(projectIdToRestore) + if (isCancelled()) { + debugLog('useTerminalRestore', `CANCELLED [${callId}] after persisted layout load`) + return + } if (layout && layout.terminals.length > 0) { - await restoreFromLayout(projectIdToRestore, layout) + await restoreFromLayout(projectIdToRestore, layout, isCancelled) } else { - await createDefaultTerminal(projectIdToRestore) + await createDefaultTerminal(projectIdToRestore, isCancelled) } } catch (err: unknown) { debugLog('useTerminalRestore', `RESTORE ERROR [${callId}]`, { error: err instanceof Error ? err.message : String(err) }) console.error('Failed to restore terminals:', err) + if (isCancelled()) { + debugLog('useTerminalRestore', `CANCELLED [${callId}] after error`) + return + } // Fall back to default terminal - await createDefaultTerminal(projectIdToRestore) + await createDefaultTerminal(projectIdToRestore, isCancelled) } finally { // Only clean up if this restore was not cancelled if (!cancelled && isRestoringRef.current.has(projectIdToRestore) && PROJECT_RESTORE_LOCKS.has(projectIdToRestore)) { @@ -310,7 +378,7 @@ export function useTerminalRestore(): void { }) isRestoringRef.current.delete(projectIdToRestore) PROJECT_RESTORE_LOCKS.delete(projectIdToRestore) - setTerminalRestoreInProgress(false) + setTerminalRestoreInProgress(projectIdToRestore, false, restoreOwnerId) const idx = RESTORE_CALL_STACK.indexOf(callId) if (idx > -1) RESTORE_CALL_STACK.splice(idx, 1) } else if (cancelled) { @@ -319,7 +387,7 @@ export function useTerminalRestore(): void { }) isRestoringRef.current.delete(projectIdToRestore) PROJECT_RESTORE_LOCKS.delete(projectIdToRestore) - setTerminalRestoreInProgress(false) + setTerminalRestoreInProgress(projectIdToRestore, false, restoreOwnerId) } } } @@ -340,6 +408,8 @@ export function useTerminalRestore(): void { // FIX #5: Also clean up the restoring flag for this project on cleanup // eslint-disable-next-line react-hooks/exhaustive-deps -- Ref access in cleanup is intentional isRestoringRef.current.delete(projectIdForCleanup) + PROJECT_RESTORE_LOCKS.delete(projectIdForCleanup) + setTerminalRestoreInProgress(projectIdForCleanup, false, restoreOwnerId) } }, [activeProjectId]) } @@ -349,15 +419,13 @@ export function useTerminalRestore(): void { * Uses multiple matching strategies: ID match, then name match, then fallback */ function selectTerminalForProject( - projectId: string, existingTerminals: Array<{ id: string; name: string; projectId: string }>, layout: PersistedTerminalLayout | null -): void { +): string | null { if (existingTerminals.length === 0) { - return + return null } - const terminalStore = useTerminalStore.getState() let terminalIdToSelect: string | null = null if (layout?.activeTerminalId) { @@ -388,14 +456,17 @@ function selectTerminalForProject( terminalIdToSelect = existingTerminals[0].id } - // Always select a terminal - ensures the project has an active terminal - terminalStore.selectTerminal(terminalIdToSelect) + return terminalIdToSelect } /** * Restore terminals from persisted layout (only when no terminals exist in memory) */ -async function restoreFromLayout(projectId: string, layout: PersistedTerminalLayout): Promise { +async function restoreFromLayout( + projectId: string, + layout: PersistedTerminalLayout, + isCancelled: () => boolean +): Promise { const restoreId = `restore-${Math.random().toString(36).slice(2, 7)}` // FIX #2: Use proper lock acquire/release with owner tracking @@ -404,9 +475,11 @@ async function restoreFromLayout(projectId: string, layout: PersistedTerminalLay return } - if (SPAWN_CALL_COUNT >= MAX_SPAWN_LIMIT) { + resetSpawnCallCount(restoreId) + + if (layout.terminals.length > MAX_SPAWN_LIMIT) { debugLog('restoreFromLayout', `BLOCKED [${restoreId}] Spawn limit exceeded`, { - count: SPAWN_CALL_COUNT, + count: layout.terminals.length, limit: MAX_SPAWN_LIMIT }) releaseGlobalSpawnLock(restoreId) @@ -414,6 +487,11 @@ async function restoreFromLayout(projectId: string, layout: PersistedTerminalLay } try { + if (isCancelled()) { + debugLog('restoreFromLayout', `CANCELLED [${restoreId}] before restore`) + return + } + const terminalStore = useTerminalStore.getState() debugLog('restoreFromLayout', `START [${restoreId}] ACQUIRING LOCK`, { @@ -424,102 +502,132 @@ async function restoreFromLayout(projectId: string, layout: PersistedTerminalLay spawnCallCount: SPAWN_CALL_COUNT }) - // Create all terminals at once to avoid multiple re-renders - const newTerminals: Array<{ - id: string - name: string - projectId: string - shell: string - cwd?: string - output: never[] - pendingScrollback?: string[] - ptyId?: string - }> = [] - - // Map old IDs to new IDs for active terminal selection and pane remapping - const idMap = new Map() - - for (const persistedTerminal of layout.terminals) { - const terminalCallId = `${restoreId}-${persistedTerminal.name}-${Math.random().toString(36).slice(2, 5)}` - SPAWN_TRACKER.set(terminalCallId, (SPAWN_TRACKER.get(terminalCallId) || 0) + 1) - - debugLog('restoreFromLayout', `Spawning terminal [${terminalCallId}]`, { - name: persistedTerminal.name, - shell: persistedTerminal.shell, - cwd: persistedTerminal.cwd, - spawnCount: SPAWN_TRACKER.get(terminalCallId) - }) + // Create all terminals at once to avoid multiple re-renders + const newTerminals: Array<{ + id: string + name: string + projectId: string + shell: string + cwd?: string + output: never[] + pendingScrollback?: string[] + ptyId?: string + }> = [] + + // Map old IDs to new IDs for active terminal selection and pane remapping + const idMap = new Map() + + for (const persistedTerminal of layout.terminals) { + if (isCancelled()) { + await cleanupSpawnedPtys(newTerminals, restoreId, 'during restore loop') + debugLog('restoreFromLayout', `CANCELLED [${restoreId}] during restore loop`) + return + } - const newId = Date.now().toString() + Math.random().toString(36).slice(2, 5) - TERMINALS_PENDING_PTY_ASSIGNMENT.add(newId) + const terminalCallId = `${restoreId}-${persistedTerminal.name}-${Math.random().toString(36).slice(2, 5)}` + SPAWN_TRACKER.set(terminalCallId, (SPAWN_TRACKER.get(terminalCallId) || 0) + 1) - try { - const resolvedShell = await resolveShellToPath(persistedTerminal.shell) - const normalizedShell = normalizeShellForStartup(resolvedShell) - const spawnResult = await terminalApi.spawn({ - shell: normalizedShell, - cwd: persistedTerminal.cwd + debugLog('restoreFromLayout', `Spawning terminal [${terminalCallId}]`, { + name: persistedTerminal.name, + shell: persistedTerminal.shell, + cwd: persistedTerminal.cwd, + spawnCount: SPAWN_TRACKER.get(terminalCallId) }) - debugLog('restoreFromLayout', `Spawn result [${terminalCallId}]`, { - success: spawnResult.success, - error: spawnResult.success ? undefined : spawnResult.error, - ptyId: spawnResult.success ? spawnResult.data.id : 'FAILED' - }) + const newId = Date.now().toString() + Math.random().toString(36).slice(2, 5) + TERMINALS_PENDING_PTY_ASSIGNMENT.add(newId) - if (!spawnResult.success) { - debugLog('restoreFromLayout', `Spawn FAILED, skipping [${terminalCallId}]`) - continue - } + try { + const resolvedShell = await resolveShellToPath(persistedTerminal.shell) + if (isCancelled()) { + await cleanupSpawnedPtys(newTerminals, restoreId, 'after shell resolution') + debugLog('restoreFromLayout', `CANCELLED [${terminalCallId}] after shell resolution`) + return + } - // FIX #6: Increment SPAWN_CALL_COUNT for each successful spawn in the loop - SPAWN_CALL_COUNT++ + const normalizedShell = normalizeShellForStartup(resolvedShell) + const spawnResult = await terminalApi.spawn({ + shell: normalizedShell, + cwd: persistedTerminal.cwd + }) - idMap.set(persistedTerminal.id, newId) - newTerminals.push({ - id: newId, - name: persistedTerminal.name, - projectId, - shell: normalizedShell, - cwd: persistedTerminal.cwd, - output: [], - pendingScrollback: persistedTerminal.scrollback, - ptyId: spawnResult.data.id - }) - } finally { - TERMINALS_PENDING_PTY_ASSIGNMENT.delete(newId) + debugLog('restoreFromLayout', `Spawn result [${terminalCallId}]`, { + success: spawnResult.success, + error: spawnResult.success ? undefined : spawnResult.error, + ptyId: spawnResult.success ? spawnResult.data.id : 'FAILED' + }) + + if (!spawnResult.success) { + debugLog('restoreFromLayout', `Spawn FAILED, skipping [${terminalCallId}]`) + continue + } + + if (isCancelled()) { + debugLog('restoreFromLayout', `CANCELLED [${terminalCallId}] after spawn; killing PTY`, { + ptyId: spawnResult.data.id + }) + + const killResult = await terminalApi.kill(spawnResult.data.id) + if (!killResult.success) { + console.error('Failed to kill cancelled restore PTY:', killResult.error) + } + continue + } + + // FIX #6: Increment SPAWN_CALL_COUNT for each successful spawn in the loop + SPAWN_CALL_COUNT++ + + idMap.set(persistedTerminal.id, newId) + newTerminals.push({ + id: newId, + name: persistedTerminal.name, + projectId, + shell: normalizedShell, + cwd: persistedTerminal.cwd, + output: [], + pendingScrollback: persistedTerminal.scrollback, + ptyId: spawnResult.data.id + }) + } finally { + TERMINALS_PENDING_PTY_ASSIGNMENT.delete(newId) + } } - } - // Add all terminals at once - const existingTerminals = terminalStore.terminals - terminalStore.setTerminals([...existingTerminals, ...newTerminals]) + if (isCancelled()) { + await cleanupSpawnedPtys(newTerminals, restoreId, 'before store mutation') + debugLog('restoreFromLayout', `CANCELLED [${restoreId}] before store mutation`) + return + } - if (idMap.size > 0) { - useWorkspaceStore.getState().remapTerminalTabs(Object.fromEntries(idMap)) - } + // Add all terminals at once + const existingTerminals = terminalStore.terminals + terminalStore.setTerminals([...existingTerminals, ...newTerminals]) - // Determine which terminal should be active - let activeId = newTerminals[0]?.id || '' - if (layout.activeTerminalId && idMap.has(layout.activeTerminalId)) { - activeId = idMap.get(layout.activeTerminalId)! - } + if (idMap.size > 0) { + useWorkspaceStore.getState().remapTerminalTabs(Object.fromEntries(idMap)) + } - // Select the active terminal - if (activeId) { - terminalStore.selectTerminal(activeId) - } + // Determine which terminal should be active + let activeId = newTerminals[0]?.id || '' + if (layout.activeTerminalId && idMap.has(layout.activeTerminalId)) { + activeId = idMap.get(layout.activeTerminalId)! + } + + // Select the active terminal + if (activeId) { + terminalStore.selectTerminal(activeId) + } - // CRITICAL: Get fresh state after mutations for accurate logging - const freshTerminalStore = useTerminalStore.getState() + // CRITICAL: Get fresh state after mutations for accurate logging + const freshTerminalStore = useTerminalStore.getState() - debugLog('restoreFromLayout', `COMPLETE [${restoreId}]`, { - terminalsCreated: newTerminals.length, - totalTerminalsInStore: freshTerminalStore.terminals.length, - terminalIds: freshTerminalStore.terminals.map(t => ({ id: t.id, ptyId: t.ptyId })), - spawnTrackerEntries: SPAWN_TRACKER.size, - totalSpawnCalls: SPAWN_CALL_COUNT - }) + debugLog('restoreFromLayout', `COMPLETE [${restoreId}]`, { + terminalsCreated: newTerminals.length, + totalTerminalsInStore: freshTerminalStore.terminals.length, + terminalIds: freshTerminalStore.terminals.map(t => ({ id: t.id, ptyId: t.ptyId })), + spawnTrackerEntries: SPAWN_TRACKER.size, + totalSpawnCalls: SPAWN_CALL_COUNT + }) } finally { // FIX #2: Always release the global spawn lock releaseGlobalSpawnLock(restoreId) @@ -532,7 +640,10 @@ async function restoreFromLayout(projectId: string, layout: PersistedTerminalLay /** * Create a default terminal when no persisted data exists */ -async function createDefaultTerminal(projectId: string): Promise { +async function createDefaultTerminal( + projectId: string, + isCancelled: () => boolean +): Promise { const defaultId = `default-${Math.random().toString(36).slice(2, 7)}` // FIX #2: Use proper lock acquire/release with owner tracking @@ -541,16 +652,14 @@ async function createDefaultTerminal(projectId: string): Promise { return } - if (SPAWN_CALL_COUNT >= MAX_SPAWN_LIMIT) { - debugLog('createDefaultTerminal', `BLOCKED [${defaultId}] Spawn limit exceeded`, { - count: SPAWN_CALL_COUNT, - limit: MAX_SPAWN_LIMIT - }) - releaseGlobalSpawnLock(defaultId) - return - } + resetSpawnCallCount(defaultId) try { + if (isCancelled()) { + debugLog('createDefaultTerminal', `CANCELLED [${defaultId}] before restore`) + return + } + const terminalStore = useTerminalStore.getState() const projectStore = useProjectStore.getState() const appSettings = useAppSettingsStore.getState() @@ -590,6 +699,11 @@ async function createDefaultTerminal(projectId: string): Promise { const project = projectStore.projects.find((p) => p.id === projectId) const shellSetting = project?.defaultShell || appSettings.settings.defaultShell || '' const resolvedShell = await resolveShellToPath(shellSetting) + if (isCancelled()) { + debugLog('createDefaultTerminal', `CANCELLED [${defaultId}] after shell resolution`) + return + } + const shell = normalizeShellForStartup(resolvedShell) debugLog('createDefaultTerminal', `Spawning default terminal [${defaultId}]`, { @@ -608,6 +722,22 @@ async function createDefaultTerminal(projectId: string): Promise { ptyId: spawnResult.success ? spawnResult.data.id : 'FAILED' }) + if (isCancelled()) { + if (spawnResult.success) { + debugLog('createDefaultTerminal', `CANCELLED [${defaultId}] after spawn; killing PTY`, { + ptyId: spawnResult.data.id + }) + + const killResult = await terminalApi.kill(spawnResult.data.id) + if (!killResult.success) { + console.error('Failed to kill cancelled default terminal PTY:', killResult.error) + } + } else { + debugLog('createDefaultTerminal', `CANCELLED [${defaultId}] after failed spawn`) + } + return + } + if (!spawnResult.success) { debugLog('createDefaultTerminal', `Spawn FAILED [${defaultId}]`) return @@ -616,21 +746,21 @@ async function createDefaultTerminal(projectId: string): Promise { SPAWN_CALL_COUNT++ // Create default terminal - addTerminal also sets it as active - const newTerminal = terminalStore.addTerminal('Terminal 1', projectId, shell, project?.path) - terminalStore.setTerminalPtyId(newTerminal.id, spawnResult.data.id) + const newTerminal = terminalStore.addTerminal('Terminal 1', projectId, shell, project?.path) + terminalStore.setTerminalPtyId(newTerminal.id, spawnResult.data.id) - // Explicitly select to ensure activeTerminalId is set correctly - terminalStore.selectTerminal(newTerminal.id) + // Explicitly select to ensure activeTerminalId is set correctly + terminalStore.selectTerminal(newTerminal.id) - // CRITICAL: Get fresh state after mutations for accurate logging - const freshTerminalStore = useTerminalStore.getState() + // CRITICAL: Get fresh state after mutations for accurate logging + const freshTerminalStore = useTerminalStore.getState() - debugLog('createDefaultTerminal', `COMPLETE [${defaultId}]`, { - terminalId: newTerminal.id, - ptyId: spawnResult.data.id, - totalTerminalsInStore: freshTerminalStore.terminals.length, - totalSpawnCalls: SPAWN_CALL_COUNT - }) + debugLog('createDefaultTerminal', `COMPLETE [${defaultId}]`, { + terminalId: newTerminal.id, + ptyId: spawnResult.data.id, + totalTerminalsInStore: freshTerminalStore.terminals.length, + totalSpawnCalls: SPAWN_CALL_COUNT + }) } finally { // FIX #2: Always release the global spawn lock releaseGlobalSpawnLock(defaultId) diff --git a/src/renderer/hooks/useTerminalAutoSave.test.ts b/src/renderer/hooks/useTerminalAutoSave.test.ts index dbec11f..26e177a 100644 --- a/src/renderer/hooks/useTerminalAutoSave.test.ts +++ b/src/renderer/hooks/useTerminalAutoSave.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -import { serializeTerminalsForProject } from './useTerminalAutoSave' +import { + serializeTerminalsForProject, + setTerminalRestoreInProgress, + isTerminalRestoreInProgress +} from './useTerminalAutoSave' import { extractScrollback } from '../utils/terminal-registry' import type { Terminal } from '@/types/project' @@ -33,6 +37,19 @@ describe('useTerminalAutoSave', () => { vi.clearAllMocks() }) + describe('setTerminalRestoreInProgress', () => { + it('only clears restore state for the matching owner', () => { + setTerminalRestoreInProgress('proj-1', true, 'owner-a') + expect(isTerminalRestoreInProgress()).toBe(true) + + setTerminalRestoreInProgress('proj-1', false, 'owner-b') + expect(isTerminalRestoreInProgress()).toBe(true) + + setTerminalRestoreInProgress('proj-1', false, 'owner-a') + expect(isTerminalRestoreInProgress()).toBe(false) + }) + }) + describe('serializeTerminalsForProject', () => { it('should serialize terminals for a specific project', () => { const terminals: Terminal[] = [ diff --git a/src/renderer/hooks/useTerminalAutoSave.ts b/src/renderer/hooks/useTerminalAutoSave.ts index fb43ffc..1b36d74 100644 --- a/src/renderer/hooks/useTerminalAutoSave.ts +++ b/src/renderer/hooks/useTerminalAutoSave.ts @@ -10,14 +10,29 @@ import type { import { PersistenceKeys } from '../../shared/types/persistence.types' import { extractScrollback } from '../utils/terminal-registry' -let terminalRestoreInProgress = false +const terminalRestoreProjectsInProgress = new Map() -export function setTerminalRestoreInProgress(isRestoring: boolean): void { - terminalRestoreInProgress = isRestoring +export function setTerminalRestoreInProgress( + projectId: string, + isRestoring: boolean, + ownerId: string +): void { + if (!projectId || !ownerId) { + return + } + + if (isRestoring) { + terminalRestoreProjectsInProgress.set(projectId, ownerId) + return + } + + if (terminalRestoreProjectsInProgress.get(projectId) === ownerId) { + terminalRestoreProjectsInProgress.delete(projectId) + } } export function isTerminalRestoreInProgress(): boolean { - return terminalRestoreInProgress + return terminalRestoreProjectsInProgress.size > 0 } /** diff --git a/src/renderer/layouts/WorkspaceLayout.tsx b/src/renderer/layouts/WorkspaceLayout.tsx index 0ef1a13..c84896b 100644 --- a/src/renderer/layouts/WorkspaceLayout.tsx +++ b/src/renderer/layouts/WorkspaceLayout.tsx @@ -186,32 +186,33 @@ export default function WorkspaceLayout(): React.JSX.Element { } }, []) - // Sync terminal tabs with workspace store - // CRITICAL: Track sync calls to prevent loops - const syncCallCountRef = useRef(0) - const lastSyncTerminalsRef = useRef([]) + // Ensure tabs exist for currently visible project terminals. + // Project workspace loading/removal is owned by persistence + restore flows. + const ensureCallCountRef = useRef(0) + const lastEnsuredTerminalIdsRef = useRef([]) useEffect(() => { const terminalIds = terminals.map((terminal) => terminal.id) - // Skip if terminals haven't actually changed (same IDs) - const prevIds = lastSyncTerminalsRef.current - if (terminalIds.length === prevIds.length && - terminalIds.every((id, i) => id === prevIds[i])) { + const prevIds = lastEnsuredTerminalIdsRef.current + if (terminalIds.length === prevIds.length && terminalIds.every((id, i) => id === prevIds[i])) { return } - const syncId = `sync-${syncCallCountRef.current++}-${Date.now().toString().slice(-6)}` + const ensureId = `ensure-${ensureCallCountRef.current++}-${Date.now().toString().slice(-6)}` - console.log(`[WorkspaceLayout] syncTerminalTabs CALL [${syncId}]`, { + console.log(`[WorkspaceLayout] ensureTerminalTabs CALL [${ensureId}]`, { terminalCount: terminalIds.length, terminalIds, prevCount: prevIds.length, - callCount: syncCallCountRef.current + callCount: ensureCallCountRef.current }) - lastSyncTerminalsRef.current = terminalIds - useWorkspaceStore.getState().syncTerminalTabs(terminalIds) + lastEnsuredTerminalIdsRef.current = terminalIds + const workspaceStore = useWorkspaceStore.getState() + for (const terminalId of terminalIds) { + workspaceStore.ensureTerminalTab(terminalId) + } }, [terminals]) // Sync legacy stores (activeTerminalId, activeFilePath) from workspace pane tree diff --git a/src/renderer/stores/workspace-store.test.ts b/src/renderer/stores/workspace-store.test.ts index e997735..74cb42c 100644 --- a/src/renderer/stores/workspace-store.test.ts +++ b/src/renderer/stores/workspace-store.test.ts @@ -181,6 +181,36 @@ describe('workspace-store split/move invariants', () => { expect(remappedRight.activeTabId).toBe('term-new-b') }) + it('ensureTerminalTab adds missing terminal without stealing active tab', () => { + const store = useWorkspaceStore.getState() + const editorTab = createEditorTab('edit-/focused.ts') + + store.addTabToPane('pane-root', editorTab) + store.ensureTerminalTab('terminal-a') + + const pane = useWorkspaceStore.getState().root as LeafNode + expect(pane.tabs).toEqual([ + editorTab, + { type: 'terminal', id: 'term-terminal-a', terminalId: 'terminal-a' } + ]) + expect(pane.activeTabId).toBe(editorTab.id) + }) + + it('ensureTerminalTab can activate inserted terminal when requested', () => { + const store = useWorkspaceStore.getState() + const editorTab = createEditorTab('edit-/focused.ts') + + store.addTabToPane('pane-root', editorTab) + store.ensureTerminalTab('terminal-a', undefined, true) + + const pane = useWorkspaceStore.getState().root as LeafNode + expect(pane.tabs).toEqual([ + editorTab, + { type: 'terminal', id: 'term-terminal-a', terminalId: 'terminal-a' } + ]) + expect(pane.activeTabId).toBe('term-terminal-a') + }) + it('syncTerminalTabs prunes orphan tabs and preserves split shape', () => { const store = useWorkspaceStore.getState() const terminalA = createTerminalTab('a') diff --git a/src/renderer/stores/workspace-store.ts b/src/renderer/stores/workspace-store.ts index c67a78a..df93531 100644 --- a/src/renderer/stores/workspace-store.ts +++ b/src/renderer/stores/workspace-store.ts @@ -159,11 +159,13 @@ export interface WorkspaceState { syncTerminalTabs: (terminalIds: string[]) => void clearEditorTabs: () => void resetLayout: () => void + loadProjectWorkspace: (root: PaneNode, activePaneId?: string | null) => void syncEditorTabs: (filePaths: string[], activeTabId?: string | null) => void remapTerminalTabs: (idMap: Record) => void // New tab helpers addTerminalTab: (terminalId: string, targetPaneId?: string) => void + ensureTerminalTab: (terminalId: string, targetPaneId?: string, makeActive?: boolean) => void addEditorTab: (filePath: string, targetPaneId?: string) => void removeTab: (tabId: string) => void getNextTabId: (direction: 1 | -1) => string | null @@ -524,6 +526,34 @@ export const useWorkspaceStore = create((set, get) => { get().addTabToPane(paneId, tab) }, + ensureTerminalTab: (terminalId: string, targetPaneId?: string, makeActive: boolean = false): void => { + const id = terminalTabId(terminalId) + const { root, activePaneId } = get() + const paneId = targetPaneId ?? activePaneId + const existing = findPaneContainingTab(root, id) + + if (existing) { + return + } + + const tab: WorkspaceTab = { type: 'terminal', id, terminalId } + if (makeActive) { + get().addTabToPane(paneId, tab) + return + } + + const pane = findPaneById(root, paneId) + if (!pane || pane.type !== 'leaf') { + return + } + + const newRoot = updateLeaf(root, paneId, (leaf) => ({ + ...leaf, + tabs: [...leaf.tabs, tab] + })) + set({ root: newRoot }) + }, + addEditorTab: (filePath: string, targetPaneId?: string): void => { const id = editorTabId(filePath) const { root, activePaneId } = get() @@ -641,6 +671,17 @@ export const useWorkspaceStore = create((set, get) => { set({ root: leaf, activePaneId: leaf.id }) }, + loadProjectWorkspace: (root: PaneNode, activePaneId?: string | null): void => { + const normalizedRoot = normalizePaneTree(root) + const leaves = getAllLeafPanes(normalizedRoot) + const resolvedActivePaneId = + activePaneId && leaves.some((leaf) => leaf.id === activePaneId) + ? activePaneId + : leaves[0]?.id ?? normalizedRoot.id + + set({ root: normalizedRoot, activePaneId: resolvedActivePaneId }) + }, + syncEditorTabs: (filePaths: string[], restoredActiveTabId?: string | null): void => { const { root, activePaneId } = get() // For backward compat: put all editor tabs in the active pane