diff --git a/src/main/persistence-pty-binding-leaf-membership-index.test.ts b/src/main/persistence-pty-binding-leaf-tab-resolution.test.ts similarity index 94% rename from src/main/persistence-pty-binding-leaf-membership-index.test.ts rename to src/main/persistence-pty-binding-leaf-tab-resolution.test.ts index 18a88afdf85..2f0dc41daaf 100644 --- a/src/main/persistence-pty-binding-leaf-membership-index.test.ts +++ b/src/main/persistence-pty-binding-leaf-tab-resolution.test.ts @@ -24,7 +24,8 @@ describe('findTerminalTabIdForLeaf after persistPtyBinding grafts a leaf', () => }) // `persistPtyBinding` grafts the leaf by assigning `layout.root` on the SAME layout object inside - // the SAME layouts record (pty-binding-persistence.ts). Any membership cache must notice that. + // the SAME layouts record (pty-binding-persistence.ts), so the resolver has to answer from the + // tree that is there now, not from anything derived on an earlier call. it('resolves a leaf grafted in place by a split spawn', async () => { const store = await createStore() store.setWorkspaceSession({ diff --git a/src/main/runtime/terminal-leaf-membership-index.test.ts b/src/main/runtime/terminal-leaf-membership-index.test.ts deleted file mode 100644 index 735960f2f11..00000000000 --- a/src/main/runtime/terminal-leaf-membership-index.test.ts +++ /dev/null @@ -1,118 +0,0 @@ -import { describe, expect, it } from 'vitest' - -import type { - TerminalLayoutSnapshot, - TerminalPaneLayoutNode -} from '../../shared/terminal-tab-types' -import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' -import { findTerminalTabIdForLeaf } from './workspace-session-terminal-membership-authority' -import { getTerminalLeafMembershipIndex } from './terminal-leaf-membership-index' - -function layout(...leafIds: string[]): TerminalLayoutSnapshot { - let root = { type: 'leaf' as const, leafId: leafIds[0] } - for (const leafId of leafIds.slice(1)) { - root = { - type: 'split', - direction: 'row', - first: root, - second: { type: 'leaf' as const, leafId } - } as never - } - return { root, activeLeafId: leafIds[0], ptyIdsByLeafId: {} } as TerminalLayoutSnapshot -} - -function session(layouts: Record): WorkspaceSessionState { - return { terminalLayoutsByTabId: layouts } as WorkspaceSessionState -} - -describe('getTerminalLeafMembershipIndex', () => { - it('maps every leaf in a split tree to its tab', () => { - const index = getTerminalLeafMembershipIndex({ - 'tab-a': layout('leaf-1', 'leaf-2', 'leaf-3'), - 'tab-b': layout('leaf-4') - }) - expect([...index]).toEqual([ - ['leaf-1', 'tab-a'], - ['leaf-2', 'tab-a'], - ['leaf-3', 'tab-a'], - ['leaf-4', 'tab-b'] - ]) - }) - - it('keeps the first tab in record order when two layouts claim one leaf', () => { - const layouts = { 'tab-a': layout('shared'), 'tab-b': layout('shared') } - expect(getTerminalLeafMembershipIndex(layouts).get('shared')).toBe('tab-a') - expect(findTerminalTabIdForLeaf(session(layouts), 'shared')).toBe('tab-a') - }) - - it('returns the same map instance for an unchanged record', () => { - const layouts = { 'tab-a': layout('leaf-1') } - expect(getTerminalLeafMembershipIndex(layouts)).toBe(getTerminalLeafMembershipIndex(layouts)) - }) - - it('rebuilds when a layout object inside the same record is replaced', () => { - const layouts: Record = { 'tab-a': layout('leaf-1') } - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBe('tab-a') - layouts['tab-a'] = layout('leaf-9') - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBeUndefined() - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-9')).toBe('tab-a') - }) - - it('rebuilds when a tab is added to or removed from the same record', () => { - const layouts: Record = { 'tab-a': layout('leaf-1') } - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBe('tab-a') - layouts['tab-b'] = layout('leaf-2') - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-2')).toBe('tab-b') - delete layouts['tab-a'] - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBeUndefined() - }) - - it('answers misses and empty sessions the same way the linear scan did', () => { - expect(findTerminalTabIdForLeaf(undefined, 'leaf-1')).toBeUndefined() - expect(findTerminalTabIdForLeaf(session({}), 'leaf-1')).toBeUndefined() - expect(findTerminalTabIdForLeaf(session({ 'tab-a': layout('leaf-1') }), 'nope')).toBeUndefined() - }) - - // `persistPtyBinding` grafts a leaf by assigning `layout.root` on the SAME layout object in the - // SAME record — a layout-identity check would keep serving an index blind to the new leaf. - it('rebuilds when a layout root is replaced in place on the same layout object', () => { - const tracked = layout('leaf-1') - const layouts: Record = { 'tab-a': tracked } - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBe('tab-a') - tracked.root = { - type: 'split', - direction: 'vertical', - first: tracked.root, - second: { type: 'leaf', leafId: 'leaf-2' } - } as never - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-2')).toBe('tab-a') - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBe('tab-a') - }) - - it('rebuilds when an empty layout gets its first root in place', () => { - const tracked = { root: null, activeLeafId: null, ptyIdsByLeafId: {} } as TerminalLayoutSnapshot - const layouts: Record = { 'tab-a': tracked } - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBeUndefined() - tracked.root = { type: 'leaf', leafId: 'leaf-1' } - expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBe('tab-a') - }) - - it('does not walk a layout tree again once the record is indexed', () => { - let nodeVisits = 0 - // Stable node object: revalidation only compares its reference, a rebuild reads `type`. - const root = { - get type() { - nodeVisits += 1 - return 'leaf' as const - }, - leafId: 'leaf-1' - } as unknown as TerminalPaneLayoutNode - const layouts = { - 'tab-a': { root, activeLeafId: 'leaf-1', ptyIdsByLeafId: {} } as TerminalLayoutSnapshot - } - for (let i = 0; i < 50; i += 1) { - findTerminalTabIdForLeaf(session(layouts), `leaf-${i}`) - } - expect(nodeVisits).toBe(1) - }) -}) diff --git a/src/main/runtime/terminal-leaf-membership-index.ts b/src/main/runtime/terminal-leaf-membership-index.ts deleted file mode 100644 index f86b7fdca0b..00000000000 --- a/src/main/runtime/terminal-leaf-membership-index.ts +++ /dev/null @@ -1,90 +0,0 @@ -import type { - TerminalLayoutSnapshot, - TerminalPaneLayoutNode -} from '../../shared/terminal-tab-types' - -type LayoutsByTabId = Record - -type LeafMembershipIndex = { - /** First tab (in record order) whose layout tree holds the leaf — matches the linear scan. */ - readonly tabIdByLeafId: ReadonlyMap - /** Root nodes the index was built from; absent tab reads back `undefined`, empty tab `null`. */ - readonly builtFrom: ReadonlyMap -} - -const EMPTY_INDEX: LeafMembershipIndex = { - tabIdByLeafId: new Map(), - builtFrom: new Map() -} - -// Sessions hand back the same layouts record across reads, so one index serves every lookup. -const indexByLayoutsRecord = new WeakMap() - -function addLeafIds( - node: TerminalPaneLayoutNode | null | undefined, - tabId: string, - tabIdByLeafId: Map -): void { - if (!node) { - return - } - if (node.type === 'leaf') { - if (!tabIdByLeafId.has(node.leafId)) { - tabIdByLeafId.set(node.leafId, tabId) - } - return - } - addLeafIds(node.first, tabId, tabIdByLeafId) - addLeafIds(node.second, tabId, tabIdByLeafId) -} - -/** - * Why an identity check and not just the WeakMap: a few writers copy the layouts record and then - * assign into the copy, so the record can be new while most layout objects are shared — and a - * cached index must not outlive a replaced layout. Comparing root references is O(tabs) pointer - * loads with no allocation, versus the rebuild's set-per-tab tree walk. - * - * Why the ROOT and not the layout object: `persistPtyBinding` grafts a leaf by assigning - * `layout.root` on the same layout object inside the same record (see the split/first-root branches - * in `persistence/loading-store/pty-binding-persistence.ts`), so a layout-identity check would keep - * serving an index that has never seen the grafted leaf. Membership is a pure function of the root - * tree, and no writer mutates a node in place, so root identity is the exact revalidation key. - */ -function isIndexCurrent(index: LeafMembershipIndex, layouts: LayoutsByTabId): boolean { - let seen = 0 - for (const tabId of Object.keys(layouts)) { - const builtFromRoot = index.builtFrom.get(tabId) - if (builtFromRoot === undefined || builtFromRoot !== (layouts[tabId]?.root ?? null)) { - return false - } - seen += 1 - } - return seen === index.builtFrom.size -} - -function buildIndex(layouts: LayoutsByTabId): LeafMembershipIndex { - const tabIdByLeafId = new Map() - const builtFrom = new Map() - for (const tabId of Object.keys(layouts)) { - const root = layouts[tabId]?.root ?? null - builtFrom.set(tabId, root) - addLeafIds(root, tabId, tabIdByLeafId) - } - return { tabIdByLeafId, builtFrom } -} - -/** leafId -> owning tabId for one session's layouts, rebuilt only when a layout object changes. */ -export function getTerminalLeafMembershipIndex( - layouts: LayoutsByTabId | undefined -): ReadonlyMap { - if (!layouts) { - return EMPTY_INDEX.tabIdByLeafId - } - const cached = indexByLayoutsRecord.get(layouts) - if (cached && isIndexCurrent(cached, layouts)) { - return cached.tabIdByLeafId - } - const built = buildIndex(layouts) - indexByLayoutsRecord.set(layouts, built) - return built.tabIdByLeafId -} diff --git a/src/main/runtime/terminal-leaf-tab-resolution.test.ts b/src/main/runtime/terminal-leaf-tab-resolution.test.ts new file mode 100644 index 00000000000..2fe8a6d0344 --- /dev/null +++ b/src/main/runtime/terminal-leaf-tab-resolution.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest' + +import type { TerminalLayoutSnapshot } from '../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' +import { findTerminalTabIdForLeaf } from './workspace-session-terminal-membership-authority' + +function layout(...leafIds: string[]): TerminalLayoutSnapshot { + let root = { type: 'leaf' as const, leafId: leafIds[0] } + for (const leafId of leafIds.slice(1)) { + root = { + type: 'split', + direction: 'row', + first: root, + second: { type: 'leaf' as const, leafId } + } as never + } + return { root, activeLeafId: leafIds[0], ptyIdsByLeafId: {} } as TerminalLayoutSnapshot +} + +function session(layouts: Record): WorkspaceSessionState { + return { terminalLayoutsByTabId: layouts } as WorkspaceSessionState +} + +describe('findTerminalTabIdForLeaf', () => { + it('resolves every leaf of a split tree to its tab', () => { + const state = session({ + 'tab-a': layout('leaf-1', 'leaf-2', 'leaf-3'), + 'tab-b': layout('leaf-4') + }) + expect(findTerminalTabIdForLeaf(state, 'leaf-2')).toBe('tab-a') + expect(findTerminalTabIdForLeaf(state, 'leaf-3')).toBe('tab-a') + expect(findTerminalTabIdForLeaf(state, 'leaf-4')).toBe('tab-b') + }) + + it('keeps the first tab in record order when two layouts claim one leaf', () => { + const layouts = { 'tab-a': layout('shared'), 'tab-b': layout('shared') } + expect(findTerminalTabIdForLeaf(session(layouts), 'shared')).toBe('tab-a') + }) + + it('answers misses, empty sessions and empty layouts with undefined', () => { + expect(findTerminalTabIdForLeaf(undefined, 'leaf-1')).toBeUndefined() + expect(findTerminalTabIdForLeaf(session({}), 'leaf-1')).toBeUndefined() + expect(findTerminalTabIdForLeaf(session({ 'tab-a': layout('leaf-1') }), 'nope')).toBeUndefined() + }) + + // The guard a leafId -> tabId cache needed and this scan does not: membership is read from the + // tree that is there NOW. `persistPtyBinding` grafts leaves by assigning into a layout already in + // the record, so anything memoized across calls has to be revalidated against every mutation + // shape a writer can produce - including one that leaves the root node's identity untouched. + it('reflects a subtree replaced in place after an earlier read', () => { + const tracked = layout('leaf-1', 'leaf-2') + const state = session({ 'tab-a': tracked }) + expect(findTerminalTabIdForLeaf(state, 'leaf-2')).toBe('tab-a') + expect(findTerminalTabIdForLeaf(state, 'leaf-9')).toBeUndefined() + + const root = tracked.root as { second: unknown } + root.second = { type: 'leaf', leafId: 'leaf-9' } + + expect(findTerminalTabIdForLeaf(state, 'leaf-9')).toBe('tab-a') + expect(findTerminalTabIdForLeaf(state, 'leaf-2')).toBeUndefined() + }) +}) diff --git a/src/main/runtime/workspace-session-terminal-membership-authority.ts b/src/main/runtime/workspace-session-terminal-membership-authority.ts index b787b1547c8..ecc0e555ac4 100644 --- a/src/main/runtime/workspace-session-terminal-membership-authority.ts +++ b/src/main/runtime/workspace-session-terminal-membership-authority.ts @@ -5,8 +5,8 @@ import type { } from '../../shared/terminal-tab-types' import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' import { getRepoIdFromWorktreeId } from '../../shared/worktree/id' +import { layoutContainsLeafId } from '../persistence/restoring-sessions/terminal-layout-normalization' import { pruneTabGroupLayoutAfterRetirement } from './mobile-session-terminal-retirement' -import { getTerminalLeafMembershipIndex } from './terminal-leaf-membership-index' function collectLeafIds(node: TerminalPaneLayoutNode | null, ids: Set): void { if (!node) { @@ -165,12 +165,25 @@ export function advanceTerminalTopologyRevision( * The tab whose live layout holds this leaf. Only the leaf half of a pane key is remint-stable — * `detachTerminalPaneToTab` moves a live pane into a new tab, so a stored tabId names the tab the * pane left. Callers fencing on location must resolve it here rather than trust a frozen tabId. + * + * Allocation-free and stateless on purpose: writers graft leaves by assigning into a layout that is + * already inside the layouts record, so any cache here would need a revalidation key that is itself + * O(tabs) per read — the same cost as this walk, with a staleness invariant to keep. */ export function findTerminalTabIdForLeaf( session: WorkspaceSessionState | undefined, leafId: string ): string | undefined { - return getTerminalLeafMembershipIndex(session?.terminalLayoutsByTabId).get(leafId) + const layouts = session?.terminalLayoutsByTabId + if (!layouts) { + return undefined + } + for (const tabId of Object.keys(layouts)) { + if (layoutContainsLeafId(layouts[tabId]?.root ?? null, leafId)) { + return tabId + } + } + return undefined } export function hasHostAuthoritativeTerminalMembership(