From 8d63c6e4fb043ca16f0d77204781d37b1b840c2a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 14 Apr 2026 14:52:44 -0700 Subject: [PATCH] fix: restore split group session state safely (#653) --- src/renderer/src/App.tsx | 2 + .../src/lib/workspace-session.test.ts | 12 ++++ src/renderer/src/lib/workspace-session.ts | 42 ++++++++++- src/renderer/src/store/slices/editor.ts | 41 +++++++---- .../slices/store-session-cascades.test.ts | 42 +++++++++-- .../src/store/slices/tabs-hydration.test.ts | 71 +++++++++++++++++++ .../src/store/slices/tabs-hydration.ts | 46 ++++++++++-- .../src/store/slices/worktrees.test.ts | 8 +++ src/renderer/src/store/slices/worktrees.ts | 3 + 9 files changed, 245 insertions(+), 22 deletions(-) diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 5daff8422..dd0deb219 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -74,6 +74,7 @@ function App(): React.JSX.Element { initGitHubCache: s.initGitHubCache, refreshAllGitHub: s.refreshAllGitHub, hydrateWorkspaceSession: s.hydrateWorkspaceSession, + hydrateTabsSession: s.hydrateTabsSession, hydrateEditorSession: s.hydrateEditorSession, hydrateBrowserSession: s.hydrateBrowserSession, fetchBrowserSessionProfiles: s.fetchBrowserSessionProfiles, @@ -151,6 +152,7 @@ function App(): React.JSX.Element { if (!cancelled) { actions.hydratePersistedUI(persistedUI) actions.hydrateWorkspaceSession(session) + actions.hydrateTabsSession(session) actions.hydrateEditorSession(session) actions.hydrateBrowserSession(session) await actions.fetchBrowserSessionProfiles() diff --git a/src/renderer/src/lib/workspace-session.test.ts b/src/renderer/src/lib/workspace-session.test.ts index 144b2a8b9..a19ad6ad9 100644 --- a/src/renderer/src/lib/workspace-session.test.ts +++ b/src/renderer/src/lib/workspace-session.test.ts @@ -100,4 +100,16 @@ describe('buildWorkspaceSessionPayload', () => { }) expect(payload.browserTabsByWorktree?.['wt-1'][0].loading).toBe(false) }) + + it('drops transient active editor markers that do not point at restored edit files', () => { + const payload = buildWorkspaceSessionPayload( + createSnapshot({ + activeFileIdByWorktree: { 'wt-1': '/tmp/demo.diff' }, + activeTabTypeByWorktree: { 'wt-1': 'editor', 'wt-2': 'terminal' } + }) + ) + + expect(payload.activeFileIdByWorktree).toEqual({}) + expect(payload.activeTabTypeByWorktree).toEqual({ 'wt-2': 'terminal' }) + }) }) diff --git a/src/renderer/src/lib/workspace-session.ts b/src/renderer/src/lib/workspace-session.ts index ae803c61a..cbfdf5a9e 100644 --- a/src/renderer/src/lib/workspace-session.ts +++ b/src/renderer/src/lib/workspace-session.ts @@ -40,6 +40,7 @@ export function buildEditorSessionData( > { const editFiles = openFiles.filter((f) => f.mode === 'edit') const byWorktree: Record = {} + const editFileIdsByWorktree: Record> = {} for (const f of editFiles) { const arr = byWorktree[f.worktreeId] ?? (byWorktree[f.worktreeId] = []) arr.push({ @@ -49,11 +50,48 @@ export function buildEditorSessionData( language: f.language, isPreview: f.isPreview || undefined }) + const ids = + editFileIdsByWorktree[f.worktreeId] ?? (editFileIdsByWorktree[f.worktreeId] = new Set()) + ids.add(f.id) } + + const activeFileEntries: [string, string][] = [] + for (const [worktreeId, fileId] of Object.entries(activeFileIdByWorktree)) { + if (!fileId) { + continue + } + if (editFileIdsByWorktree[worktreeId]?.has(fileId)) { + activeFileEntries.push([worktreeId, fileId]) + } + } + const persistedActiveFileIdByWorktree = Object.fromEntries(activeFileEntries) as Record< + string, + string + > + + const activeTabTypeEntries: [string, WorkspaceVisibleTabType][] = [] + for (const [worktreeId, tabType] of Object.entries(activeTabTypeByWorktree)) { + if (tabType !== 'editor') { + activeTabTypeEntries.push([worktreeId, tabType]) + continue + } + // Why: restart only restores edit-mode files. Persisting "editor" with a + // transient diff/conflict file ID creates a session payload that cannot be + // satisfied on startup and leaves the UI with no real editor tab to select. + // Only keep the editor marker when it points at a restored file. + if (persistedActiveFileIdByWorktree[worktreeId]) { + activeTabTypeEntries.push([worktreeId, tabType]) + } + } + const persistedActiveTabTypeByWorktree = Object.fromEntries(activeTabTypeEntries) as Record< + string, + WorkspaceVisibleTabType + > + return { openFilesByWorktree: byWorktree, - activeFileIdByWorktree, - activeTabTypeByWorktree + activeFileIdByWorktree: persistedActiveFileIdByWorktree, + activeTabTypeByWorktree: persistedActiveTabTypeByWorktree } } diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index 2a2235bbe..a6271644e 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -1468,21 +1468,27 @@ export const createEditorSlice: StateCreator = (s } } - if (openFiles.length === 0) { - return {} - } - // Why: use the store's activeWorktreeId (set by hydrateWorkspaceSession) // rather than the raw session value. hydrateWorkspaceSession may have // nulled out an invalid worktree ID, and we must respect that decision. const activeWorktreeId = s.activeWorktreeId - const activeFileId = activeWorktreeId + const fallbackActiveFileId = activeWorktreeId + ? (openFiles.find((f) => f.worktreeId === activeWorktreeId)?.id ?? null) + : null + const persistedActiveFileId = activeWorktreeId ? (persistedActiveFileIdByWorktree[activeWorktreeId] ?? null) : null // Why: verify the persisted active file still exists in the restored set. // The file may have been removed due to worktree validation or the // persisted data may reference a stale path. - const activeFileExists = activeFileId ? openFiles.some((f) => f.id === activeFileId) : false + const activeFileExists = persistedActiveFileId + ? openFiles.some((f) => f.id === persistedActiveFileId) + : false + // Why: if the previously active editor surface pointed at a transient + // diff/conflict tab, restart still restores any normal edit tabs for the + // worktree. Promote the first restored edit file so the UI comes back on + // a concrete file tab instead of an unselected editor surface. + const nextActiveFileId = activeFileExists ? persistedActiveFileId : fallbackActiveFileId const activeTabType: WorkspaceVisibleTabType = activeWorktreeId && persistedActiveTabTypeByWorktree[activeWorktreeId] ? persistedActiveTabTypeByWorktree[activeWorktreeId] @@ -1490,10 +1496,14 @@ export const createEditorSlice: StateCreator = (s // Filter per-worktree maps to only valid worktrees with valid file references const filteredActiveFileIdByWorktree = Object.fromEntries( - Object.entries(persistedActiveFileIdByWorktree).filter( - ([wId, fileId]) => - validWorktreeIds.has(wId) && fileId && openFiles.some((f) => f.id === fileId) - ) + [...validWorktreeIds].flatMap((wId) => { + const persistedFileId = persistedActiveFileIdByWorktree[wId] + if (persistedFileId && openFiles.some((f) => f.id === persistedFileId)) { + return [[wId, persistedFileId]] + } + const fallbackFileId = openFiles.find((f) => f.worktreeId === wId)?.id + return fallbackFileId ? [[wId, fallbackFileId]] : [] + }) ) const filteredActiveTabTypeByWorktree = Object.fromEntries( Object.entries(persistedActiveTabTypeByWorktree).filter(([wId, tabType]) => { @@ -1511,11 +1521,18 @@ export const createEditorSlice: StateCreator = (s }) ) + // Why: restart only restores edit-mode files. If the previous active + // surface for the current worktree was a transient diff/conflict view, + // we must clear the stale "editor" marker here so startup falls back to + // browser or terminal instead of showing an empty editor surface. + const nextActiveTabType = + nextActiveFileId || activeTabType !== 'editor' ? activeTabType : 'terminal' + return { openFiles, - activeFileId: activeFileExists ? activeFileId : null, + activeFileId: nextActiveFileId, activeFileIdByWorktree: filteredActiveFileIdByWorktree, - activeTabType: activeFileExists ? activeTabType : 'terminal', + activeTabType: nextActiveTabType, activeTabTypeByWorktree: filteredActiveTabTypeByWorktree } }) diff --git a/src/renderer/src/store/slices/store-session-cascades.test.ts b/src/renderer/src/store/slices/store-session-cascades.test.ts index 8bcacd057..bf2ab59f8 100644 --- a/src/renderer/src/store/slices/store-session-cascades.test.ts +++ b/src/renderer/src/store/slices/store-session-cascades.test.ts @@ -964,7 +964,40 @@ describe('hydrateEditorSession', () => { expect(s.activeTabType).toBe('terminal') }) - it('falls back to terminal if persisted activeFileId is missing from restored files', () => { + it('clears stale editor markers when no edit-mode files restore for the active worktree', () => { + const store = createTestStore() + const wt = 'repo1::/path/wt1' + + store.setState({ + repos: [ + { id: 'repo1', path: '/repo1', displayName: 'Repo 1', badgeColor: '#000', addedAt: 0 } + ], + worktreesByRepo: { + repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })] + }, + activeWorktreeId: wt, + activeTabType: 'editor' + }) + + store.getState().hydrateEditorSession({ + activeRepoId: 'repo1', + activeWorktreeId: wt, + activeTabId: null, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + activeFileIdByWorktree: { [wt]: `${wt}::diff::unstaged::src/index.ts` }, + activeTabTypeByWorktree: { [wt]: 'editor' } + }) + + const s = store.getState() + expect(s.openFiles).toHaveLength(0) + expect(s.activeFileId).toBeNull() + expect(s.activeTabType).toBe('terminal') + expect(s.activeFileIdByWorktree[wt]).toBeUndefined() + expect(s.activeTabTypeByWorktree[wt]).toBeUndefined() + }) + + it('promotes the first restored edit file if persisted activeFileId is missing', () => { const store = createTestStore() const wt = 'repo1::/path/wt1' @@ -1001,9 +1034,10 @@ describe('hydrateEditorSession', () => { const s = store.getState() expect(s.openFiles).toHaveLength(1) - expect(s.activeFileId).toBeNull() - expect(s.activeTabType).toBe('terminal') - expect(s.activeTabTypeByWorktree[wt]).toBeUndefined() + expect(s.activeFileId).toBe('/path/wt1/src/index.ts') + expect(s.activeTabType).toBe('editor') + expect(s.activeFileIdByWorktree[wt]).toBe('/path/wt1/src/index.ts') + expect(s.activeTabTypeByWorktree[wt]).toBe('editor') }) it('filters out files for deleted worktrees', () => { diff --git a/src/renderer/src/store/slices/tabs-hydration.test.ts b/src/renderer/src/store/slices/tabs-hydration.test.ts index 5b4c2f3dd..08c6385d9 100644 --- a/src/renderer/src/store/slices/tabs-hydration.test.ts +++ b/src/renderer/src/store/slices/tabs-hydration.test.ts @@ -137,6 +137,77 @@ describe('buildHydratedTabState – unified format', () => { expect(group.activeTabId).toBeNull() expect(group.tabOrder).toEqual(['t1']) }) + + it('collapses groups and layout when transient tabs are dropped during hydration', () => { + const session: WorkspaceSessionState = { + ...makeBaseSession(), + openFilesByWorktree: { + w1: [ + { + filePath: '/editor.ts', + relativePath: 'editor.ts', + worktreeId: 'w1', + language: 'typescript' + } + ] + }, + unifiedTabs: { + w1: [ + { + id: 'diff-1', + entityId: '/diff.ts', + groupId: 'g1', + worktreeId: 'w1', + contentType: 'diff', + label: 'diff.ts', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + }, + { + id: 'editor-1', + entityId: '/editor.ts', + groupId: 'g2', + worktreeId: 'w1', + contentType: 'editor', + label: 'editor.ts', + customLabel: null, + color: null, + sortOrder: 1, + createdAt: 2 + } + ] + }, + tabGroups: { + w1: [ + { id: 'g1', worktreeId: 'w1', activeTabId: 'diff-1', tabOrder: ['diff-1'] }, + { id: 'g2', worktreeId: 'w1', activeTabId: 'editor-1', tabOrder: ['editor-1'] } + ] + }, + tabGroupLayouts: { + w1: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', groupId: 'g1' }, + second: { type: 'leaf', groupId: 'g2' }, + ratio: 0.5 + } + }, + activeGroupIdByWorktree: { w1: 'g1' } + } + + const result = buildHydratedTabState(session, new Set(['w1'])) + + expect(result.unifiedTabsByWorktree.w1).toEqual([ + expect.objectContaining({ id: 'editor-1', groupId: 'g2', contentType: 'editor' }) + ]) + expect(result.groupsByWorktree.w1).toEqual([ + { id: 'g2', worktreeId: 'w1', activeTabId: 'editor-1', tabOrder: ['editor-1'] } + ]) + expect(result.activeGroupIdByWorktree.w1).toBe('g2') + expect(result.layoutByWorktree.w1).toEqual({ type: 'leaf', groupId: 'g2' }) + }) }) describe('buildHydratedTabState – legacy format', () => { diff --git a/src/renderer/src/store/slices/tabs-hydration.ts b/src/renderer/src/store/slices/tabs-hydration.ts index 81bdf1d30..d3547cf20 100644 --- a/src/renderer/src/store/slices/tabs-hydration.ts +++ b/src/renderer/src/store/slices/tabs-hydration.ts @@ -17,6 +17,27 @@ type HydratedTabState = { layoutByWorktree: Record } +function pruneLayoutForGroups( + root: TabGroupLayoutNode, + validGroupIds: Set +): TabGroupLayoutNode | null { + if (root.type === 'leaf') { + return validGroupIds.has(root.groupId) ? root : null + } + + const first = pruneLayoutForGroups(root.first, validGroupIds) + const second = pruneLayoutForGroups(root.second, validGroupIds) + + if (first === null) { + return second + } + if (second === null) { + return first + } + + return { ...root, first, second } +} + function hydrateUnifiedFormat( session: WorkspaceSessionState, validWorktreeIds: Set @@ -69,18 +90,35 @@ function hydrateUnifiedFormat( activeTabId: g.activeTabId && validTabIds.has(g.activeTabId) ? g.activeTabId : null } }) + const hydratedGroups = validatedGroups.filter((group, index) => { + const hadTabsBeforeHydration = groups[index]?.tabOrder.length > 0 + return !hadTabsBeforeHydration || group.tabOrder.length > 0 + }) + if (hydratedGroups.length === 0) { + if ((tabsByWorktree[worktreeId] ?? []).length === 0) { + delete tabsByWorktree[worktreeId] + } + continue + } - groupsByWorktree[worktreeId] = validatedGroups + groupsByWorktree[worktreeId] = hydratedGroups const activeGroupId = selectHydratedActiveGroupId( - validatedGroups, + hydratedGroups, session.activeGroupIdByWorktree?.[worktreeId] ) if (activeGroupId) { activeGroupIdByWorktree[worktreeId] = activeGroupId } - layoutByWorktree[worktreeId] = session.tabGroupLayouts?.[worktreeId] ?? { + const hydratedGroupIds = new Set(hydratedGroups.map((group) => group.id)) + const hydratedLayout = session.tabGroupLayouts?.[worktreeId] + ? pruneLayoutForGroups(session.tabGroupLayouts[worktreeId], hydratedGroupIds) + : null + layoutByWorktree[worktreeId] = hydratedLayout ?? { type: 'leaf', - groupId: validatedGroups[0].id + // Why: if transient-only groups were removed during hydration, the + // persisted split tree can collapse to a single surviving group. The + // fallback leaf keeps restore aligned with the remaining real tabs. + groupId: hydratedGroups[0].id } } diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 5e9ae7617..9d061e540 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -38,6 +38,7 @@ function createTestStore() { unifiedTabsByWorktree: {}, groupsByWorktree: {}, activeGroupIdByWorktree: {}, + layoutByWorktree: {}, openFiles: [], editorDrafts: {}, markdownViewMode: {}, @@ -304,6 +305,10 @@ describe('removeWorktree state cleanup', () => { activeGroupIdByWorktree: { 'repo1::/path/wt1': 'group-1', 'repo1::/path/wt2': 'group-2' + }, + layoutByWorktree: { + 'repo1::/path/wt1': { type: 'leaf', groupId: 'group-1' }, + 'repo1::/path/wt2': { type: 'leaf', groupId: 'group-2' } } } as unknown as Partial) @@ -321,6 +326,9 @@ describe('removeWorktree state cleanup', () => { expect(store.getState().activeGroupIdByWorktree).toEqual({ 'repo1::/path/wt2': 'group-2' }) + expect(store.getState().layoutByWorktree).toEqual({ + 'repo1::/path/wt2': { type: 'leaf', groupId: 'group-2' } + }) }) it('cleans up git caches for the removed worktree', async () => { diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 0de318659..ccbfb7a35 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -164,6 +164,8 @@ export const createWorktreeSlice: StateCreator delete nextGroupsByWorktree[worktreeId] const nextActiveGroupIdByWorktree = { ...s.activeGroupIdByWorktree } delete nextActiveGroupIdByWorktree[worktreeId] + const nextLayoutByWorktree = { ...s.layoutByWorktree } + delete nextLayoutByWorktree[worktreeId] // Why: git status / compare caches are keyed by worktree and stop being // refreshed once the worktree is deleted. Remove them here so deleted // worktrees cannot retain stale conflict badges, branch diffs, or compare @@ -231,6 +233,7 @@ export const createWorktreeSlice: StateCreator unifiedTabsByWorktree: nextUnifiedTabsByWorktree, groupsByWorktree: nextGroupsByWorktree, activeGroupIdByWorktree: nextActiveGroupIdByWorktree, + layoutByWorktree: nextLayoutByWorktree, editorDrafts: nextEditorDrafts, markdownViewMode: nextMarkdownViewMode, expandedDirs: nextExpandedDirs,