From 2c1240c45868e300514bec0ec657133ffdecad33 Mon Sep 17 00:00:00 2001 From: Matt Leaverton Date: Mon, 30 Mar 2026 11:55:09 -0500 Subject: [PATCH 1/5] =?UTF-8?q?fix:=20remove=20dead=20code=20=E2=80=94=20t?= =?UTF-8?q?erminal.runtime.updated=20broadcast=20and=20unused=20Redux=20ac?= =?UTF-8?q?tions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove server broadcast `terminal.runtime.updated` (no client handler existed), unused sessionsSlice actions (collapseAll, expandAll), and unused tabRegistrySlice action (clearClosedTabSnapshot). Clears noise before structural stability fixes. Co-Authored-By: Claude Opus 4.6 (1M context) --- server/ws-handler.ts | 58 +------------ shared/ws-protocol.ts | 11 --- src/store/sessionsSlice.ts | 8 -- src/store/tabRegistrySlice.ts | 4 - test/e2e-browser/perf/scenarios.ts | 4 +- ...rminal-create-reuse-running-claude.test.ts | 13 +-- ...erminal-create-reuse-running-codex.test.ts | 13 +-- test/server/ws-terminal-meta.test.ts | 26 ------ test/unit/client/store/sessionsSlice.test.ts | 81 ++----------------- .../client/store/state-edge-cases.test.ts | 24 +----- 10 files changed, 16 insertions(+), 226 deletions(-) diff --git a/server/ws-handler.ts b/server/ws-handler.ts index d5c4037c..7531c096 100644 --- a/server/ws-handler.ts +++ b/server/ws-handler.ts @@ -309,7 +309,7 @@ export class WsHandler { private screenshotRequests = new Map() private sessionsRevision = 0 private terminalsRevision = 0 - private terminalRuntimeRevisions = new Map() + private readonly serverInstanceId: string private readonly bootId: string // The runtime validator is authoritative here; we keep the field typed broadly because @@ -1435,7 +1435,6 @@ export class WsHandler { } if (attachResult === 'duplicate') return state.attachedTerminalIds.add(m.terminalId) - this.broadcastTerminalRuntimeUpdatedForId(m.terminalId) return } @@ -1447,7 +1446,6 @@ export class WsHandler { return } this.send(ws, { type: 'terminal.detached', terminalId: m.terminalId }) - this.broadcastTerminalRuntimeUpdatedForId(m.terminalId) return } @@ -1474,7 +1472,6 @@ export class WsHandler { return } this.broadcastTerminalsChanged() - this.broadcastTerminalRuntimeUpdatedForId(m.terminalId) return } @@ -2037,59 +2034,6 @@ export class WsHandler { }) } - private resolveTerminalRuntimePayload(terminalId: string): { - terminalId: string - title: string - status: 'running' | 'detached' | 'exited' - cwd?: string - pid?: number - } | null { - const record = this.registry.get(terminalId) as - | { - terminalId: string - title: string - status: 'running' | 'exited' - cwd?: string - pty?: { pid?: number } - clients?: Set - } - | null - | undefined - - if (!record) return null - - return { - terminalId: record.terminalId, - title: record.title, - status: record.status === 'exited' - ? 'exited' - : ((record.clients?.size ?? 0) > 0 ? 'running' : 'detached'), - ...(record.cwd ? { cwd: record.cwd } : {}), - ...(typeof record.pty?.pid === 'number' ? { pid: record.pty.pid } : {}), - } - } - - broadcastTerminalRuntimeUpdated(msg: { - terminalId: string - title: string - status: 'running' | 'detached' | 'exited' - cwd?: string - pid?: number - }): void { - const revision = (this.terminalRuntimeRevisions.get(msg.terminalId) ?? 0) + 1 - this.terminalRuntimeRevisions.set(msg.terminalId, revision) - this.broadcastAuthenticated({ - type: 'terminal.runtime.updated', - revision, - ...msg, - }) - } - - private broadcastTerminalRuntimeUpdatedForId(terminalId: string): void { - const payload = this.resolveTerminalRuntimePayload(terminalId) - if (!payload) return - this.broadcastTerminalRuntimeUpdated(payload) - } broadcastTerminalMetaUpdated(msg: { upsert?: TerminalMeta[]; remove?: string[] }): void { const parsed = TerminalMetaUpdatedSchema.safeParse({ diff --git a/shared/ws-protocol.ts b/shared/ws-protocol.ts index fb0190b8..69078dd3 100644 --- a/shared/ws-protocol.ts +++ b/shared/ws-protocol.ts @@ -484,16 +484,6 @@ export type TerminalsChangedMessage = { revision: number } -export type TerminalRuntimeUpdatedMessage = { - type: 'terminal.runtime.updated' - terminalId: string - revision: number - status: 'running' | 'detached' | 'exited' - title: string - cwd?: string - pid?: number -} - export type TerminalMetaUpdatedMessage = z.infer export type CodexActivityListResponseMessage = z.infer @@ -697,7 +687,6 @@ export type ServerMessage = | TerminalTitleUpdatedMessage | TerminalSessionAssociatedMessage | TerminalsChangedMessage - | TerminalRuntimeUpdatedMessage | TerminalMetaUpdatedMessage | TerminalInventoryMessage | CodexActivityListResponseMessage diff --git a/src/store/sessionsSlice.ts b/src/store/sessionsSlice.ts index e1117206..8fd47873 100644 --- a/src/store/sessionsSlice.ts +++ b/src/store/sessionsSlice.ts @@ -372,12 +372,6 @@ export const sessionsSlice = createSlice({ if (expanded) state.expandedProjects.add(projectPath) else state.expandedProjects.delete(projectPath) }, - collapseAll: (state) => { - state.expandedProjects = new Set() - }, - expandAll: (state) => { - state.expandedProjects = new Set(state.projects.map((p) => p.projectPath)) - }, }, }) @@ -398,8 +392,6 @@ export const { setLoadingMore, toggleProjectExpanded, setProjectExpanded, - collapseAll, - expandAll, } = sessionsSlice.actions diff --git a/src/store/tabRegistrySlice.ts b/src/store/tabRegistrySlice.ts index 9568c36b..0d2e2ac2 100644 --- a/src/store/tabRegistrySlice.ts +++ b/src/store/tabRegistrySlice.ts @@ -304,9 +304,6 @@ export const tabRegistrySlice = createSlice({ recordClosedTabSnapshot: (state, action: PayloadAction) => { state.localClosed[action.payload.tabKey] = action.payload }, - clearClosedTabSnapshot: (state, action: PayloadAction) => { - delete state.localClosed[action.payload] - }, pushReopenEntry: (state, action: PayloadAction) => { state.reopenStack.push(action.payload) if (state.reopenStack.length > REOPEN_STACK_MAX) { @@ -329,7 +326,6 @@ export const { setTabRegistrySnapshot, setTabRegistrySyncError, recordClosedTabSnapshot, - clearClosedTabSnapshot, pushReopenEntry, popReopenEntry, } = tabRegistrySlice.actions diff --git a/test/e2e-browser/perf/scenarios.ts b/test/e2e-browser/perf/scenarios.ts index 15304598..9de55214 100644 --- a/test/e2e-browser/perf/scenarios.ts +++ b/test/e2e-browser/perf/scenarios.ts @@ -54,7 +54,7 @@ export const AUDIT_SCENARIOS: readonly AuditScenarioDefinition[] = [ 'terminal.attach.ready', 'terminal.output', 'terminal.output.gap', - 'terminal.runtime.updated', + 'terminals.changed', ], buildUrl: ({ token }) => buildRootUrl(token), @@ -87,7 +87,7 @@ export const AUDIT_SCENARIOS: readonly AuditScenarioDefinition[] = [ 'terminal.attach.ready', 'terminal.output', 'terminal.output.gap', - 'terminal.runtime.updated', + 'terminals.changed', ], buildUrl: ({ token }) => buildRootUrl(token), diff --git a/test/server/ws-terminal-create-reuse-running-claude.test.ts b/test/server/ws-terminal-create-reuse-running-claude.test.ts index 5cd2a87d..c9e13a04 100644 --- a/test/server/ws-terminal-create-reuse-running-claude.test.ts +++ b/test/server/ws-terminal-create-reuse-running-claude.test.ts @@ -332,10 +332,9 @@ describe('terminal.create reuse running claude terminal', () => { await terminalsChangedPromise expect(registry.attachCalls).toHaveLength(0) - const attachMessagesPromise = waitForMessages(ws, [ + const attachReadyPromise = waitForMessage(ws, (m) => m.type === 'terminal.attach.ready' && m.attachRequestId === 'reuse-1-attach', - (m) => m.type === 'terminal.runtime.updated' && m.terminalId === created.terminalId, - ]) + ) ws.send(JSON.stringify({ type: 'terminal.attach', terminalId: created.terminalId, @@ -344,15 +343,9 @@ describe('terminal.create reuse running claude terminal', () => { rows: 40, attachRequestId: 'reuse-1-attach', })) - const [ready, runtimeUpdated] = await attachMessagesPromise + const ready = await attachReadyPromise expect(ready.type).toBe('terminal.attach.ready') - expect(runtimeUpdated).toEqual(expect.objectContaining({ - type: 'terminal.runtime.updated', - terminalId: created.terminalId, - status: 'running', - title: 'Shell', - })) expect(registry.attachCalls).toHaveLength(1) expect(registry.attachCalls[0]?.opts?.suppressOutput).toBe(true) expect(snapshotSpy).not.toHaveBeenCalled() diff --git a/test/server/ws-terminal-create-reuse-running-codex.test.ts b/test/server/ws-terminal-create-reuse-running-codex.test.ts index 4de63a5e..8f7605df 100644 --- a/test/server/ws-terminal-create-reuse-running-codex.test.ts +++ b/test/server/ws-terminal-create-reuse-running-codex.test.ts @@ -332,10 +332,9 @@ describe('terminal.create reuse running codex terminal', () => { expect(registry.attachCalls).toHaveLength(0) expect(registry.createCalls).toHaveLength(0) - const attachMessagesPromise = waitForMessages(ws, [ + const attachReadyPromise = waitForMessage(ws, (m) => m.type === 'terminal.attach.ready' && m.attachRequestId === 'reuse-existing-codex-attach', - (m) => m.type === 'terminal.runtime.updated' && m.terminalId === created.terminalId, - ]) + ) ws.send(JSON.stringify({ type: 'terminal.attach', terminalId: created.terminalId, @@ -344,14 +343,8 @@ describe('terminal.create reuse running codex terminal', () => { rows: 40, attachRequestId: 'reuse-existing-codex-attach', })) - const [ready, runtimeUpdated] = await attachMessagesPromise + const ready = await attachReadyPromise expect(ready.headSeq).toBeGreaterThanOrEqual(0) - expect(runtimeUpdated).toEqual(expect.objectContaining({ - type: 'terminal.runtime.updated', - terminalId: created.terminalId, - status: 'running', - title: 'Codex', - })) expect(registry.attachCalls).toHaveLength(1) expect(registry.attachCalls[0]?.terminalId).toBe('term-codex-existing') } finally { diff --git a/test/server/ws-terminal-meta.test.ts b/test/server/ws-terminal-meta.test.ts index c3a6b500..09efb6b1 100644 --- a/test/server/ws-terminal-meta.test.ts +++ b/test/server/ws-terminal-meta.test.ts @@ -131,30 +131,4 @@ describe('ws terminal metadata protocol', () => { ws.close() }) - it('broadcasts terminal.runtime.updated payloads', async () => { - const ws = new WebSocket(`ws://127.0.0.1:${port}/ws`) - await new Promise((resolve) => ws.on('open', () => resolve())) - ws.send(JSON.stringify({ type: 'hello', token: 'terminal-meta-token', protocolVersion: WS_PROTOCOL_VERSION })) - await waitForMessage(ws, (msg) => msg.type === 'ready') - - wsHandler.broadcastTerminalRuntimeUpdated({ - terminalId: 'term-1', - title: 'Shell', - status: 'running', - cwd: '/workspace/repo', - pid: 4242, - }) - - const updated = await waitForMessage(ws, (msg) => msg.type === 'terminal.runtime.updated') - expect(updated).toEqual({ - type: 'terminal.runtime.updated', - terminalId: 'term-1', - revision: 1, - title: 'Shell', - status: 'running', - cwd: '/workspace/repo', - pid: 4242, - }) - ws.close() - }) }) diff --git a/test/unit/client/store/sessionsSlice.test.ts b/test/unit/client/store/sessionsSlice.test.ts index 143fa408..47d261c3 100644 --- a/test/unit/client/store/sessionsSlice.test.ts +++ b/test/unit/client/store/sessionsSlice.test.ts @@ -8,8 +8,6 @@ import sessionsReducer, { applySessionsPatch, toggleProjectExpanded, setProjectExpanded, - collapseAll, - expandAll, SessionsState, setActiveSessionSurface, setSessionWindowData, @@ -447,73 +445,6 @@ describe('sessionsSlice', () => { }) }) - describe('collapseAll', () => { - it('collapses all expanded projects', () => { - const stateWithExpanded = { - ...initialState, - expandedProjects: new Set(['/project/one', '/project/two', '/project/three']), - } - const state = sessionsReducer(stateWithExpanded, collapseAll()) - expect(state.expandedProjects.size).toBe(0) - }) - - it('works when no projects are expanded', () => { - const state = sessionsReducer(initialState, collapseAll()) - expect(state.expandedProjects.size).toBe(0) - }) - - it('preserves projects list', () => { - const stateWithProjects = { - ...initialState, - projects: mockProjects, - expandedProjects: new Set(['/project/one']), - } - const state = sessionsReducer(stateWithProjects, collapseAll()) - expect(state.projects).toEqual(mockProjects) - }) - }) - - describe('expandAll', () => { - it('expands all projects in the list', () => { - const stateWithProjects = { - ...initialState, - projects: mockProjects, - expandedProjects: new Set(), - } - const state = sessionsReducer(stateWithProjects, expandAll()) - expect(state.expandedProjects.size).toBe(3) - expect(state.expandedProjects.has('/project/one')).toBe(true) - expect(state.expandedProjects.has('/project/two')).toBe(true) - expect(state.expandedProjects.has('/project/three')).toBe(true) - }) - - it('works when some projects are already expanded', () => { - const stateWithProjects = { - ...initialState, - projects: mockProjects, - expandedProjects: new Set(['/project/one']), - } - const state = sessionsReducer(stateWithProjects, expandAll()) - expect(state.expandedProjects.size).toBe(3) - }) - - it('replaces expandedProjects with new Set', () => { - const stateWithProjects = { - ...initialState, - projects: mockProjects, - expandedProjects: new Set(['/old/project']), - } - const state = sessionsReducer(stateWithProjects, expandAll()) - expect(state.expandedProjects.has('/old/project')).toBe(false) - expect(state.expandedProjects.size).toBe(3) - }) - - it('handles empty projects list', () => { - const state = sessionsReducer(initialState, expandAll()) - expect(state.expandedProjects.size).toBe(0) - }) - }) - describe('state immutability', () => { it('does not mutate original state on setProjects', () => { const originalProjects = [...initialState.projects] @@ -533,7 +464,7 @@ describe('sessionsSlice', () => { }) describe('complex scenarios', () => { - it('handles workflow: load projects, expand some, collapse all, expand all', () => { + it('handles workflow: load projects, expand some, toggle back', () => { let state = sessionsReducer(initialState, setProjects(mockProjects)) expect(state.projects.length).toBe(3) @@ -541,16 +472,16 @@ describe('sessionsSlice', () => { state = sessionsReducer(state, toggleProjectExpanded('/project/two')) expect(state.expandedProjects.size).toBe(2) - state = sessionsReducer(state, collapseAll()) + state = sessionsReducer(state, toggleProjectExpanded('/project/one')) + state = sessionsReducer(state, toggleProjectExpanded('/project/two')) expect(state.expandedProjects.size).toBe(0) - - state = sessionsReducer(state, expandAll()) - expect(state.expandedProjects.size).toBe(3) }) it('handles replacing projects while some are expanded', () => { let state = sessionsReducer(initialState, setProjects(mockProjects)) - state = sessionsReducer(state, expandAll()) + state = sessionsReducer(state, toggleProjectExpanded('/project/one')) + state = sessionsReducer(state, toggleProjectExpanded('/project/two')) + state = sessionsReducer(state, toggleProjectExpanded('/project/three')) expect(state.expandedProjects.size).toBe(3) const newProjects: ProjectGroup[] = [ diff --git a/test/unit/client/store/state-edge-cases.test.ts b/test/unit/client/store/state-edge-cases.test.ts index e047588a..c9419040 100644 --- a/test/unit/client/store/state-edge-cases.test.ts +++ b/test/unit/client/store/state-edge-cases.test.ts @@ -25,8 +25,6 @@ import sessionsReducer, { setProjects, toggleProjectExpanded, setProjectExpanded, - collapseAll, - expandAll, SessionsState, } from '@/store/sessionsSlice' import connectionReducer, { @@ -500,26 +498,6 @@ describe('State Edge Cases', () => { expect(store.getState().sessions.expandedProjects.has('/old/project')).toBe(false) }) - it('handles expandAll when projects are empty', () => { - const store = createTestStore() - - store.dispatch(expandAll()) - - expect(store.getState().sessions.expandedProjects.size).toBe(0) - }) - - it('handles collapseAll when nothing is expanded', () => { - const store = createTestStore() - - const projects: ProjectGroup[] = [ - { projectPath: '/project/a', sessions: [] }, - ] - store.dispatch(setProjects(projects)) - store.dispatch(collapseAll()) - - expect(store.getState().sessions.expandedProjects.size).toBe(0) - }) - it('handles rapid setProjects calls', () => { const store = createTestStore() @@ -575,7 +553,7 @@ describe('State Edge Cases', () => { { projectPath: 'C:\\Windows\\Path', sessions: [] }, ] store.dispatch(setProjects(projects)) - store.dispatch(expandAll()) + for (const p of projects) store.dispatch(toggleProjectExpanded(p.projectPath)) const expanded = store.getState().sessions.expandedProjects expect(expanded.size).toBe(4) From 4a676eb4f07bff034b5a4f7e8ec1d6fed6b4863f Mon Sep 17 00:00:00 2001 From: Matt Leaverton Date: Mon, 30 Mar 2026 12:23:01 -0500 Subject: [PATCH 2/5] =?UTF-8?q?fix:=20tombstone=20merge=20for=20cross-tab?= =?UTF-8?q?=20sync=20=E2=80=94=20prevent=20silent=20tab=20loss?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace full-replacement hydrateTabs with set-union merge. Tabs created in different browser tabs now survive cross-tab sync instead of being silently overwritten. Per-tab updatedAt resolves property conflicts (newer wins). Tombstone set prevents deleted tabs from resurrecting via remote sync. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/store/crossTabSync.ts | 14 +- src/store/persistMiddleware.ts | 6 + src/store/persistedState.ts | 8 + src/store/tabsSlice.ts | 109 +++++--- src/store/types.ts | 1 + .../client/store/state-edge-cases.test.ts | 11 +- .../unit/client/store/tabsSlice.merge.test.ts | 264 ++++++++++++++++++ 7 files changed, 366 insertions(+), 47 deletions(-) create mode 100644 test/unit/client/store/tabsSlice.merge.test.ts diff --git a/src/store/crossTabSync.ts b/src/store/crossTabSync.ts index 31a0d2a5..057f278c 100644 --- a/src/store/crossTabSync.ts +++ b/src/store/crossTabSync.ts @@ -51,22 +51,14 @@ function dispatchHydrateTabsFromPersisted(store: StoreLike, raw: string) { if (!parsed) return const remoteTabs = parsed.tabs.tabs - const remoteIds = new Set(remoteTabs.map((t: any) => t?.id).filter((id: any): id is string => typeof id === 'string')) - - const state = store.getState() - const localActive = state?.tabs?.activeTabId as string | null | undefined - - const desired = (localActive && remoteIds.has(localActive)) ? localActive : parsed.tabs.activeTabId - const activeTabId = - desired && remoteIds.has(desired) - ? desired - : (remoteTabs[0]?.id ?? null) + const remoteTombstones = parsed.tombstones || [] store.dispatch({ ...hydrateTabs({ tabs: remoteTabs, - activeTabId, + activeTabId: parsed.tabs.activeTabId, renameRequestTabId: null, + tombstones: remoteTombstones, } as any), meta: { skipPersist: true, source: 'cross-tab' }, }) diff --git a/src/store/persistMiddleware.ts b/src/store/persistMiddleware.ts index c370adc8..db2e1057 100644 --- a/src/store/persistMiddleware.ts +++ b/src/store/persistMiddleware.ts @@ -323,12 +323,18 @@ export const persistMiddleware: Middleware<{}, PersistState> = (store) => { const state = store.getState() if (tabsDirty) { + // Prune tombstones older than 1 hour + const TOMBSTONE_MAX_AGE_MS = 60 * 60 * 1000 + const tombstoneCutoff = Date.now() - TOMBSTONE_MAX_AGE_MS + const tombstones = (state.tabs.tombstones || []).filter((t: { deletedAt: number }) => t.deletedAt > tombstoneCutoff) + const tabsPayload = { tabs: { // Persist only stable tab state. Keep ephemeral UI fields out of storage. activeTabId: state.tabs.activeTabId, tabs: state.tabs.tabs.map(stripTabVolatileFields), }, + tombstones, } try { diff --git a/src/store/persistedState.ts b/src/store/persistedState.ts index e3daf187..8efd7b10 100644 --- a/src/store/persistedState.ts +++ b/src/store/persistedState.ts @@ -24,14 +24,21 @@ const zPersistedTabsState = z.object({ tabs: z.array(zTab), }).passthrough() +const zTombstone = z.object({ + id: z.string(), + deletedAt: z.number(), +}) + const zPersistedTabsPayload = z.object({ version: z.number().optional(), tabs: zPersistedTabsState, + tombstones: z.array(zTombstone).optional(), }).passthrough() export type ParsedPersistedTabs = { version: number tabs: z.infer + tombstones: Array<{ id: string; deletedAt: number }> } export function parsePersistedTabsRaw(raw: string): ParsedPersistedTabs | null { @@ -54,6 +61,7 @@ export function parsePersistedTabsRaw(raw: string): ParsedPersistedTabs | null { ...res.data.tabs, activeTabId: res.data.tabs.activeTabId ?? null, }, + tombstones: res.data.tombstones || [], } } diff --git a/src/store/tabsSlice.ts b/src/store/tabsSlice.ts index 5ad624a9..93a08104 100644 --- a/src/store/tabsSlice.ts +++ b/src/store/tabsSlice.ts @@ -24,12 +24,31 @@ import { mergeSessionMetadataByKey } from '@/lib/session-metadata' const log = createLogger('TabsSlice') +export type Tombstone = { id: string; deletedAt: number } + export interface TabsState { tabs: Tab[] activeTabId: string | null // Ephemeral UI signal: request TabBar to enter inline rename mode for a tab. // This must never be persisted. renameRequestTabId: string | null + // IDs of tabs that were explicitly closed. Prevents resurrection during cross-tab merge. + tombstones: Tombstone[] +} + +function migrateTabFields(t: Tab): Tab { + const legacyClaudeSessionId = (t as any).claudeSessionId as string | undefined + return { + ...t, + codingCliSessionId: t.codingCliSessionId || legacyClaudeSessionId, + codingCliProvider: t.codingCliProvider || (legacyClaudeSessionId ? 'claude' : undefined), + createdAt: t.createdAt || Date.now(), + createRequestId: (t as any).createRequestId || t.id, + status: t.status || 'creating', + mode: t.mode || 'shell', + shell: t.shell || 'system', + lastInputAt: t.lastInputAt, + } } // Load persisted tabs state directly at module initialization time @@ -39,6 +58,7 @@ function loadInitialTabsState(): TabsState { tabs: [], activeTabId: null, renameRequestTabId: null, + tombstones: [], } try { @@ -51,21 +71,7 @@ function loadInitialTabsState(): TabsState { log.debug('Loaded initial state from localStorage:', tabsState.tabs.map((t) => t.id)) - // Apply same transformations as hydrateTabs to ensure consistency - const mappedTabs = tabsState.tabs.map((t: Tab) => { - const legacyClaudeSessionId = (t as any).claudeSessionId as string | undefined - return { - ...t, - codingCliSessionId: t.codingCliSessionId || legacyClaudeSessionId, - codingCliProvider: t.codingCliProvider || (legacyClaudeSessionId ? 'claude' : undefined), - createdAt: t.createdAt || Date.now(), - createRequestId: (t as any).createRequestId || t.id, - status: t.status || 'creating', - mode: t.mode || 'shell', - shell: t.shell || 'system', - lastInputAt: t.lastInputAt, - } - }) + const mappedTabs = tabsState.tabs.map(migrateTabFields) const desired = tabsState.activeTabId const has = desired && mappedTabs.some((t) => t.id === desired) @@ -73,6 +79,7 @@ function loadInitialTabsState(): TabsState { tabs: mappedTabs, activeTabId: has ? desired! : (mappedTabs[0]?.id ?? null), renameRequestTabId: null, + tombstones: Array.isArray((parsed as any)?.tombstones) ? (parsed as any).tombstones : [], } } catch (err) { log.error('Failed to load from localStorage:', err) @@ -130,6 +137,7 @@ export const tabsSlice = createSlice({ resumeSessionId: payload.resumeSessionId, sessionMetadataByKey: payload.sessionMetadataByKey, createdAt: Date.now(), + updatedAt: Date.now(), titleSetByUser: payload.titleSetByUser, lastInputAt: undefined, } @@ -147,7 +155,10 @@ export const tabsSlice = createSlice({ }, updateTab: (state, action: PayloadAction<{ id: string; updates: Partial }>) => { const tab = state.tabs.find((t) => t.id === action.payload.id) - if (tab) Object.assign(tab, action.payload.updates) + if (tab) { + Object.assign(tab, action.payload.updates) + tab.updatedAt = Date.now() + } }, removeTab: (state, action: PayloadAction) => { const removedTabId = action.payload @@ -155,6 +166,8 @@ export const tabsSlice = createSlice({ const wasActive = state.activeTabId === removedTabId state.tabs = state.tabs.filter((t) => t.id !== removedTabId) + if (!state.tombstones) state.tombstones = [] + state.tombstones.push({ id: removedTabId, deletedAt: Date.now() }) if (wasActive) { if (state.tabs.length === 0) { @@ -167,23 +180,55 @@ export const tabsSlice = createSlice({ } }, hydrateTabs: (state, action: PayloadAction) => { - // Basic sanity: ensure dates exist, status defaults. - state.tabs = (action.payload.tabs || []).map((t) => { - const legacyClaudeSessionId = (t as any).claudeSessionId as string | undefined - return { - ...t, - codingCliSessionId: t.codingCliSessionId || legacyClaudeSessionId, - codingCliProvider: t.codingCliProvider || (legacyClaudeSessionId ? 'claude' : undefined), - createdAt: t.createdAt || Date.now(), - createRequestId: (t as any).createRequestId || t.id, - status: t.status || 'creating', - mode: t.mode || 'shell', - shell: t.shell || 'system', + const remoteTabs = (action.payload.tabs || []).map(migrateTabFields) + const remoteTombstones: Tombstone[] = Array.isArray(action.payload.tombstones) ? action.payload.tombstones : [] + + // Union tombstones from both sides, deduped by ID + const tombstoneMap = new Map() + for (const t of (state.tombstones || [])) tombstoneMap.set(t.id, Math.max(tombstoneMap.get(t.id) ?? 0, t.deletedAt)) + for (const t of remoteTombstones) tombstoneMap.set(t.id, Math.max(tombstoneMap.get(t.id) ?? 0, t.deletedAt)) + state.tombstones = Array.from(tombstoneMap, ([id, deletedAt]) => ({ id, deletedAt })) + + const tombstoned = new Set(tombstoneMap.keys()) + const localById = new Map(state.tabs.map((t) => [t.id, t])) + const remoteById = new Map(remoteTabs.map((t) => [t.id, t])) + + // Build merged list: remote order for shared tabs, then local-only tabs appended + const merged: Tab[] = [] + const seen = new Set() + + for (const remoteTab of remoteTabs) { + if (tombstoned.has(remoteTab.id)) continue + seen.add(remoteTab.id) + const localTab = localById.get(remoteTab.id) + if (localTab) { + // Both sides have this tab — resolve by updatedAt (remote wins ties) + merged.push((localTab.updatedAt ?? 0) > (remoteTab.updatedAt ?? 0) ? localTab : remoteTab) + } else { + merged.push(remoteTab) } - }) - const desired = action.payload.activeTabId - const has = desired && state.tabs.some((t) => t.id === desired) - state.activeTabId = has ? desired! : (state.tabs[0]?.id ?? null) + } + + // Append local-only tabs (not in remote, not tombstoned) + for (const localTab of state.tabs) { + if (seen.has(localTab.id) || tombstoned.has(localTab.id)) continue + if (!remoteById.has(localTab.id)) { + merged.push(localTab) + } + } + + state.tabs = merged + + // Prefer local activeTabId if it still exists in merged set + const localActive = state.activeTabId + const mergedIds = new Set(merged.map((t) => t.id)) + if (localActive && mergedIds.has(localActive)) { + // keep local + } else { + const desired = action.payload.activeTabId + state.activeTabId = (desired && mergedIds.has(desired)) ? desired : (merged[0]?.id ?? null) + } + state.renameRequestTabId = null }, reorderTabs: ( diff --git a/src/store/types.ts b/src/store/types.ts index 5f69ea0f..3d89e638 100644 --- a/src/store/types.ts +++ b/src/store/types.ts @@ -60,6 +60,7 @@ export interface Tab { resumeSessionId?: string // Mirrored from pane content on session association; serves as fallback if pane layout is lost sessionMetadataByKey?: Record createdAt: number + updatedAt?: number titleSetByUser?: boolean // If true, don't auto-update title lastInputAt?: number } diff --git a/test/unit/client/store/state-edge-cases.test.ts b/test/unit/client/store/state-edge-cases.test.ts index c9419040..d9447e31 100644 --- a/test/unit/client/store/state-edge-cases.test.ts +++ b/test/unit/client/store/state-edge-cases.test.ts @@ -350,7 +350,7 @@ describe('State Edge Cases', () => { expect(store.getState().tabs.tabs).toHaveLength(0) }) - it('handles hydrateTabs called multiple times', () => { + it('handles hydrateTabs called multiple times — set union', () => { const store = createTestStore() const firstHydration: Tab[] = [ @@ -364,11 +364,14 @@ describe('State Edge Cases', () => { store.dispatch(hydrateTabs({ tabs: firstHydration, activeTabId: 'first-1' })) store.dispatch(hydrateTabs({ tabs: secondHydration, activeTabId: 'second-2' })) - // Second hydration should completely replace first + // Set union: remote tabs (second-1, second-2) + local-only (first-1) appended const state = store.getState().tabs - expect(state.tabs).toHaveLength(2) + expect(state.tabs).toHaveLength(3) expect(state.tabs[0].id).toBe('second-1') - expect(state.activeTabId).toBe('second-2') + expect(state.tabs[1].id).toBe('second-2') + expect(state.tabs[2].id).toBe('first-1') + // Local activeTabId (first-1) still exists in merged set, so it's preserved + expect(state.activeTabId).toBe('first-1') }) }) diff --git a/test/unit/client/store/tabsSlice.merge.test.ts b/test/unit/client/store/tabsSlice.merge.test.ts new file mode 100644 index 00000000..9e964f08 --- /dev/null +++ b/test/unit/client/store/tabsSlice.merge.test.ts @@ -0,0 +1,264 @@ +// Tests for cross-tab tombstone merge in hydrateTabs +import { describe, it, expect } from 'vitest' +import tabsReducer, { addTab, hydrateTabs, removeTab, updateTab } from '@/store/tabsSlice' +import type { Tab } from '@/store/types' +import type { TabsState } from '@/store/tabsSlice' + +function makeTab(overrides: Partial & { id: string }): Tab { + return { + createRequestId: overrides.id, + title: `Tab ${overrides.id}`, + status: 'running', + mode: 'shell', + shell: 'system', + createdAt: Date.now(), + updatedAt: Date.now(), + ...overrides, + } +} + +function makeState(tabs: Tab[], activeTabId?: string | null, tombstones?: TabsState['tombstones']): TabsState { + return { + tabs, + activeTabId: activeTabId ?? tabs[0]?.id ?? null, + renameRequestTabId: null, + tombstones: tombstones ?? [], + } +} + +describe('hydrateTabs merge', () => { + it('merges local and remote tabs as a set union', () => { + const shared = makeTab({ id: 'shared', title: 'Shared' }) + const localOnly = makeTab({ id: 'local-only', title: 'Local Only' }) + const remoteOnly = makeTab({ id: 'remote-only', title: 'Remote Only' }) + + const localState = makeState([shared, localOnly], 'shared') + + const result = tabsReducer( + localState, + hydrateTabs({ + tabs: [shared, remoteOnly], + activeTabId: 'shared', + renameRequestTabId: null, + tombstones: [], + }) + ) + + const ids = result.tabs.map(t => t.id) + // All three should be present — union, not replacement + expect(ids).toContain('shared') + expect(ids).toContain('local-only') + expect(ids).toContain('remote-only') + expect(result.tabs).toHaveLength(3) + }) + + it('does not resurrect tombstoned tabs from remote', () => { + const localTab = makeTab({ id: 'alive', title: 'Alive' }) + const deletedTab = makeTab({ id: 'deleted', title: 'Deleted' }) + + const localState = makeState( + [localTab], + 'alive', + [{ id: 'deleted', deletedAt: Date.now() }], + ) + + const result = tabsReducer( + localState, + hydrateTabs({ + tabs: [localTab, deletedTab], + activeTabId: 'alive', + renameRequestTabId: null, + tombstones: [], + }) + ) + + const ids = result.tabs.map(t => t.id) + expect(ids).toContain('alive') + expect(ids).not.toContain('deleted') + }) + + it('does not resurrect tabs tombstoned in remote', () => { + const alive = makeTab({ id: 'alive', title: 'Alive' }) + const deletedRemotely = makeTab({ id: 'deleted-remote', title: 'Was Deleted' }) + + const localState = makeState([alive, deletedRemotely], 'alive') + + const result = tabsReducer( + localState, + hydrateTabs({ + tabs: [alive], + activeTabId: 'alive', + renameRequestTabId: null, + tombstones: [{ id: 'deleted-remote', deletedAt: Date.now() }], + }) + ) + + const ids = result.tabs.map(t => t.id) + expect(ids).toContain('alive') + expect(ids).not.toContain('deleted-remote') + }) + + it('resolves property conflicts using updatedAt — newer wins', () => { + const now = Date.now() + const localTab = makeTab({ id: 'tab-1', title: 'Local Title', updatedAt: now + 1000 }) + const remoteTab = makeTab({ id: 'tab-1', title: 'Remote Title', updatedAt: now }) + + const localState = makeState([localTab]) + + const result = tabsReducer( + localState, + hydrateTabs({ + tabs: [remoteTab], + activeTabId: 'tab-1', + renameRequestTabId: null, + tombstones: [], + }) + ) + + // Local tab is newer, so local properties win + expect(result.tabs[0].title).toBe('Local Title') + }) + + it('resolves property conflicts — remote wins when newer', () => { + const now = Date.now() + const localTab = makeTab({ id: 'tab-1', title: 'Local Title', updatedAt: now }) + const remoteTab = makeTab({ id: 'tab-1', title: 'Remote Title', updatedAt: now + 1000 }) + + const localState = makeState([localTab]) + + const result = tabsReducer( + localState, + hydrateTabs({ + tabs: [remoteTab], + activeTabId: 'tab-1', + renameRequestTabId: null, + tombstones: [], + }) + ) + + // Remote tab is newer, so remote properties win + expect(result.tabs[0].title).toBe('Remote Title') + }) + + it('remote wins ties on updatedAt', () => { + const now = Date.now() + const localTab = makeTab({ id: 'tab-1', title: 'Local', updatedAt: now }) + const remoteTab = makeTab({ id: 'tab-1', title: 'Remote', updatedAt: now }) + + const localState = makeState([localTab]) + + const result = tabsReducer( + localState, + hydrateTabs({ + tabs: [remoteTab], + activeTabId: 'tab-1', + renameRequestTabId: null, + tombstones: [], + }) + ) + + expect(result.tabs[0].title).toBe('Remote') + }) + + it('uses remote order for shared tabs, appends local-only tabs', () => { + const a = makeTab({ id: 'a', title: 'A' }) + const b = makeTab({ id: 'b', title: 'B' }) + const c = makeTab({ id: 'c', title: 'C' }) + const localOnly = makeTab({ id: 'local', title: 'Local' }) + + // Local order: a, b, c, local + const localState = makeState([a, b, c, localOnly]) + + // Remote order is reversed: c, b, a + const result = tabsReducer( + localState, + hydrateTabs({ + tabs: [c, b, a], + activeTabId: 'a', + renameRequestTabId: null, + tombstones: [], + }) + ) + + const ids = result.tabs.map(t => t.id) + // Shared tabs use remote order (c, b, a), local-only appended + expect(ids).toEqual(['c', 'b', 'a', 'local']) + }) + + it('preserves local activeTabId if it exists in merged set', () => { + const tab1 = makeTab({ id: 't1' }) + const tab2 = makeTab({ id: 't2' }) + + const localState = makeState([tab1], 't1') + + const result = tabsReducer( + localState, + hydrateTabs({ + tabs: [tab1, tab2], + activeTabId: 't2', + renameRequestTabId: null, + tombstones: [], + }) + ) + + // Local activeTabId is preferred if it exists in merged set + expect(result.activeTabId).toBe('t1') + }) + + it('unions tombstones from both local and remote', () => { + const tab = makeTab({ id: 'alive' }) + const localState = makeState( + [tab], + 'alive', + [{ id: 'local-deleted', deletedAt: Date.now() }], + ) + + const result = tabsReducer( + localState, + hydrateTabs({ + tabs: [tab], + activeTabId: 'alive', + renameRequestTabId: null, + tombstones: [{ id: 'remote-deleted', deletedAt: Date.now() }], + }) + ) + + const tombstoneIds = result.tombstones.map(t => t.id) + expect(tombstoneIds).toContain('local-deleted') + expect(tombstoneIds).toContain('remote-deleted') + }) +}) + +describe('removeTab adds tombstone', () => { + it('records a tombstone when removing a tab', () => { + const tab1 = makeTab({ id: 't1' }) + const tab2 = makeTab({ id: 't2' }) + const state = makeState([tab1, tab2], 't1') + + const result = tabsReducer(state, removeTab('t1')) + + expect(result.tabs.map(t => t.id)).toEqual(['t2']) + expect(result.tombstones).toHaveLength(1) + expect(result.tombstones[0].id).toBe('t1') + expect(result.tombstones[0].deletedAt).toBeGreaterThan(0) + }) +}) + +describe('tab mutations set updatedAt', () => { + it('addTab sets updatedAt', () => { + const before = Date.now() + const state = tabsReducer(makeState([]), addTab({ title: 'New' })) + const tab = state.tabs[0] + expect(tab.updatedAt).toBeGreaterThanOrEqual(before) + }) + + it('updateTab bumps updatedAt', () => { + const tab = makeTab({ id: 't1', updatedAt: 1000 }) + const state = makeState([tab]) + + const before = Date.now() + const result = tabsReducer(state, updateTab({ id: 't1', updates: { title: 'Renamed' } })) + expect(result.tabs[0].updatedAt).toBeGreaterThanOrEqual(before) + expect(result.tabs[0].title).toBe('Renamed') + }) +}) From b6636ec39af57feebabd28615cc41a487d757d4c Mon Sep 17 00:00:00 2001 From: Matt Leaverton Date: Mon, 30 Mar 2026 12:37:21 -0500 Subject: [PATCH 3/5] fix: atomic tabs+panes persistence via single layout key Merge separate freshell.tabs.v2 and freshell.panes.v2 localStorage keys into a single freshell.layout.v3 key. Eliminates partial-write vulnerability where tabs could persist but panes fail, causing orphaned tabs with picker content. Cross-tab sync now handles one storage event instead of two, removing the readAndHydratePairedKey workaround. Includes migration from v2 keys on first load. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/store/crossTabSync.ts | 72 ++--- src/store/panesSlice.ts | 10 +- src/store/persistMiddleware.ts | 133 ++++++--- src/store/persistedState.ts | 106 ++++++- src/store/storage-keys.ts | 2 + src/store/tabsSlice.ts | 17 +- .../client/store/atomicPersistence.test.ts | 114 ++++++++ test/unit/client/store/crossTabSync.test.ts | 270 ++++++++++-------- .../client/store/panesPersistence.test.ts | 42 +-- .../unit/client/store/tabsPersistence.test.ts | 3 +- 10 files changed, 521 insertions(+), 248 deletions(-) create mode 100644 test/unit/client/store/atomicPersistence.test.ts diff --git a/src/store/crossTabSync.ts b/src/store/crossTabSync.ts index 057f278c..4a3c56d7 100644 --- a/src/store/crossTabSync.ts +++ b/src/store/crossTabSync.ts @@ -5,7 +5,7 @@ import { setLocalSettings } from './settingsSlice' import { setTabRegistrySearchRangeDays } from './tabRegistrySlice' import { hydrateTabs } from './tabsSlice' import { getPendingBrowserPreferencesWriteState } from './browserPreferencesPersistence' -import { parsePersistedPanesRaw, parsePersistedTabsRaw, PANES_STORAGE_KEY, TABS_STORAGE_KEY } from './persistedState' +import { parsePersistedLayoutRaw, LAYOUT_STORAGE_KEY } from './persistedState' import { getPersistBroadcastSourceId, onPersistBroadcast, PERSIST_BROADCAST_CHANNEL_NAME } from './persistBroadcast' import { BROWSER_PREFERENCES_STORAGE_KEY } from './storage-keys' import { parseBrowserPreferencesRaw, resolveBrowserPreferenceSettings } from '@/lib/browser-preferences' @@ -46,34 +46,27 @@ function collectPaneIdsSafe(node: unknown): string[] { return ids } -function dispatchHydrateTabsFromPersisted(store: StoreLike, raw: string) { - const parsed = parsePersistedTabsRaw(raw) +function dispatchHydrateLayoutFromPersisted(store: StoreLike, raw: string) { + const parsed = parsePersistedLayoutRaw(raw) if (!parsed) return - const remoteTabs = parsed.tabs.tabs - const remoteTombstones = parsed.tombstones || [] - + // Hydrate tabs with merge store.dispatch({ ...hydrateTabs({ - tabs: remoteTabs, + tabs: parsed.tabs.tabs, activeTabId: parsed.tabs.activeTabId, renameRequestTabId: null, - tombstones: remoteTombstones, + tombstones: parsed.tombstones, } as any), meta: { skipPersist: true, source: 'cross-tab' }, }) -} - -function dispatchHydratePanesFromPersisted(store: StoreLike, raw: string) { - const parsed = parsePersistedPanesRaw(raw) - if (!parsed) return + // Hydrate panes const state = store.getState() const localActiveByTab = (state?.panes?.activePane || {}) as Record - const nextActive: Record = {} - for (const [tabId, node] of Object.entries(parsed.layouts || {})) { + for (const [tabId, node] of Object.entries(parsed.panes.layouts || {})) { const leafIds = collectPaneIdsSafe(node) if (leafIds.length === 0) continue const leafSet = new Set(leafIds) @@ -84,7 +77,7 @@ function dispatchHydratePanesFromPersisted(store: StoreLike, raw: string) { continue } - const remoteDesired = parsed.activePane?.[tabId] + const remoteDesired = parsed.panes.activePane?.[tabId] if (typeof remoteDesired === 'string' && leafSet.has(remoteDesired)) { nextActive[tabId] = remoteDesired continue @@ -95,10 +88,10 @@ function dispatchHydratePanesFromPersisted(store: StoreLike, raw: string) { store.dispatch({ ...hydratePanes({ - layouts: parsed.layouts as any, + layouts: parsed.panes.layouts as any, activePane: nextActive, - paneTitles: parsed.paneTitles, - paneTitleSetByUser: parsed.paneTitleSetByUser, + paneTitles: parsed.panes.paneTitles, + paneTitleSetByUser: parsed.panes.paneTitleSetByUser, } as any), meta: { skipPersist: true, source: 'cross-tab' }, }) @@ -156,54 +149,27 @@ function handleIncomingRaw( key: string, raw: string, previousRaw?: string, - dedupeCheck?: (key: string, raw: string) => boolean, ) { - if (key === TABS_STORAGE_KEY) { - dispatchHydrateTabsFromPersisted(store, raw) - // Also hydrate panes from localStorage to keep tabs+panes atomic. - // The writing tab's flush() wrote both synchronously, so panes data - // is already in localStorage even if the panes event hasn't arrived yet. - readAndHydratePairedKey(store, PANES_STORAGE_KEY, dedupeCheck) - } else if (key === PANES_STORAGE_KEY) { - dispatchHydratePanesFromPersisted(store, raw) - readAndHydratePairedKey(store, TABS_STORAGE_KEY, dedupeCheck) + if (key === LAYOUT_STORAGE_KEY) { + dispatchHydrateLayoutFromPersisted(store, raw) } else if (key === BROWSER_PREFERENCES_STORAGE_KEY) { dispatchHydrateBrowserPreferencesFromPersisted(store, raw, previousRaw) } } -function readAndHydratePairedKey( - store: StoreLike, - key: string, - dedupeCheck?: (key: string, raw: string) => boolean, -) { - const raw = localStorage.getItem(key) - if (typeof raw !== 'string') return - if (dedupeCheck && !dedupeCheck(key, raw)) return - if (key === TABS_STORAGE_KEY) { - dispatchHydrateTabsFromPersisted(store, raw) - } else if (key === PANES_STORAGE_KEY) { - dispatchHydratePanesFromPersisted(store, raw) - } -} - export function installCrossTabSync(store: StoreLike): () => void { if (typeof window === 'undefined') return () => {} // Storage events and BroadcastChannel can both deliver the same persisted payload. // Dedupe by exact raw value so we don't hydrate twice. const lastProcessedRawByKey = new Map() - for (const key of [TABS_STORAGE_KEY, PANES_STORAGE_KEY, BROWSER_PREFERENCES_STORAGE_KEY]) { + for (const key of [LAYOUT_STORAGE_KEY, BROWSER_PREFERENCES_STORAGE_KEY]) { const existingRaw = localStorage.getItem(key) if (typeof existingRaw === 'string') { lastProcessedRawByKey.set(key, existingRaw) } } - // tryDedupeAndMark is passed into handleIncomingRaw so that when a tabs event - // triggers a paired panes read from localStorage (or vice versa), the paired - // key also goes through dedup. This prevents the later StorageEvent for the - // paired key from re-hydrating what we already processed eagerly. const tryDedupeAndMark = (key: string, raw: string): boolean => { if (lastProcessedRawByKey.get(key) === raw) return false lastProcessedRawByKey.set(key, raw) @@ -213,7 +179,7 @@ export function installCrossTabSync(store: StoreLike): () => void { const handleIncomingRawDeduped = (key: string, raw: string) => { const previousRaw = lastProcessedRawByKey.get(key) if (!tryDedupeAndMark(key, raw)) return - handleIncomingRaw(store, key, raw, previousRaw, tryDedupeAndMark) + handleIncomingRaw(store, key, raw, previousRaw) } // Keep dedupe state in sync with local writes too. Otherwise, if we process a remote raw, @@ -221,8 +187,7 @@ export function installCrossTabSync(store: StoreLike): () => void { // could be incorrectly ignored. const unsubscribeLocal = onPersistBroadcast((msg) => { if ( - msg.key !== TABS_STORAGE_KEY - && msg.key !== PANES_STORAGE_KEY + msg.key !== LAYOUT_STORAGE_KEY && msg.key !== BROWSER_PREFERENCES_STORAGE_KEY ) { return @@ -234,8 +199,7 @@ export function installCrossTabSync(store: StoreLike): () => void { if (e.storageArea && e.storageArea !== localStorage) return const key = e.key if ( - key !== TABS_STORAGE_KEY - && key !== PANES_STORAGE_KEY + key !== LAYOUT_STORAGE_KEY && key !== BROWSER_PREFERENCES_STORAGE_KEY ) { return diff --git a/src/store/panesSlice.ts b/src/store/panesSlice.ts index 28c54563..8fc92277 100644 --- a/src/store/panesSlice.ts +++ b/src/store/panesSlice.ts @@ -4,9 +4,8 @@ import type { PanesState, PaneContent, PaneContentInput, PaneNode, PaneRefreshRe import { derivePaneTitle } from '@/lib/derivePaneTitle' import { isValidClaudeSessionId } from '@/lib/claude-session-id' import { buildPaneRefreshTarget, paneRefreshTargetMatchesContent } from '@/lib/pane-utils' -import { loadPersistedPanes } from './persistMiddleware.js' +import { loadPersistedPanes, loadPersistedTabs } from './persistMiddleware.js' import { hasPaneTreeShape, isWellFormedPaneTree } from './paneTreeValidation.js' -import { TABS_STORAGE_KEY } from './storage-keys' import { createLogger } from '@/lib/client-logger' @@ -105,10 +104,9 @@ function normalizePaneContent( */ function cleanOrphanedLayouts(state: PanesState): PanesState { try { - const rawTabs = localStorage.getItem(TABS_STORAGE_KEY) - if (!rawTabs) return state - const parsedTabs = JSON.parse(rawTabs) - const tabs = parsedTabs?.tabs?.tabs + const persistedTabs = loadPersistedTabs() + if (!persistedTabs) return state + const tabs = persistedTabs?.tabs?.tabs if (!Array.isArray(tabs)) return state const tabIds = new Set(tabs.map((t: any) => t?.id).filter(Boolean)) diff --git a/src/store/persistMiddleware.ts b/src/store/persistMiddleware.ts index db2e1057..ad7d8037 100644 --- a/src/store/persistMiddleware.ts +++ b/src/store/persistMiddleware.ts @@ -5,8 +5,8 @@ import type { Tab } from './types' import { nanoid } from 'nanoid' import { broadcastPersistedRaw } from './persistBroadcast' import { isWellFormedPaneTree } from './paneTreeValidation.js' -import { PANES_SCHEMA_VERSION } from './persistedState.js' -import { PANES_STORAGE_KEY, TABS_STORAGE_KEY } from './storage-keys' +import { PANES_SCHEMA_VERSION, LAYOUT_SCHEMA_VERSION, parsePersistedLayoutRaw, parsePersistedTabsRaw, parsePersistedPanesRaw } from './persistedState.js' +import { LAYOUT_STORAGE_KEY, PANES_STORAGE_KEY, TABS_STORAGE_KEY } from './storage-keys' import { createLogger } from '@/lib/client-logger' @@ -67,15 +67,56 @@ export function resetPersistedPanesCacheForTests() { cachedPersistedPanes = undefined } -export function loadPersistedTabs(): any | null { +import { migrateV2ToV3 } from './persistedState.js' + +let cachedPersistedLayout: { tabs: any; panes: any; tombstones: any } | null | undefined + +/** + * Load the combined layout from v3 key, or migrate from v2 keys. + * Cached so both tabs and panes loading see the same data. + */ +export function loadPersistedLayout(): typeof cachedPersistedLayout { + if (cachedPersistedLayout !== undefined) return cachedPersistedLayout + try { - const raw = localStorage.getItem(TABS_STORAGE_KEY) - if (!raw) return null - const parsed = JSON.parse(raw) - return parsed + const raw = localStorage.getItem(LAYOUT_STORAGE_KEY) + if (raw) { + const layoutParsed = parsePersistedLayoutRaw(raw) + if (layoutParsed) { + cachedPersistedLayout = { + tabs: { tabs: layoutParsed.tabs }, + panes: layoutParsed.panes, + tombstones: layoutParsed.tombstones, + } + return cachedPersistedLayout + } + } + + // Try migration from v2 keys + const migrated = migrateV2ToV3() + if (migrated) { + cachedPersistedLayout = { + tabs: { tabs: migrated.tabs }, + panes: migrated.panes, + tombstones: migrated.tombstones, + } + return cachedPersistedLayout + } } catch { - return null + // ignore } + + cachedPersistedLayout = null + return null +} + +export function resetPersistedLayoutCacheForTests() { + cachedPersistedLayout = undefined +} + +export function loadPersistedTabs(): any | null { + const layout = loadPersistedLayout() + return layout?.tabs ?? null } /** @@ -201,15 +242,32 @@ export function loadPersistedPanes(): any | null { // Memoize: legacy migrations generate nanoid() values, so both callers // (panesSlice and terminal-restore) must see the same result. if (cachedPersistedPanes !== undefined) return cachedPersistedPanes - cachedPersistedPanes = loadPersistedPanesUncached() + + // Try the combined v3 layout first + const layout = loadPersistedLayout() + if (layout?.panes) { + cachedPersistedPanes = migratePanesData(layout.panes) + return cachedPersistedPanes + } + + // Fall back to v2 panes key + cachedPersistedPanes = loadPersistedPanesFromV2Key() return cachedPersistedPanes } -function loadPersistedPanesUncached(): any | null { +function loadPersistedPanesFromV2Key(): any | null { try { const raw = localStorage.getItem(PANES_STORAGE_KEY) if (!raw) return null const parsed = JSON.parse(raw) + return migratePanesData(parsed) + } catch { + return null + } +} + +function migratePanesData(parsed: any): any | null { + try { // Check if migration needed const currentVersion = parsed.version || 1 @@ -322,36 +380,24 @@ export const persistMiddleware: Middleware<{}, PersistState> = (store) => { const state = store.getState() - if (tabsDirty) { + try { // Prune tombstones older than 1 hour const TOMBSTONE_MAX_AGE_MS = 60 * 60 * 1000 const tombstoneCutoff = Date.now() - TOMBSTONE_MAX_AGE_MS const tombstones = (state.tabs.tombstones || []).filter((t: { deletedAt: number }) => t.deletedAt > tombstoneCutoff) - const tabsPayload = { - tabs: { - // Persist only stable tab state. Keep ephemeral UI fields out of storage. - activeTabId: state.tabs.activeTabId, - tabs: state.tabs.tabs.map(stripTabVolatileFields), - }, - tombstones, - } - - try { - const raw = JSON.stringify(tabsPayload) - localStorage.setItem(TABS_STORAGE_KEY, raw) - broadcastPersistedRaw(TABS_STORAGE_KEY, raw) - } catch { - // ignore quota - } - } - - if (panesDirty) { - try { - const sanitizedLayouts: Record = {} + const sanitizedLayouts: Record = {} + if (state.panes?.layouts) { for (const [tabId, node] of Object.entries(state.panes.layouts)) { sanitizedLayouts[tabId] = stripEditorContentFromNode(node) } + } + + let persistablePanesSection: Record = { + layouts: sanitizedLayouts, + version: PANES_SCHEMA_VERSION, + } + if (state.panes) { const { renameRequestTabId: _rrt, renameRequestPaneId: _rrp, @@ -359,17 +405,28 @@ export const persistMiddleware: Middleware<{}, PersistState> = (store) => { refreshRequestsByPane: _rrbp, ...persistablePanes } = state.panes - const panesPayload = { + persistablePanesSection = { ...persistablePanes, layouts: sanitizedLayouts, version: PANES_SCHEMA_VERSION, } - const panesJson = JSON.stringify(panesPayload) - localStorage.setItem(PANES_STORAGE_KEY, panesJson) - broadcastPersistedRaw(PANES_STORAGE_KEY, panesJson) - } catch (err) { - log.error('Failed to save to localStorage:', err) } + + const layoutPayload = { + version: LAYOUT_SCHEMA_VERSION, + tabs: { + activeTabId: state.tabs.activeTabId, + tabs: state.tabs.tabs.map(stripTabVolatileFields), + }, + panes: persistablePanesSection, + tombstones, + } + + const raw = JSON.stringify(layoutPayload) + localStorage.setItem(LAYOUT_STORAGE_KEY, raw) + broadcastPersistedRaw(LAYOUT_STORAGE_KEY, raw) + } catch (err) { + log.error('Failed to save to localStorage:', err) } tabsDirty = false diff --git a/src/store/persistedState.ts b/src/store/persistedState.ts index 8efd7b10..60bada0a 100644 --- a/src/store/persistedState.ts +++ b/src/store/persistedState.ts @@ -1,6 +1,7 @@ import { z } from 'zod' +import { LAYOUT_STORAGE_KEY, TABS_STORAGE_KEY, PANES_STORAGE_KEY } from './storage-keys' -export { TABS_STORAGE_KEY, PANES_STORAGE_KEY } from './storage-keys' +export { LAYOUT_STORAGE_KEY, TABS_STORAGE_KEY, PANES_STORAGE_KEY } export const TABS_SCHEMA_VERSION = 1 export const PANES_SCHEMA_VERSION = 6 @@ -109,3 +110,106 @@ export function parsePersistedPanesRaw(raw: string): ParsedPersistedPanes | null paneTitleSetByUser: (res.data.paneTitleSetByUser || {}) as Record>, } } + +// --- Combined layout key (v3) --- + +export const LAYOUT_SCHEMA_VERSION = 3 + +const zPersistedLayoutPayload = z.object({ + version: z.number(), + tabs: zPersistedTabsState, + panes: zPersistedPanesPayload, + tombstones: z.array(zTombstone).optional(), +}).passthrough() + +export type ParsedPersistedLayout = { + version: number + tabs: z.infer + panes: ParsedPersistedPanes + tombstones: Array<{ id: string; deletedAt: number }> +} + +export function parsePersistedLayoutRaw(raw: string): ParsedPersistedLayout | null { + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch { + return null + } + + const res = zPersistedLayoutPayload.safeParse(parsed) + if (!res.success) return null + + const panes = res.data.panes + let panesVersion = typeof panes.version === 'number' ? panes.version : 1 + if (panesVersion < 1) panesVersion = 1 + + return { + version: res.data.version, + tabs: { + ...res.data.tabs, + activeTabId: res.data.tabs.activeTabId ?? null, + }, + panes: { + version: panesVersion, + layouts: (panes.layouts || {}) as Record, + activePane: (panes.activePane || {}) as Record, + paneTitles: (panes.paneTitles || {}) as Record>, + paneTitleSetByUser: (panes.paneTitleSetByUser || {}) as Record>, + }, + tombstones: res.data.tombstones || [], + } +} + +/** + * Migrate from separate v2 tabs+panes keys to the combined v3 layout key. + * Returns the parsed v3 layout, or null if no v2 data existed. + */ +export function migrateV2ToV3(): ParsedPersistedLayout | null { + const tabsKey = TABS_STORAGE_KEY + const panesKey = PANES_STORAGE_KEY + const layoutKey = LAYOUT_STORAGE_KEY + + const tabsRaw = localStorage.getItem(tabsKey) + if (!tabsRaw) return null + + const tabsParsed = parsePersistedTabsRaw(tabsRaw) + if (!tabsParsed) return null + + const panesRaw = localStorage.getItem(panesKey) + const panesParsed = panesRaw ? parsePersistedPanesRaw(panesRaw) : null + + const emptyPanes: ParsedPersistedPanes = { + version: PANES_SCHEMA_VERSION, + layouts: {}, + activePane: {}, + paneTitles: {}, + paneTitleSetByUser: {}, + } + + const layout: ParsedPersistedLayout = { + version: LAYOUT_SCHEMA_VERSION, + tabs: tabsParsed.tabs, + panes: panesParsed || emptyPanes, + tombstones: tabsParsed.tombstones, + } + + // Write v3 key and clean up v2 keys + const v3Payload = { + version: LAYOUT_SCHEMA_VERSION, + tabs: layout.tabs, + panes: { + version: layout.panes.version, + layouts: layout.panes.layouts, + activePane: layout.panes.activePane, + paneTitles: layout.panes.paneTitles, + paneTitleSetByUser: layout.panes.paneTitleSetByUser, + }, + tombstones: layout.tombstones, + } + localStorage.setItem(layoutKey, JSON.stringify(v3Payload)) + localStorage.removeItem(tabsKey) + localStorage.removeItem(panesKey) + + return layout +} diff --git a/src/store/storage-keys.ts b/src/store/storage-keys.ts index 6350716e..63462fde 100644 --- a/src/store/storage-keys.ts +++ b/src/store/storage-keys.ts @@ -1,4 +1,5 @@ export const STORAGE_KEYS = { + layout: 'freshell.layout.v3', tabs: 'freshell.tabs.v2', panes: 'freshell.panes.v2', sessionActivity: 'freshell.sessionActivity.v2', @@ -12,6 +13,7 @@ export const STORAGE_KEYS = { deviceDismissed: 'freshell.device-dismissed.v1', } as const +export const LAYOUT_STORAGE_KEY = STORAGE_KEYS.layout export const TABS_STORAGE_KEY = STORAGE_KEYS.tabs export const PANES_STORAGE_KEY = STORAGE_KEYS.panes export const SESSION_ACTIVITY_STORAGE_KEY = STORAGE_KEYS.sessionActivity diff --git a/src/store/tabsSlice.ts b/src/store/tabsSlice.ts index 93a08104..178f064f 100644 --- a/src/store/tabsSlice.ts +++ b/src/store/tabsSlice.ts @@ -17,7 +17,7 @@ import { } from '@/lib/tab-registry-snapshot' import { UNKNOWN_SERVER_INSTANCE_ID } from './tabRegistryConstants' import type { RootState } from './store' -import { TABS_STORAGE_KEY } from './storage-keys' +import { loadPersistedLayout } from './persistMiddleware' import { createLogger } from '@/lib/client-logger' import { mergeSessionMetadataByKey } from '@/lib/session-metadata' @@ -62,24 +62,23 @@ function loadInitialTabsState(): TabsState { } try { - const raw = localStorage.getItem(TABS_STORAGE_KEY) - if (!raw) return defaultState - const parsed = JSON.parse(raw) - // The persisted format is { tabs: TabsState } - const tabsState = parsed?.tabs as Partial | undefined + const layout = loadPersistedLayout() + if (!layout) return defaultState + + const tabsState = layout.tabs?.tabs as Partial | undefined if (!Array.isArray(tabsState?.tabs)) return defaultState - log.debug('Loaded initial state from localStorage:', tabsState.tabs.map((t) => t.id)) + log.debug('Loaded initial state from localStorage:', tabsState.tabs.map((t: Tab) => t.id)) const mappedTabs = tabsState.tabs.map(migrateTabFields) const desired = tabsState.activeTabId - const has = desired && mappedTabs.some((t) => t.id === desired) + const has = desired && mappedTabs.some((t: Tab) => t.id === desired) return { tabs: mappedTabs, activeTabId: has ? desired! : (mappedTabs[0]?.id ?? null), renameRequestTabId: null, - tombstones: Array.isArray((parsed as any)?.tombstones) ? (parsed as any).tombstones : [], + tombstones: Array.isArray(layout.tombstones) ? layout.tombstones : [], } } catch (err) { log.error('Failed to load from localStorage:', err) diff --git a/test/unit/client/store/atomicPersistence.test.ts b/test/unit/client/store/atomicPersistence.test.ts new file mode 100644 index 00000000..041654bd --- /dev/null +++ b/test/unit/client/store/atomicPersistence.test.ts @@ -0,0 +1,114 @@ +// Tests for atomic tabs+panes persistence via freshell.layout.v3 +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { LAYOUT_STORAGE_KEY, TABS_STORAGE_KEY, PANES_STORAGE_KEY } from '@/store/storage-keys' +import { parsePersistedLayoutRaw, migrateV2ToV3 } from '@/store/persistedState' + +describe('atomic persistence', () => { + beforeEach(() => { + localStorage.clear() + }) + + describe('LAYOUT_STORAGE_KEY', () => { + it('is freshell.layout.v3', () => { + expect(LAYOUT_STORAGE_KEY).toBe('freshell.layout.v3') + }) + }) + + describe('parsePersistedLayoutRaw', () => { + it('parses a valid v3 layout payload', () => { + const payload = { + version: 3, + tabs: { + activeTabId: 'tab-1', + tabs: [{ id: 'tab-1', title: 'Tab 1', status: 'running', mode: 'shell', createdAt: 1000 }], + }, + panes: { + layouts: { 'tab-1': { type: 'leaf', id: 'pane-1', content: { kind: 'terminal', mode: 'shell' } } }, + activePane: { 'tab-1': 'pane-1' }, + paneTitles: {}, + paneTitleSetByUser: {}, + }, + tombstones: [{ id: 'deleted-tab', deletedAt: 1000 }], + } + + const result = parsePersistedLayoutRaw(JSON.stringify(payload)) + expect(result).not.toBeNull() + expect(result!.tabs.tabs).toHaveLength(1) + expect(result!.tabs.tabs[0].id).toBe('tab-1') + expect(result!.panes.layouts).toHaveProperty('tab-1') + expect(result!.tombstones).toHaveLength(1) + expect(result!.tombstones[0].id).toBe('deleted-tab') + }) + + it('returns null for invalid JSON', () => { + expect(parsePersistedLayoutRaw('not json')).toBeNull() + }) + + it('returns null for missing tabs', () => { + expect(parsePersistedLayoutRaw(JSON.stringify({ version: 3, panes: {} }))).toBeNull() + }) + + it('defaults tombstones to empty array', () => { + const payload = { + version: 3, + tabs: { activeTabId: null, tabs: [] }, + panes: { layouts: {}, activePane: {}, paneTitles: {}, paneTitleSetByUser: {} }, + } + const result = parsePersistedLayoutRaw(JSON.stringify(payload)) + expect(result!.tombstones).toEqual([]) + }) + }) + + describe('migrateV2ToV3', () => { + it('combines v2 tabs and panes into v3 layout', () => { + const v2Tabs = JSON.stringify({ + tabs: { + activeTabId: 'tab-1', + tabs: [{ id: 'tab-1', title: 'Shell', status: 'running', mode: 'shell', createdAt: 1000 }], + }, + tombstones: [{ id: 'old-tab', deletedAt: 500 }], + }) + const v2Panes = JSON.stringify({ + version: 6, + layouts: { 'tab-1': { type: 'leaf', id: 'pane-1', content: { kind: 'terminal', mode: 'shell' } } }, + activePane: { 'tab-1': 'pane-1' }, + paneTitles: {}, + paneTitleSetByUser: {}, + }) + + localStorage.setItem(TABS_STORAGE_KEY, v2Tabs) + localStorage.setItem(PANES_STORAGE_KEY, v2Panes) + + const result = migrateV2ToV3() + expect(result).not.toBeNull() + + // Should have written the v3 key + const v3Raw = localStorage.getItem(LAYOUT_STORAGE_KEY) + expect(v3Raw).not.toBeNull() + + const v3 = JSON.parse(v3Raw!) + expect(v3.version).toBe(3) + expect(v3.tabs.tabs).toHaveLength(1) + expect(v3.panes.layouts).toHaveProperty('tab-1') + expect(v3.tombstones).toHaveLength(1) + + // Should have deleted v2 keys + expect(localStorage.getItem(TABS_STORAGE_KEY)).toBeNull() + expect(localStorage.getItem(PANES_STORAGE_KEY)).toBeNull() + }) + + it('returns null when no v2 keys exist', () => { + expect(migrateV2ToV3()).toBeNull() + }) + + it('handles missing panes key — creates empty panes', () => { + localStorage.setItem(TABS_STORAGE_KEY, JSON.stringify({ + tabs: { activeTabId: null, tabs: [] }, + })) + + const result = migrateV2ToV3() + expect(result).not.toBeNull() + expect(result!.panes.layouts).toEqual({}) + }) + }) +}) diff --git a/test/unit/client/store/crossTabSync.test.ts b/test/unit/client/store/crossTabSync.test.ts index a08ac34d..9760e7f5 100644 --- a/test/unit/client/store/crossTabSync.test.ts +++ b/test/unit/client/store/crossTabSync.test.ts @@ -12,7 +12,7 @@ import { resetBrowserPreferencesFlushListenersForTests, } from '../../../../src/store/browserPreferencesPersistence' import { broadcastPersistedRaw, resetPersistBroadcastForTests } from '../../../../src/store/persistBroadcast' -import { BROWSER_PREFERENCES_STORAGE_KEY, PANES_STORAGE_KEY, TABS_STORAGE_KEY } from '../../../../src/store/storage-keys' +import { BROWSER_PREFERENCES_STORAGE_KEY, LAYOUT_STORAGE_KEY } from '../../../../src/store/storage-keys' import { resolveLocalSettings } from '@shared/settings' describe('crossTabSync', () => { @@ -43,7 +43,7 @@ describe('crossTabSync', () => { cleanups.push(installCrossTabSync(store as any)) const remoteRaw = JSON.stringify({ - version: 1, + version: 3, tabs: { activeTabId: 't2', tabs: [ @@ -52,9 +52,11 @@ describe('crossTabSync', () => { { id: 't3', title: 'T3', createdAt: 3 }, ], }, + panes: { version: 6, layouts: {}, activePane: {}, paneTitles: {}, paneTitleSetByUser: {} }, + tombstones: [], }) - window.dispatchEvent(new StorageEvent('storage', { key: TABS_STORAGE_KEY, newValue: remoteRaw })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: remoteRaw })) expect(store.getState().tabs.tabs.map((t) => t.id)).toEqual(['t1', 't2', 't3']) expect(store.getState().tabs.activeTabId).toBe('t1') @@ -86,25 +88,29 @@ describe('crossTabSync', () => { cleanups.push(installCrossTabSync(store as any)) const remoteRaw = JSON.stringify({ - version: 4, - layouts: { - 'tab-1': { - type: 'split', - id: 'split-remote', - direction: 'horizontal', - sizes: [50, 50], - children: [ - { type: 'leaf', id: 'pane-a', content: { kind: 'terminal', mode: 'shell', createRequestId: 'req-a', status: 'running' } }, - { type: 'leaf', id: 'pane-b', content: { kind: 'browser', url: 'https://example.com', devToolsOpen: false } }, - ], + version: 3, + tabs: { activeTabId: null, tabs: [] }, + panes: { + version: 6, + layouts: { + 'tab-1': { + type: 'split', + id: 'split-remote', + direction: 'horizontal', + sizes: [50, 50], + children: [ + { type: 'leaf', id: 'pane-a', content: { kind: 'terminal', mode: 'shell', createRequestId: 'req-a', status: 'running' } }, + { type: 'leaf', id: 'pane-b', content: { kind: 'browser', url: 'https://example.com', devToolsOpen: false } }, + ], + }, }, + activePane: { 'tab-1': 'pane-b' }, + paneTitles: {}, }, - activePane: { 'tab-1': 'pane-b' }, - paneTitles: {}, - + tombstones: [], }) - window.dispatchEvent(new StorageEvent('storage', { key: PANES_STORAGE_KEY, newValue: remoteRaw })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: remoteRaw })) expect(store.getState().panes.layouts['tab-1']?.id).toBe('split-remote') expect(store.getState().panes.activePane['tab-1']).toBe('pane-a') @@ -132,12 +138,14 @@ describe('crossTabSync', () => { const cleanup = installCrossTabSync(storeLike as any) const raw = JSON.stringify({ - version: 1, + version: 3, tabs: { activeTabId: null, tabs: [{ id: 't1', title: 'T1', createdAt: 1 }] }, + panes: { version: 6, layouts: {}, activePane: {}, paneTitles: {}, paneTitleSetByUser: {} }, + tombstones: [], }) - window.dispatchEvent(new StorageEvent('storage', { key: TABS_STORAGE_KEY, newValue: raw })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: raw })) - MockBC.instance!.onmessage?.({ data: { type: 'persist', key: TABS_STORAGE_KEY, raw, sourceId: 'other' } }) + MockBC.instance!.onmessage?.({ data: { type: 'persist', key: LAYOUT_STORAGE_KEY, raw, sourceId: 'other' } }) const hydrateCalls = dispatchSpy.mock.calls .map((c) => c[0]) @@ -446,28 +454,32 @@ describe('crossTabSync', () => { // Remote state arrives WITHOUT terminalId (stale data from before creation) const remoteRaw = JSON.stringify({ - version: 4, - layouts: { - 'tab-1': { - type: 'leaf', - id: 'pane-1', - content: { - kind: 'terminal', - mode: 'shell', - createRequestId: 'req-1', - status: 'creating', - // NO terminalId + version: 3, + tabs: { activeTabId: null, tabs: [] }, + panes: { + version: 6, + layouts: { + 'tab-1': { + type: 'leaf', + id: 'pane-1', + content: { + kind: 'terminal', + mode: 'shell', + createRequestId: 'req-1', + status: 'creating', + // NO terminalId + }, }, }, + activePane: { 'tab-1': 'pane-1' }, + paneTitles: { 'tab-1': { 'pane-1': 'Remote broken title' } }, + paneTitleSetByUser: { 'tab-1': { 'pane-1': false } }, }, - activePane: { 'tab-1': 'pane-1' }, - paneTitles: { 'tab-1': { 'pane-1': 'Remote broken title' } }, - paneTitleSetByUser: { 'tab-1': { 'pane-1': false } }, - + tombstones: [], }) cleanups.push(installCrossTabSync(store as any)) - window.dispatchEvent(new StorageEvent('storage', { key: PANES_STORAGE_KEY, newValue: remoteRaw })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: remoteRaw })) // Local terminalId should be preserved const content = (store.getState().panes.layouts['tab-1'] as any).content @@ -502,28 +514,32 @@ describe('crossTabSync', () => { // Remote: stale state with old createRequestId and old terminalId const remoteRaw = JSON.stringify({ - version: 4, - layouts: { - 'tab-1': { - type: 'leaf', - id: 'pane-1', - content: { - kind: 'terminal', - mode: 'shell', - createRequestId: 'req-old', - status: 'running', - terminalId: 'stale-terminal-id', + version: 3, + tabs: { activeTabId: null, tabs: [] }, + panes: { + version: 6, + layouts: { + 'tab-1': { + type: 'leaf', + id: 'pane-1', + content: { + kind: 'terminal', + mode: 'shell', + createRequestId: 'req-old', + status: 'running', + terminalId: 'stale-terminal-id', + }, }, }, + activePane: { 'tab-1': 'pane-1' }, + paneTitles: { 'tab-1': { 'pane-1': 'Remote broken title' } }, + paneTitleSetByUser: { 'tab-1': { 'pane-1': false } }, }, - activePane: { 'tab-1': 'pane-1' }, - paneTitles: { 'tab-1': { 'pane-1': 'Remote broken title' } }, - paneTitleSetByUser: { 'tab-1': { 'pane-1': false } }, - + tombstones: [], }) cleanups.push(installCrossTabSync(store as any)) - window.dispatchEvent(new StorageEvent('storage', { key: PANES_STORAGE_KEY, newValue: remoteRaw })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: remoteRaw })) // Local reconnection state must be preserved — stale remote must not overwrite const content = (store.getState().panes.layouts['tab-1'] as any).content @@ -558,27 +574,31 @@ describe('crossTabSync', () => { // Remote: terminal has exited (no terminalId, status: exited) const remoteRaw = JSON.stringify({ - version: 4, - layouts: { - 'tab-1': { - type: 'leaf', - id: 'pane-1', - content: { - kind: 'terminal', - mode: 'shell', - createRequestId: 'req-1', - status: 'exited', - // NO terminalId — terminal exited + version: 3, + tabs: { activeTabId: null, tabs: [] }, + panes: { + version: 6, + layouts: { + 'tab-1': { + type: 'leaf', + id: 'pane-1', + content: { + kind: 'terminal', + mode: 'shell', + createRequestId: 'req-1', + status: 'exited', + // NO terminalId — terminal exited + }, }, }, + activePane: { 'tab-1': 'pane-1' }, + paneTitles: {}, }, - activePane: { 'tab-1': 'pane-1' }, - paneTitles: {}, - + tombstones: [], }) cleanups.push(installCrossTabSync(store as any)) - window.dispatchEvent(new StorageEvent('storage', { key: PANES_STORAGE_KEY, newValue: remoteRaw })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: remoteRaw })) // Exit state should propagate — local should NOT keep stale terminalId const content = (store.getState().panes.layouts['tab-1'] as any).content @@ -613,27 +633,31 @@ describe('crossTabSync', () => { // Remote state: malformed split with missing children const remoteRaw = JSON.stringify({ - version: 4, - layouts: { - 'tab-1': { - type: 'split', - id: 'split-bad', - direction: 'horizontal', - sizes: [50, 50], - // children is missing entirely — corrupted data + version: 3, + tabs: { activeTabId: null, tabs: [] }, + panes: { + version: 6, + layouts: { + 'tab-1': { + type: 'split', + id: 'split-bad', + direction: 'horizontal', + sizes: [50, 50], + // children is missing entirely — corrupted data + }, }, + activePane: { 'tab-1': 'pane-1' }, + paneTitles: { 'tab-1': { 'pane-1': 'Remote broken title' } }, + paneTitleSetByUser: { 'tab-1': { 'pane-1': false } }, }, - activePane: { 'tab-1': 'pane-1' }, - paneTitles: { 'tab-1': { 'pane-1': 'Remote broken title' } }, - paneTitleSetByUser: { 'tab-1': { 'pane-1': false } }, - + tombstones: [], }) cleanups.push(installCrossTabSync(store as any)) // Should not throw — malformed remote data is ignored and local state wins. expect(() => { - window.dispatchEvent(new StorageEvent('storage', { key: PANES_STORAGE_KEY, newValue: remoteRaw })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: remoteRaw })) }).not.toThrow() expect(store.getState().panes.layouts['tab-1']).toEqual({ @@ -677,28 +701,32 @@ describe('crossTabSync', () => { // Remote: same createRequestId but different resumeSessionId (from another tab) const remoteRaw = JSON.stringify({ - version: 4, - layouts: { - 'tab-1': { - type: 'leaf', - id: 'pane-1', - content: { - kind: 'terminal', - mode: 'claude', - createRequestId: 'req-1', - status: 'running', - terminalId: 'remote-terminal-456', - resumeSessionId: 'session-B', + version: 3, + tabs: { activeTabId: null, tabs: [] }, + panes: { + version: 6, + layouts: { + 'tab-1': { + type: 'leaf', + id: 'pane-1', + content: { + kind: 'terminal', + mode: 'claude', + createRequestId: 'req-1', + status: 'running', + terminalId: 'remote-terminal-456', + resumeSessionId: 'session-B', + }, }, }, + activePane: { 'tab-1': 'pane-1' }, + paneTitles: {}, }, - activePane: { 'tab-1': 'pane-1' }, - paneTitles: {}, - + tombstones: [], }) cleanups.push(installCrossTabSync(store as any)) - window.dispatchEvent(new StorageEvent('storage', { key: PANES_STORAGE_KEY, newValue: remoteRaw })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: remoteRaw })) // Local resumeSessionId must NOT be overwritten by remote const content = (store.getState().panes.layouts['tab-1'] as any).content @@ -718,18 +746,22 @@ describe('crossTabSync', () => { cleanups.push(installCrossTabSync(storeLike as any)) const raw1 = JSON.stringify({ - version: 1, + version: 3, tabs: { activeTabId: null, tabs: [{ id: 't1', title: 'T1', createdAt: 1 }] }, + panes: { version: 6, layouts: {}, activePane: {}, paneTitles: {}, paneTitleSetByUser: {} }, + tombstones: [], }) - window.dispatchEvent(new StorageEvent('storage', { key: TABS_STORAGE_KEY, newValue: raw1 })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: raw1 })) const raw2 = JSON.stringify({ - version: 1, + version: 3, tabs: { activeTabId: null, tabs: [{ id: 't1', title: 'T1 local change', createdAt: 1 }] }, + panes: { version: 6, layouts: {}, activePane: {}, paneTitles: {}, paneTitleSetByUser: {} }, + tombstones: [], }) - broadcastPersistedRaw(TABS_STORAGE_KEY, raw2) + broadcastPersistedRaw(LAYOUT_STORAGE_KEY, raw2) - window.dispatchEvent(new StorageEvent('storage', { key: TABS_STORAGE_KEY, newValue: raw1 })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: raw1 })) const hydrateCalls = dispatchSpy.mock.calls .map((c) => c[0]) @@ -737,16 +769,15 @@ describe('crossTabSync', () => { expect(hydrateCalls).toHaveLength(2) }) - it('hydrates paired panes when tabs event arrives first', () => { + it('hydrates both tabs and panes from a single combined layout event', () => { const store = configureStore({ reducer: { tabs: tabsReducer, panes: panesReducer }, }) - // Install sync first (captures current empty localStorage as baseline) cleanups.push(installCrossTabSync(store as any)) - const tabsRaw = JSON.stringify({ - version: 1, + const layoutRaw = JSON.stringify({ + version: 3, tabs: { activeTabId: 't1', tabs: [ @@ -754,26 +785,21 @@ describe('crossTabSync', () => { { id: 't2', title: 'T2', mode: 'shell' }, ], }, - }) - - const panesRaw = JSON.stringify({ - version: 6, - layouts: { - 't1': { type: 'leaf', id: 'p1', content: { kind: 'terminal', mode: 'shell', createRequestId: 'r1', status: 'running' } }, - 't2': { type: 'leaf', id: 'p2', content: { kind: 'terminal', mode: 'shell', createRequestId: 'r2', status: 'running' } }, + panes: { + version: 6, + layouts: { + 't1': { type: 'leaf', id: 'p1', content: { kind: 'terminal', mode: 'shell', createRequestId: 'r1', status: 'running' } }, + 't2': { type: 'leaf', id: 'p2', content: { kind: 'terminal', mode: 'shell', createRequestId: 'r2', status: 'running' } }, + }, + activePane: { 't1': 'p1', 't2': 'p2' }, + paneTitles: {}, }, - activePane: { 't1': 'p1', 't2': 'p2' }, - paneTitles: {}, + tombstones: [], }) - // Simulate flush() in another tab: both are written to localStorage - localStorage.setItem(TABS_STORAGE_KEY, tabsRaw) - localStorage.setItem(PANES_STORAGE_KEY, panesRaw) - - // Only the tabs event arrives — panes should also be hydrated from localStorage - window.dispatchEvent(new StorageEvent('storage', { key: TABS_STORAGE_KEY, newValue: tabsRaw })) + window.dispatchEvent(new StorageEvent('storage', { key: LAYOUT_STORAGE_KEY, newValue: layoutRaw })) - // Both tabs and panes should be hydrated consistently + // Both tabs and panes should be hydrated from the single combined event expect(store.getState().tabs.tabs.map((t: any) => t.id)).toEqual(['t1', 't2']) expect(store.getState().panes.layouts).toHaveProperty('t1') expect(store.getState().panes.layouts).toHaveProperty('t2') diff --git a/test/unit/client/store/panesPersistence.test.ts b/test/unit/client/store/panesPersistence.test.ts index 5d7780c6..d33ec6ad 100644 --- a/test/unit/client/store/panesPersistence.test.ts +++ b/test/unit/client/store/panesPersistence.test.ts @@ -25,6 +25,7 @@ import { persistMiddleware, resetPersistFlushListenersForTests, resetPersistedPanesCacheForTests, + resetPersistedLayoutCacheForTests, } from '../../../../src/store/persistMiddleware' import { PANES_SCHEMA_VERSION } from '../../../../src/store/persistedState' @@ -35,6 +36,7 @@ describe('Panes Persistence Integration', () => { vi.useFakeTimers() resetPersistFlushListenersForTests() resetPersistedPanesCacheForTests() + resetPersistedLayoutCacheForTests() }) afterEach(() => { @@ -74,10 +76,10 @@ describe('Panes Persistence Integration', () => { // 6. Check localStorage was updated vi.runAllTimers() - const savedPanes = localStorage.getItem('freshell.panes.v2') - expect(savedPanes).not.toBeNull() - const parsedPanes = JSON.parse(savedPanes!) - expect(parsedPanes.layouts[tabId].type).toBe('split') + const savedLayout = localStorage.getItem('freshell.layout.v3') + expect(savedLayout).not.toBeNull() + const parsedLayout = JSON.parse(savedLayout!) + expect(parsedLayout.panes.layouts[tabId].type).toBe('split') // 7. Simulate page refresh - create new store and hydrate // (Using explicit hydration to test that path still works) @@ -191,9 +193,9 @@ describe('Panes Persistence Integration', () => { // Verify state was persisted vi.runAllTimers() - const savedPanes = localStorage.getItem('freshell.panes.v2') - expect(savedPanes).not.toBeNull() - expect(JSON.parse(savedPanes!).layouts[tabId].type).toBe('split') + const savedLayout = localStorage.getItem('freshell.layout.v3') + expect(savedLayout).not.toBeNull() + expect(JSON.parse(savedLayout!).panes.layouts[tabId].type).toBe('split') // 2. Verify loadPersistedPanes returns correct data const loaded = loadPersistedPanes() @@ -228,10 +230,10 @@ describe('Panes Persistence Integration', () => { vi.runAllTimers() - const savedPanes = localStorage.getItem('freshell.panes.v2') - expect(savedPanes).not.toBeNull() - const parsedPanes = JSON.parse(savedPanes!) - const layout = parsedPanes.layouts[tabId] + const savedLayout = localStorage.getItem('freshell.layout.v3') + expect(savedLayout).not.toBeNull() + const parsedLayout = JSON.parse(savedLayout!) + const layout = parsedLayout.panes.layouts[tabId] expect(layout.content.kind).toBe('editor') expect(layout.content.content).toBe('') }) @@ -353,8 +355,8 @@ describe('Panes Persistence Integration', () => { vi.runAllTimers() - const saved = JSON.parse(localStorage.getItem('freshell.panes.v2')!) - expect(saved.refreshRequestsByPane).toBeUndefined() + const saved = JSON.parse(localStorage.getItem('freshell.layout.v3')!) + expect(saved.panes.refreshRequestsByPane).toBeUndefined() }) it('flushes pending writes on visibility change', () => { @@ -370,12 +372,12 @@ describe('Panes Persistence Integration', () => { const tabId = store.getState().tabs.tabs[0].id store.dispatch(initLayout({ tabId, content: { kind: 'terminal', mode: 'shell' } })) - expect(localStorage.getItem('freshell.panes.v2')).toBeNull() + expect(localStorage.getItem('freshell.layout.v3')).toBeNull() Object.defineProperty(document, 'visibilityState', { value: 'hidden', configurable: true }) document.dispatchEvent(new Event('visibilitychange')) - expect(localStorage.getItem('freshell.panes.v2')).not.toBeNull() + expect(localStorage.getItem('freshell.layout.v3')).not.toBeNull() }) }) @@ -383,6 +385,7 @@ describe('PaneContent migration', () => { beforeEach(() => { localStorageMock.clear() resetPersistedPanesCacheForTests() + resetPersistedLayoutCacheForTests() }) it('migrates old terminal pane content to include lifecycle fields', () => { @@ -542,6 +545,7 @@ describe('version 3 migration', () => { beforeEach(() => { localStorageMock.clear() resetPersistedPanesCacheForTests() + resetPersistedLayoutCacheForTests() }) it('adds empty paneTitles when migrating from version 2', () => { @@ -578,6 +582,7 @@ describe('loadInitialPanesState consistency', () => { beforeEach(() => { localStorageMock.clear() resetPersistedPanesCacheForTests() + resetPersistedLayoutCacheForTests() }) it('initial pane state matches loadPersistedPanes output for migrated data', async () => { @@ -618,6 +623,7 @@ describe('orphaned layout cleanup', () => { beforeEach(() => { localStorageMock.clear() resetPersistedPanesCacheForTests() + resetPersistedLayoutCacheForTests() }) it('removes pane layouts for tabs that no longer exist', async () => { @@ -660,6 +666,7 @@ describe('version 5 migration (drop claude-chat panes)', () => { beforeEach(() => { localStorageMock.clear() resetPersistedPanesCacheForTests() + resetPersistedLayoutCacheForTests() }) it('drops claude-chat leaf panes during v4→v5 migration', () => { @@ -743,6 +750,7 @@ describe('schema version consistency', () => { vi.useFakeTimers() resetPersistFlushListenersForTests() resetPersistedPanesCacheForTests() + resetPersistedLayoutCacheForTests() }) afterEach(() => { @@ -760,9 +768,9 @@ describe('schema version consistency', () => { store.dispatch(initLayout({ tabId, content: { kind: 'terminal', mode: 'shell' } })) vi.runAllTimers() - const raw = localStorage.getItem('freshell.panes.v2')! + const raw = localStorage.getItem('freshell.layout.v3')! const parsed = JSON.parse(raw) // The version written by persist middleware must match persistedState's version - expect(parsed.version).toBe(PANES_SCHEMA_VERSION) + expect(parsed.panes.version).toBe(PANES_SCHEMA_VERSION) }) }) diff --git a/test/unit/client/store/tabsPersistence.test.ts b/test/unit/client/store/tabsPersistence.test.ts index 80d1a24e..01edad19 100644 --- a/test/unit/client/store/tabsPersistence.test.ts +++ b/test/unit/client/store/tabsPersistence.test.ts @@ -36,6 +36,7 @@ function makeStore() { lastInputAt: 111, }], activeTabId: 'tab-1', + tombstones: [], }, }, }) @@ -76,7 +77,7 @@ describe('tabs persistence - skipPersist + strip volatile fields', () => { store.dispatch(updateTab({ id: 'tab-1', updates: { lastInputAt: 999 } })) vi.runAllTimers() - const raw = localStorage.getItem('freshell.tabs.v2') + const raw = localStorage.getItem('freshell.layout.v3') expect(raw).not.toBeNull() const parsed = JSON.parse(raw!) expect(parsed.tabs.tabs[0].lastInputAt).toBeUndefined() From ca5d6bddc6b50c187d0a2ee0d524428f9d0556ad Mon Sep 17 00:00:00 2001 From: Matt Leaverton Date: Mon, 30 Mar 2026 12:56:22 -0500 Subject: [PATCH 4/5] =?UTF-8?q?fix:=20session=20repair=20hot-path=20?= =?UTF-8?q?=E2=80=94=20check=20cache=20before=20queue=20wait?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When waitForSession finds a session in the queue, check the disk cache before falling through to queue.waitFor(). Sessions discovered at startup are enqueued before cache is consulted, so this avoids blocking terminal creation on queue processing when the cache already has a valid result. Co-Authored-By: Claude Opus 4.6 (1M context) --- server/session-scanner/service.ts | 15 ++++++ .../server/session-repair-service.test.ts | 53 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/server/session-scanner/service.ts b/server/session-scanner/service.ts index 16cc78cd..a5412c56 100644 --- a/server/session-scanner/service.ts +++ b/server/session-scanner/service.ts @@ -161,6 +161,21 @@ export class SessionRepairService extends EventEmitter { } if (this.queue.has(sessionId)) { + // Check disk cache before waiting for queue processing. + // Sessions discovered at startup are enqueued before cache is consulted, + // so this avoids blocking on the queue when the cache has a valid result. + const cachedPath = this.resolveCachedPath(sessionId) + if (cachedPath) { + const cached = await this.cache.get(cachedPath, { allowStaleMs: ACTIVE_CACHE_GRACE_MS }) + if (cached) { + const normalized = cached.sessionId === sessionId + ? cached + : { ...cached, sessionId } + this.queue.seedResult(sessionId, normalized) + await this.ensureSessionArtifacts(normalized) + return normalized + } + } return this.queue.waitFor(sessionId, timeoutMs) } diff --git a/test/unit/server/session-repair-service.test.ts b/test/unit/server/session-repair-service.test.ts index 5308adff..2295f0a9 100644 --- a/test/unit/server/session-repair-service.test.ts +++ b/test/unit/server/session-repair-service.test.ts @@ -320,4 +320,57 @@ describe('SessionRepairService', () => { await service.stop() } }) + + it('returns cached result immediately when session is queued but cache is valid', async () => { + const projectDir = path.join(tempDir, '.claude', 'projects', 'test-project') + await fs.mkdir(projectDir, { recursive: true }) + + const targetSessionId = 'target-session' + const targetFile = path.join(projectDir, `${targetSessionId}.jsonl`) + await fs.writeFile(targetFile, createTranscript(targetSessionId, '/tmp/target')) + + // Scanner that should never be called for the target session + const scan = vi.fn(async (filePath: string): Promise => { + await new Promise(() => {}) // block forever + throw new Error('unreachable') + }) + + const service = new SessionRepairService({ + cacheDir: tempDir, + scanner: { scan, repair: vi.fn() } as any, + }) + + // Seed the cache and session path index + const cachedResult: SessionScanResult = { + sessionId: targetSessionId, + filePath: targetFile, + status: 'healthy', + chainDepth: 5, + orphanCount: 0, + fileSize: 100, + messageCount: 10, + } + await (service as any).cache.set(targetFile, cachedResult) + + // Directly enqueue the session in the queue (simulating what discovery does) + // but DON'T start the queue, so it stays queued and never processes + const queue = (service as any).queue + queue.enqueue([{ sessionId: targetSessionId, filePath: targetFile, priority: 'disk' }]) + + // Also set the session path index (normally done by discovery) + ;(service as any).sessionPathIndex.set(targetSessionId, targetFile) + + // The session is now queued but not processed. + // waitForSession should find it via cache and return immediately. + const before = Date.now() + const result = await service.waitForSession(targetSessionId, 100) + const elapsed = Date.now() - before + + expect(result.sessionId).toBe(targetSessionId) + expect(result.status).toBe('healthy') + expect(result.chainDepth).toBe(5) + expect(elapsed).toBeLessThan(50) + // Scanner should NOT have been called — cache hit bypassed it + expect(scan).not.toHaveBeenCalled() + }) }) From c5670cdb7eb1474dbc84e5a1ed383e0961c0318a Mon Sep 17 00:00:00 2001 From: Matt Leaverton Date: Mon, 30 Mar 2026 13:36:41 -0500 Subject: [PATCH 5/5] fix: remove tab.terminalId dual source of truth Remove the legacy terminalId field from Tab type. All terminal identity now lives exclusively in pane.content.terminalId. Add pane-tree-walking selectors (selectTerminalIdsForTab, selectPrimaryTerminalIdForTab, selectTabIdByTerminalId) and replace 27 read sites across 16 source files. Migration strips legacy terminalId from persisted data on load. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/components/BackgroundSessions.tsx | 7 +- src/components/OverviewView.tsx | 34 ++- src/components/Sidebar.tsx | 5 +- src/components/TabBar.tsx | 3 +- src/components/TabContent.tsx | 18 +- src/components/TerminalView.tsx | 10 +- .../context-menu/ContextMenuProvider.tsx | 38 ++-- src/components/panes/PaneContainer.tsx | 20 +- src/lib/codex-activity-resolver.ts | 3 +- src/lib/pane-activity.ts | 7 - src/lib/session-type-utils.ts | 3 - src/lib/tab-codex-activity.ts | 8 - src/lib/ui-commands.ts | 15 +- src/store/selectors/paneTerminalSelectors.ts | 65 ++++++ src/store/tabRegistrySync.ts | 1 - src/store/tabsSlice.ts | 26 ++- src/store/types.ts | 1 - .../codex-activity-indicator-flow.test.tsx | 9 +- .../components/BackgroundSessions.test.tsx | 13 +- .../components/ContextMenuProvider.test.tsx | 5 +- test/unit/client/components/Sidebar.test.tsx | 9 +- test/unit/client/components/TabBar.test.tsx | 60 ++++-- .../client/components/TabContent.test.tsx | 41 +--- .../TerminalView.lifecycle.test.tsx | 7 +- .../lib/codex-activity-resolver.test.ts | 17 +- .../client/lib/session-type-utils.test.ts | 21 +- .../selectors/paneTerminalSelectors.test.ts | 199 ++++++++++++++++++ test/unit/client/store/tabsSlice.test.ts | 15 +- 28 files changed, 467 insertions(+), 193 deletions(-) create mode 100644 src/store/selectors/paneTerminalSelectors.ts create mode 100644 test/unit/client/store/selectors/paneTerminalSelectors.test.ts diff --git a/src/components/BackgroundSessions.tsx b/src/components/BackgroundSessions.tsx index c0732be8..657c75bd 100644 --- a/src/components/BackgroundSessions.tsx +++ b/src/components/BackgroundSessions.tsx @@ -1,9 +1,11 @@ import { useCallback, useEffect, useMemo } from 'react' +import { nanoid } from 'nanoid' import { getWsClient } from '@/lib/ws-client' import { Button } from '@/components/ui/button' import { Badge } from '@/components/ui/badge' import { useAppDispatch, useAppSelector } from '@/store/hooks' import { addTab } from '@/store/tabsSlice' +import { initLayout } from '@/store/panesSlice' import { fetchTerminalDirectoryWindow } from '@/store/terminalDirectoryThunks' type BackgroundTerminal = { @@ -97,7 +99,10 @@ export default function BackgroundSessions() { size="sm" variant="outline" onClick={() => { - dispatch(addTab({ title: t.title, terminalId: t.terminalId, status: 'running', mode: (t.mode as any) || 'shell', resumeSessionId: t.resumeSessionId })) + const tabId = nanoid() + const mode = ((t.mode as any) || 'shell') as string + dispatch(addTab({ id: tabId, title: t.title, status: 'running', mode: mode as any, resumeSessionId: t.resumeSessionId })) + dispatch(initLayout({ tabId, content: { kind: 'terminal', mode: mode as any, terminalId: t.terminalId, status: 'running', resumeSessionId: t.resumeSessionId } })) }} > Attach diff --git a/src/components/OverviewView.tsx b/src/components/OverviewView.tsx index 4f06400b..00ab071b 100644 --- a/src/components/OverviewView.tsx +++ b/src/components/OverviewView.tsx @@ -1,8 +1,11 @@ -import { useEffect, useMemo, useState } from 'react' +import { useCallback, useEffect, useMemo, useState } from 'react' +import { nanoid } from 'nanoid' import { api } from '@/lib/api' import { useAppDispatch, useAppSelector } from '@/store/hooks' import { addTab, setActiveTab, updateTab } from '@/store/tabsSlice' +import { initLayout } from '@/store/panesSlice' import { getWsClient } from '@/lib/ws-client' +import { collectTerminalIds } from '@/lib/pane-utils' import { cn } from '@/lib/utils' import { RefreshCw, Circle, Play, Pencil, Trash2, Sparkles, ExternalLink } from 'lucide-react' import { ContextIds } from '@/components/context-menu/context-menu-constants' @@ -44,9 +47,20 @@ function formatDuration(ms: number): string { export default function OverviewView({ onOpenTab }: { onOpenTab?: () => void }) { const dispatch = useAppDispatch() const tabs = useAppSelector((s) => s.tabs.tabs) + const paneLayouts = useAppSelector((s) => s.panes.layouts) const ws = useMemo(() => getWsClient(), []) + const findTabByTerminalId = useCallback((terminalId: string) => { + for (const tab of tabs) { + const layout = paneLayouts[tab.id] + if (layout && collectTerminalIds(layout).includes(terminalId)) { + return tab + } + } + return undefined + }, [tabs, paneLayouts]) + const [items, setItems] = useState([]) const [loading, setLoading] = useState(false) const [error, setError] = useState(null) @@ -137,15 +151,17 @@ export default function OverviewView({ onOpenTab }: { onOpenTab?: () => void }) x.terminalId === t.terminalId)} + isOpen={!!findTabByTerminalId(t.terminalId)} onOpen={() => { - const existing = tabs.find((x) => x.terminalId === t.terminalId) + const existing = findTabByTerminalId(t.terminalId) if (existing) { dispatch(setActiveTab(existing.id)) onOpenTab?.() return } - dispatch(addTab({ title: t.title, terminalId: t.terminalId, status: 'running', mode: 'shell' })) + const tabId = nanoid() + dispatch(addTab({ id: tabId, title: t.title, status: 'running', mode: 'shell' })) + dispatch(initLayout({ tabId, content: { kind: 'terminal', mode: 'shell', terminalId: t.terminalId, status: 'running' } })) onOpenTab?.() }} onRename={async (title, description) => { @@ -153,7 +169,7 @@ export default function OverviewView({ onOpenTab }: { onOpenTab?: () => void }) titleOverride: title || undefined, descriptionOverride: description || undefined, }) - const existing = tabs.find((x) => x.terminalId === t.terminalId) + const existing = findTabByTerminalId(t.terminalId) if (existing && title) { dispatch(updateTab({ id: existing.id, updates: { title } })) } @@ -192,15 +208,17 @@ export default function OverviewView({ onOpenTab }: { onOpenTab?: () => void }) x.terminalId === t.terminalId)} + isOpen={!!findTabByTerminalId(t.terminalId)} onOpen={() => { - const existing = tabs.find((x) => x.terminalId === t.terminalId) + const existing = findTabByTerminalId(t.terminalId) if (existing) { dispatch(setActiveTab(existing.id)) onOpenTab?.() return } - dispatch(addTab({ title: t.title, terminalId: t.terminalId, status: 'exited', mode: 'shell' })) + const tabId = nanoid() + dispatch(addTab({ id: tabId, title: t.title, status: 'exited', mode: 'shell' })) + dispatch(initLayout({ tabId, content: { kind: 'terminal', mode: 'shell', terminalId: t.terminalId, status: 'exited' } })) onOpenTab?.() }} onRename={async (title, description) => { diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx index 239ad382..9a3dbcfc 100644 --- a/src/components/Sidebar.tsx +++ b/src/components/Sidebar.tsx @@ -19,6 +19,7 @@ import { getInstalledPerfAuditBridge } from '@/lib/perf-audit-bridge' import { fetchSessionWindow } from '@/store/sessionsThunks' import { mergeSessionMetadataByKey } from '@/lib/session-metadata' import { collectBusySessionKeys } from '@/lib/pane-activity' +import { selectPrimaryTerminalIdForTab } from '@/store/selectors/paneTerminalSelectors' import type { ChatSessionState } from '@/store/agentChatTypes' import type { PaneRuntimeActivityRecord } from '@/store/paneRuntimeActivitySlice' @@ -337,7 +338,6 @@ export default function Sidebar({ sessionType, sessionId: item.sessionId, cwd: item.cwd, - terminalId: runningTerminalId, agentChatProviderSettings: providerSettings, }), })) @@ -370,9 +370,8 @@ export default function Sidebar({ { id: 'settings' as const, label: 'Settings', icon: Settings, shortcut: ',' }, ] - const activeTab = tabs.find((t) => t.id === activeTabId) const activeSessionKey = activeSessionKeyFromPanes - const activeTerminalId = activeTab?.terminalId + const activeTerminalId = useAppSelector((s) => activeTabId ? selectPrimaryTerminalIdForTab(s, activeTabId) : undefined) const activeSearchTier = sidebarWindow?.searchTier ?? searchTier const hasLoadedSidebarWindow = typeof sidebarWindow?.lastLoadedAt === 'number' const sidebarWindowHasItems = (sidebarWindow?.projects ?? []).some((project) => (project.sessions?.length ?? 0) > 0) diff --git a/src/components/TabBar.tsx b/src/components/TabBar.tsx index 63e54458..a90ac573 100644 --- a/src/components/TabBar.tsx +++ b/src/components/TabBar.tsx @@ -185,7 +185,6 @@ export default function TabBar({ sidebarCollapsed, onToggleSidebar }: TabBarProp shell: tab.shell, createRequestId: tab.createRequestId, status: tab.status, - terminalId: tab.terminalId, resumeSessionId: tab.resumeSessionId, initialCwd: tab.initialCwd, }, @@ -202,7 +201,7 @@ export default function TabBar({ sidebarCollapsed, onToggleSidebar }: TabBarProp return Array.from(new Set(ids)) } } - return tab.terminalId ? [tab.terminalId] : [] + return [] }, [paneLayouts]) const getBusyPaneIds = useCallback((tab: Tab): string[] => getBusyPaneIdsForTab({ diff --git a/src/components/TabContent.tsx b/src/components/TabContent.tsx index fd941817..4ea91e1b 100644 --- a/src/components/TabContent.tsx +++ b/src/components/TabContent.tsx @@ -15,6 +15,7 @@ interface TabContentProps { export default function TabContent({ tabId, hidden }: TabContentProps) { const tab = useAppSelector((s) => s.tabs.tabs.find((t) => t.id === tabId)) + const hasLayout = useAppSelector((s) => !!s.panes.layouts[tabId]) const defaultNewPane = useAppSelector((s) => s.settings.settings.panes?.defaultNewPane || 'ask') const previousHiddenRef = useRef(hidden) @@ -27,8 +28,8 @@ export default function TabContent({ tabId, hidden }: TabContentProps) { if (!tab) return null - // For coding CLI session views with no terminal, use SessionView - if (tab.codingCliSessionId && !tab.terminalId) { + // For coding CLI session views with no terminal pane, use SessionView + if (tab.codingCliSessionId && !hasLayout) { return