diff --git a/src/main/persistence/closed-terminal-tab-write-fence.test.ts b/src/main/persistence/closed-terminal-tab-write-fence.test.ts new file mode 100644 index 00000000000..dc2fdc8db43 --- /dev/null +++ b/src/main/persistence/closed-terminal-tab-write-fence.test.ts @@ -0,0 +1,186 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from '../../shared/constants' +import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' +import { Store } from './loading-store/store' +import { closeTestStores, createSqliteTestStore } from '../persistence-test-harness' +import { OrcaRuntimeService } from '../runtime/orca-runtime' +import { + REPO_ID, + TAB_ID, + WORKTREE_ID, + WORKTREE_PATH, + makeSession +} from '../runtime/__fixtures__/orca-runtime-terminal-close-continuity-fixtures' +import { advanceTerminalTopologyRevision } from './terminal-topology/terminal-topology-membership' + +const SSH_HOST_ID = 'ssh:target-1' +const SSH_WORKTREE_ID = 'ssh-repo::/srv/app' +const OTHER_TAB_ID = '7a1b2c3d-4e5f-4a6b-8c7d-9e0f1a2b3c4d' + +const directories: string[] = [] +afterEach(async () => { + await closeTestStores() + for (const directory of directories.splice(0)) { + rmSync(directory, { recursive: true, force: true }) + } +}) + +function createPersistedStore() { + const directory = mkdtempSync(join(tmpdir(), 'orca-closed-tab-fence-')) + directories.push(directory) + const dataFile = join(directory, 'orca-data.json') + const store = createSqliteTestStore(Store, { dataFile }) + store.addRepo({ + id: REPO_ID, + path: WORKTREE_PATH, + displayName: 'L', + badgeColor: 'gray', + addedAt: 1 + }) + store.addRepo({ + id: 'ssh-repo', + path: '/srv/app', + displayName: 'R', + badgeColor: 'gray', + addedAt: 1, + connectionId: 'target-1' + }) + return { + store, + runtime: new OrcaRuntimeService(store), + reload: async () => { + store.flush() + store.freezeWrites() + await store.waitForPendingWrite() + return createSqliteTestStore(Store, { dataFile }) + } + } +} + +/** A session holding TAB_ID (and OTHER_TAB_ID) for one worktree, with its unified tab and group. */ +function sessionFor(worktreeId: string): WorkspaceSessionState { + const base = makeSession() + const rows = base.tabsByWorktree[WORKTREE_ID]!.map((tab) => ({ ...tab, worktreeId })) + const unified = (id: string, sortOrder: number) => ({ + id, + entityId: id, + groupId: 'group', + worktreeId, + contentType: 'terminal' as const, + label: id, + customLabel: null, + color: null, + sortOrder, + createdAt: 1 + }) + return { + ...base, + tabsByWorktree: { [worktreeId]: [...rows, { ...rows[0]!, id: OTHER_TAB_ID, sortOrder: 1 }] }, + unifiedTabs: { [worktreeId]: [unified(TAB_ID, 0), unified(OTHER_TAB_ID, 1)] }, + tabGroups: { + [worktreeId]: [ + { id: 'group', worktreeId, activeTabId: TAB_ID, tabOrder: [TAB_ID, OTHER_TAB_ID] } + ] + }, + activeTabIdByWorktree: { [worktreeId]: TAB_ID } + } +} + +describe('a write cannot bring back a tab its partition recorded as closed (#12447)', () => { + // The close lands before the renderer save that first lists the tab, so no topology revision + // fences that save: only the close record can refuse it. + it.each([ + ['local', undefined, WORKTREE_ID], + ['SSH', SSH_HOST_ID, SSH_WORKTREE_ID] + ] as const)( + 'drops a closed tab a late %s renderer save still lists', + async (_label, hostId, worktreeId) => { + const { store, runtime, reload } = createPersistedStore() + store.setWorkspaceSession( + { ...getDefaultWorkspaceSession(), tabsByWorktree: { [worktreeId]: [] } }, + hostId + ) + + await runtime.closeTerminalSurfaceFromRenderer({ + worktreeId, + target: { kind: 'tab', tabId: TAB_ID } + }) + store.setWorkspaceSession(sessionFor(worktreeId), hostId) + + const after = (await reload()).getWorkspaceSession(hostId) + expect(after.tabsByWorktree[worktreeId]?.map((tab) => tab.id)).toEqual([OTHER_TAB_ID]) + expect(after.terminalLayoutsByTabId[TAB_ID]).toBeUndefined() + expect(after.unifiedTabs?.[worktreeId]?.map((tab) => tab.id)).toEqual([OTHER_TAB_ID]) + expect(after.tabGroups?.[worktreeId]?.[0]?.tabOrder).toEqual([OTHER_TAB_ID]) + expect(after.closedTerminalTabTombstonesByTabId?.[TAB_ID]).toBeDefined() + } + ) + + it('drops the closed tab from a unified-tabs-only patch', async () => { + const { store, runtime } = createPersistedStore() + const before = advanceTerminalTopologyRevision(sessionFor(WORKTREE_ID), WORKTREE_ID) + store.setWorkspaceSession(before) + await runtime.closeTerminalSurfaceFromRenderer({ + worktreeId: WORKTREE_ID, + target: { kind: 'tab', tabId: TAB_ID } + }) + + store.patchWorkspaceSession({ unifiedTabs: before.unifiedTabs }) + + expect(store.getWorkspaceSession().unifiedTabs?.[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([ + OTHER_TAB_ID + ]) + }) + + it('keeps a same-id tab under another worktree than the one the record names', async () => { + const { store } = createPersistedStore() + const session = sessionFor(WORKTREE_ID) + store.setWorkspaceSession({ + ...session, + closedTerminalTabTombstonesByTabId: { + [TAB_ID]: { closedAt: Date.now(), worktreeId: 'repo::/elsewhere', reason: 'user' } + } + }) + + expect(store.getWorkspaceSession().tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([ + TAB_ID, + OTHER_TAB_ID + ]) + }) + + // A host slice with no terminal rows omits the terminal maps altogether. + it.each(['set', 'stage-before-unload'] as const)( + 'accepts a browser-only host slice once the partition has a close record (%s)', + async (write) => { + const { store, runtime } = createPersistedStore() + store.setWorkspaceSession( + { ...getDefaultWorkspaceSession(), tabsByWorktree: { [SSH_WORKTREE_ID]: [] } }, + SSH_HOST_ID + ) + await runtime.closeTerminalSurfaceFromRenderer({ + worktreeId: SSH_WORKTREE_ID, + target: { kind: 'tab', tabId: TAB_ID } + }) + const { + tabsByWorktree: _omitted, + terminalLayoutsByTabId: _alsoOmitted, + ...slice + } = { ...getDefaultWorkspaceSession(), activeWorktreeId: SSH_WORKTREE_ID } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: models the renderer's host split, which omits empty terminal maps from its IPC payload. + const sparse = slice as WorkspaceSessionState + + if (write === 'set') { + store.setWorkspaceSession(sparse, SSH_HOST_ID) + } else { + store.stageWorkspaceSessionBeforeUnload(sparse, SSH_HOST_ID) + } + + const after = store.getWorkspaceSession(SSH_HOST_ID) + expect(after.activeWorktreeId).toBe(SSH_WORKTREE_ID) + expect(after.closedTerminalTabTombstonesByTabId?.[TAB_ID]).toBeDefined() + } + ) +}) diff --git a/src/main/persistence/closed-terminal-tab-write-fence.ts b/src/main/persistence/closed-terminal-tab-write-fence.ts new file mode 100644 index 00000000000..51419541f45 --- /dev/null +++ b/src/main/persistence/closed-terminal-tab-write-fence.ts @@ -0,0 +1,39 @@ +import { hasClosedTerminalTabRecord } from '../../shared/closed-terminal-tab-tombstones' +import { + closeTerminalTabInWorkspaceSession, + workspaceSessionListsTerminalTab +} from '../../shared/workspace-session-terminal-tab-close' +import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' + +/** + * Removes every terminal tab this partition's own live close records name, from whichever writer + * sent it back (a late renderer save, a relay snapshot, a fast patch). Tab ids are uuids, so a + * recorded id is never a new tab; scoped to the record's worktree, and records past the TTL stop + * suppressing. Main's rollback path bypasses this: it restores rows a failed close never committed. + */ +export function dropClosedTerminalTabs( + session: WorkspaceSessionState, + now = Date.now() +): WorkspaceSessionState { + const records = session.closedTerminalTabTombstonesByTabId + if (!records) { + return session + } + // Why: a host slice omits terminal maps when that host has no terminal rows. + const withMaps = { + ...session, + tabsByWorktree: session.tabsByWorktree ?? {}, + terminalLayoutsByTabId: session.terminalLayoutsByTabId ?? {} + } + let next: WorkspaceSessionState = withMaps + for (const [tabId, { worktreeId }] of Object.entries(records)) { + if ( + hasClosedTerminalTabRecord(records, tabId, undefined, now) && + workspaceSessionListsTerminalTab(next, worktreeId, tabId) + ) { + next = closeTerminalTabInWorkspaceSession(next, worktreeId, tabId, { force: true }).session + } + } + // Leaves a sparse slice as sent when nothing was dropped. + return next === withMaps ? session : next +} diff --git a/src/main/persistence/loading-store/session-host-partitions.ts b/src/main/persistence/loading-store/session-host-partitions.ts index 942e5060454..a446ae30576 100644 --- a/src/main/persistence/loading-store/session-host-partitions.ts +++ b/src/main/persistence/loading-store/session-host-partitions.ts @@ -13,6 +13,7 @@ import { withoutRedundantGlobalFields } from '../../../shared/workspace-session- import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id' import { readTerminalScrollbackSnapshotSync } from '../../terminal-scrollback-snapshots' import { preserveRuntimeAuthoredWorkspaceSessionFields } from '../runtime-authored-workspace-session-fields' +import { dropClosedTerminalTabs } from '../closed-terminal-tab-write-fence' import { findWorktreeIdForTab } from '../restoring-sessions/pane-identity-migration' import { invalidateLocalWorktreeMetadataPruneInputs } from '../../local-worktree-metadata-prune-gate' import { @@ -197,7 +198,7 @@ export function setHostWorkspaceSession( owner[sessionHostPartitionOperationsContext].runtime.state.workspaceSessionsByHostId?.[hostId] // Why here and not at the callers: the before-unload stage path writes the renderer's payload // straight through, so a per-caller guard leaves the quit write erasing runtime-authored rows. - session = preserveRuntimeAuthoredWorkspaceSessionFields(session, prior) + session = dropClosedTerminalTabs(preserveRuntimeAuthoredWorkspaceSessionFields(session, prior)) // Why: each partition owns its topology fence; renderer writes omit it and must rebase locally. session = sanitizeWorkspaceSessionTerminalRetirements(session, prior) session = preserveMissingWorkspaceSessionTerminalBindings( diff --git a/src/main/persistence/loading-store/session-snapshot-operations.ts b/src/main/persistence/loading-store/session-snapshot-operations.ts index fe63e0437f2..ed5114c9905 100644 --- a/src/main/persistence/loading-store/session-snapshot-operations.ts +++ b/src/main/persistence/loading-store/session-snapshot-operations.ts @@ -18,6 +18,7 @@ import { setHostWorkspaceSession, withRequiredWorkspaceSessionMaps } from './session-host-partitions' +import { dropClosedTerminalTabs } from '../closed-terminal-tab-write-fence' import { scheduleSave } from './write-scheduling' type SessionSnapshotOperationsRuntime = Pick< @@ -96,6 +97,10 @@ export class SessionSnapshotOperations { if (Object.hasOwn(patch, 'browserUrlHistory')) { next = pruneWorkspaceSessionBrowserHistory(next) } + // Why: a unified-tabs patch skips full normalization, so it would bypass the close fence. + if (Object.hasOwn(patch, 'unifiedTabs')) { + next = dropClosedTerminalTabs(next) + } this.publishSession(next, resolved) } diff --git a/src/main/persistence/loading-store/workspace-session-snapshot-publication.ts b/src/main/persistence/loading-store/workspace-session-snapshot-publication.ts index 178f1d833ff..b410112e22d 100644 --- a/src/main/persistence/loading-store/workspace-session-snapshot-publication.ts +++ b/src/main/persistence/loading-store/workspace-session-snapshot-publication.ts @@ -11,6 +11,7 @@ import { migrateWorkspaceSessionTerminalScrollbackSnapshotsAsync } from '../../terminal-scrollback-snapshot-async-migration' import { preserveRuntimeAuthoredWorkspaceSessionFields } from '../runtime-authored-workspace-session-fields' +import { dropClosedTerminalTabs } from '../closed-terminal-tab-write-fence' import { registerPersistedPaneKeyAlias } from '../restoring-sessions/pane-alias-normalization' import { normalizeWorkspaceSessionPaneIdentities, @@ -37,7 +38,7 @@ export function setLocalWorkspaceSession( const prior = context.runtime.state.workspaceSession // Why here and not at the callers: the before-unload stage path writes the renderer's payload // straight through, so a per-caller guard leaves the quit write erasing runtime-authored rows. - session = preserveRuntimeAuthoredWorkspaceSessionFields(session, prior) + session = dropClosedTerminalTabs(preserveRuntimeAuthoredWorkspaceSessionFields(session, prior)) session = sanitizeWorkspaceSessionTerminalRetirements(session, prior) session = pruneWorkspaceSessionBrowserHistory( pruneLocalTerminalScrollbackBuffers(session, context.runtime.state.repos) diff --git a/src/main/runtime/orca-runtime-terminal-close-records.test.ts b/src/main/runtime/orca-runtime-terminal-close-records.test.ts index 28bb3e62d2e..c5d54c9e428 100644 --- a/src/main/runtime/orca-runtime-terminal-close-records.test.ts +++ b/src/main/runtime/orca-runtime-terminal-close-records.test.ts @@ -141,25 +141,17 @@ describe('close records', () => { ) }) - it('lets a close that removes a listed tab replace a record that tab already had', async () => { + // A write cannot list a tab its partition recorded as closed, so legacy rows beside a record go. + it('drops a listed tab its partition already recorded as closed, and keeps the record', async () => { const earlier = Date.now() - 24 * 60 * 60 * 1000 - const { store, runtime } = createPersistedRuntime({ + const record = { closedAt: earlier, worktreeId: WORKTREE_ID, reason: 'cleanup' as const } + const { store } = createPersistedRuntime({ ...makeSession(), - closedTerminalTabTombstonesByTabId: { - [TAB_ID]: { closedAt: earlier, worktreeId: WORKTREE_ID, reason: 'cleanup' } - } + closedTerminalTabTombstonesByTabId: { [TAB_ID]: record } }) - await runtime.closeTerminalSurfaceFromRenderer({ - worktreeId: WORKTREE_ID, - target: { kind: 'tab', tabId: TAB_ID }, - reason: 'user' - }) - - const record = store.getWorkspaceSession().closedTerminalTabTombstonesByTabId?.[TAB_ID] expect(store.getWorkspaceSession().tabsByWorktree[WORKTREE_ID]).toEqual([]) - expect(record?.reason).toBe('user') - expect(record?.closedAt).toBeGreaterThan(earlier) + expect(store.getWorkspaceSession().closedTerminalTabTombstonesByTabId?.[TAB_ID]).toEqual(record) }) it('admits a late spawn for a tab whose close record is past the TTL', async () => {