From 4381d0c206b1222322e6b4dacb4ce04d0e4449b5 Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 19 Sep 2026 05:41:32 -0700 Subject: [PATCH] fix(sidebar): preserve OMP status across sparse split pane ids Use stable PTY-to-leaf bindings when in-session pane closes leave sparse runtime pane ids. Keep the existing parked and dense slot resolution paths, and cover the completed OMP pane/sibling-running case from #15557. Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> --- .../worktree-title-derived-agent-rows.test.ts | 137 ++++++++++++++++++ .../worktree-title-derived-agent-rows.ts | 101 +++++++++---- .../src/lib/runtime-pane-title-leaf-id.ts | 36 +++++ 3 files changed, 248 insertions(+), 26 deletions(-) diff --git a/src/renderer/src/components/sidebar/worktree-title-derived-agent-rows.test.ts b/src/renderer/src/components/sidebar/worktree-title-derived-agent-rows.test.ts index b0e7340f373c..e30581af05d1 100644 --- a/src/renderer/src/components/sidebar/worktree-title-derived-agent-rows.test.ts +++ b/src/renderer/src/components/sidebar/worktree-title-derived-agent-rows.test.ts @@ -402,3 +402,140 @@ describe('buildTitleDerivedAgentRows', () => { expect(rows).toHaveLength(0) }) }) + +// Why: `runtimePaneTitlesByTabId` mixes two disjoint id spaces — live PaneManager +// ids (>= 1) and the `-(leafIndex + 1)` slots a parked tab mints — so attributing a +// title by its position in the numerically sorted slot list puts one split pane's +// lifecycle on its sibling's row (STA-3264). +describe('split-pane runtime title attribution', () => { + const LEAF_ID_3 = '99999999-9999-4999-8999-999999999999' + + function makeNestedSplitLayout(): TerminalLayoutSnapshot { + // Split once (leaf 1 | leaf 2), then split the FIRST pane again (leaf 3). + // Layout traversal order is [1, 3, 2]; pane-creation order is [1, 2, 3]. + return { + root: { + type: 'split', + direction: 'vertical', + first: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF_ID_1 }, + second: { type: 'leaf', leafId: LEAF_ID_3 } + }, + second: { type: 'leaf', leafId: LEAF_ID_2 } + }, + activeLeafId: LEAF_ID_1, + expandedLeafId: null + } + } + + function rowsFor( + paneTitles: Record, + layout: TerminalLayoutSnapshot, + ptyIds: string[] + ) { + return buildWorktreeAgentRows({ + tabs: [makeTab('tab-1', { title: '⠋ Codex' })], + entries: [], + retained: [], + runtimePaneTitlesByTabId: { 'tab-1': paneTitles }, + ptyIdsByTabId: { 'tab-1': ptyIds }, + terminalLayoutsByTabId: { 'tab-1': layout }, + now: 2000 + }) + } + + it('keeps a finished split pane out of Running while its sibling keeps working', () => { + // A parked split tab reports its panes through synthetic slots numbered off the + // in-order leaf list: -1 is the first leaf, -2 the second. + const rows = rowsFor({ '-1': 'Codex', '-2': '⠋ Codex' }, makeSplitLayout(), ['pty-a', 'pty-b']) + + expect(rows.map((row) => [row.paneKey, row.state, row.entry.lastAssistantMessage])).toEqual([ + [makePaneKey('tab-1', LEAF_ID_1), 'idle', 'Idle'], + [makePaneKey('tab-1', LEAF_ID_2), 'working', 'Running'] + ]) + }) + + it('lets a revealed tab’s live slots outrank the parked slots it left behind', () => { + // Revealing a parked tab mounts live slots without clearing the parked ones, so + // both id spaces describe the same two leaves at once. The live pair is current: + // leaf 1 has finished, leaf 2 is still working. + const rows = rowsFor( + { '-1': '⠋ Codex', '-2': '⠋ Codex', 1: 'Codex', 2: '⠋ Codex' }, + makeSplitLayout(), + ['pty-a', 'pty-b'] + ) + + expect(rows.map((row) => [row.paneKey, row.state])).toEqual([ + [makePaneKey('tab-1', LEAF_ID_1), 'idle'], + [makePaneKey('tab-1', LEAF_ID_2), 'working'] + ]) + }) + + it('does not let sibling panes inherit each other’s agent or state', () => { + const rows = rowsFor( + { 1: 'Antigravity', 2: '⠋ Codex', 3: '⠋ Gemini CLI' }, + makeNestedSplitLayout(), + ['pty-a', 'pty-b', 'pty-c'] + ) + + expect( + rows + .map((row) => [row.paneKey, row.agentType, row.state]) + .sort((a, b) => (a[0] < b[0] ? -1 : 1)) + ).toEqual([ + [makePaneKey('tab-1', LEAF_ID_1), 'antigravity', 'idle'], + [makePaneKey('tab-1', LEAF_ID_2), 'codex', 'working'], + [makePaneKey('tab-1', LEAF_ID_3), 'gemini', 'working'] + ]) + }) + + it('does not recycle a closed split pane’s row onto a surviving sibling', () => { + // Panes 1|2|3 were open; closing pane 1 promotes the surviving pair and clears + // only that pane's slot, leaving the survivors' live ids sparse (2, 3). + const survivingLayout: TerminalLayoutSnapshot = { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF_ID_2 }, + second: { type: 'leaf', leafId: LEAF_ID_3 } + }, + activeLeafId: LEAF_ID_2, + expandedLeafId: null + } + const rows = rowsFor({ 2: '⠋ Codex', 3: 'Gemini CLI' }, survivingLayout, ['pty-b', 'pty-c']) + + expect(rows.map((row) => [row.paneKey, row.agentType, row.state])).toEqual([ + [makePaneKey('tab-1', LEAF_ID_2), 'codex', 'working'], + [makePaneKey('tab-1', LEAF_ID_3), 'gemini', 'idle'] + ]) + expect(rows.some((row) => row.paneKey === makePaneKey('tab-1', LEAF_ID_1))).toBe(false) + }) + + it('uses live PTY bindings when sparse pane ids no longer match layout order', () => { + // Closing the first pane and splitting the second leaves ids 2 and 4 while + // the surviving layout is ordered [new pane, old pane]. The PTY bindings + // are the only authoritative bridge from those sparse runtime slots to leaves. + const survivingLayout: TerminalLayoutSnapshot = { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF_ID_3 }, + second: { type: 'leaf', leafId: LEAF_ID_2 } + }, + activeLeafId: LEAF_ID_3, + expandedLeafId: null, + ptyIdsByLeafId: { + [LEAF_ID_2]: 'pty-old', + [LEAF_ID_3]: 'pty-new' + } + } + const rows = rowsFor({ 2: 'Codex', 4: '⠋ Gemini CLI' }, survivingLayout, ['pty-old', 'pty-new']) + + expect(rows.map((row) => [row.paneKey, row.agentType, row.state])).toEqual([ + [makePaneKey('tab-1', LEAF_ID_2), 'codex', 'idle'], + [makePaneKey('tab-1', LEAF_ID_3), 'gemini', 'working'] + ]) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-title-derived-agent-rows.ts b/src/renderer/src/components/sidebar/worktree-title-derived-agent-rows.ts index 08c0d27b9e74..341cb0e1ff28 100644 --- a/src/renderer/src/components/sidebar/worktree-title-derived-agent-rows.ts +++ b/src/renderer/src/components/sidebar/worktree-title-derived-agent-rows.ts @@ -9,12 +9,14 @@ import type { AgentStatusState, AgentType } from '../../../../shared/agent-status-types' +import { FIRST_PANE_ID } from '../../../../shared/pane-key' +import { + resolveRuntimePaneTitleLeafIdFromRoot, + resolveRuntimePaneTitleLeafIdFromSparseSlots, + collectRuntimePaneLeafIds +} from '@/lib/runtime-pane-title-leaf-id' import { isTerminalLeafId, makePaneKey } from '../../../../shared/stable-pane-id' -import type { - TerminalLayoutSnapshot, - TerminalPaneLayoutNode, - TerminalTab -} from '../../../../shared/terminal-tab-types' +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../../shared/terminal-tab-types' import { normalizeCompatibleAgentTitleForOwner, resolveCompatibleAgentTypeForOwner, @@ -74,14 +76,32 @@ export function buildTitleDerivedAgentRows(args: { const paneTitles = runtimePaneTitlesByTabId[tab.id] const paneTitleEntries = paneTitles && Object.keys(paneTitles).length > 0 - ? Object.entries(paneTitles).sort(([a], [b]) => Number(a) - Number(b)) + ? Object.entries(paneTitles).sort(([a], [b]) => { + const paneIdA = Number(a) + const paneIdB = Number(b) + const isLiveA = paneIdA >= FIRST_PANE_ID + return isLiveA !== paneIdB >= FIRST_PANE_ID ? (isLiveA ? -1 : 1) : paneIdA - paneIdB + }) : [] if (paneTitleEntries.length > 0) { + // Why: hoisted per tab — the leaf lists are layout-derived, not pane-derived. + const leafIds = collectRuntimePaneLeafIds(layout?.root ?? null) + const liveSlotIds = paneTitleEntries + .map(([paneId]) => Number(paneId)) + .filter((paneId) => paneId >= FIRST_PANE_ID) + // Why: pane ids only encode creation order while they are the dense sequence a + // fresh mount or replay allocates; an in-session pane close leaves them sparse. + const liveSlotsAreDense = + liveSlotIds.length === leafIds.length && + liveSlotIds.every((paneId, index) => paneId === FIRST_PANE_ID + index) for (const [paneId, title] of paneTitleEntries) { const leafId = resolveLeafIdForTitleFallback({ layout, - paneTitleEntries, + leafIds, + ptyIds: ptyIdsByTabId[tab.id] ?? [], + liveSlotIds, + liveSlotsAreDense, paneId: Number(paneId), title }) @@ -105,7 +125,7 @@ export function buildTitleDerivedAgentRows(args: { continue } - const leafId = layout?.activeLeafId ?? collectLeafIds(layout?.root ?? null)[0] + const leafId = layout?.activeLeafId ?? collectRuntimePaneLeafIds(layout?.root ?? null)[0] if (!leafId) { continue } @@ -297,12 +317,54 @@ function titleStatusToRowState( return 'idle' } +/** + * Resolves the layout leaf that owns a runtime pane title. + * + * `runtimePaneTitlesByTabId` mixes two disjoint id spaces: live PaneManager ids + * (`>= FIRST_PANE_ID`, allocated in pane-creation order) and the `-(leafIndex + 1)` + * slots parked tabs mint in `fallbackParkedPaneCandidates`. Neither space is ordered + * like the layout's in-order leaf traversal, so attributing a title by its position + * in the slot list lands one pane's status on a sibling's row. + */ function resolveLeafIdForTitleFallback(args: { layout: TerminalLayoutSnapshot | undefined - paneTitleEntries: [string, string][] + leafIds: string[] + ptyIds: string[] + liveSlotIds: number[] + liveSlotsAreDense: boolean paneId: number title: string }): string | null { + if (args.leafIds.length === 1) { + return args.leafIds[0] + } + if (args.paneId < FIRST_PANE_ID) { + // Parked slots are defined off the in-order leaf list, so invert that definition. + return args.leafIds[-args.paneId - 1] ?? null + } + if (args.liveSlotsAreDense) { + const creationOrderLeafId = resolveRuntimePaneTitleLeafIdFromRoot( + args.layout?.root, + String(args.paneId) + ) + if (creationOrderLeafId) { + return creationOrderLeafId + } + } + + // After an in-session close, PaneManager ids are sparse while the tab's live + // PTYs retain their relative order. Use the durable PTY-to-leaf bindings to + // recover the exact leaf instead of assigning a survivor by layout position. + const ptyBoundLeafId = resolveRuntimePaneTitleLeafIdFromSparseSlots({ + layout: args.layout, + paneId: args.paneId, + liveSlotIds: args.liveSlotIds, + ptyIds: args.ptyIds + }) + if (ptyBoundLeafId) { + return ptyBoundLeafId + } + const matchingTitleLeafIds = Object.entries(args.layout?.titlesByLeafId ?? {}) .filter(([, title]) => title === args.title) .map(([leafId]) => leafId) @@ -310,21 +372,8 @@ function resolveLeafIdForTitleFallback(args: { return matchingTitleLeafIds[0] } - const leafIds = collectLeafIds(args.layout?.root ?? null) - if (leafIds.length === 1) { - return leafIds[0] - } - - const paneIndex = args.paneTitleEntries.findIndex(([paneId]) => Number(paneId) === args.paneId) - return paneIndex !== -1 ? (leafIds[paneIndex] ?? null) : null -} - -function collectLeafIds(node: TerminalPaneLayoutNode | null): string[] { - if (!node) { - return [] - } - if (node.type === 'leaf') { - return [node.leafId] - } - return [...collectLeafIds(node.first), ...collectLeafIds(node.second)] + // Why: in-session pane closes leave the survivors' ids sparse, which creation order + // cannot resolve. Index within the LIVE slots only — never across both id spaces. + const paneIndex = args.liveSlotIds.indexOf(args.paneId) + return paneIndex !== -1 ? (args.leafIds[paneIndex] ?? null) : null } diff --git a/src/renderer/src/lib/runtime-pane-title-leaf-id.ts b/src/renderer/src/lib/runtime-pane-title-leaf-id.ts index 78c669cef66a..741d34a9d4b1 100644 --- a/src/renderer/src/lib/runtime-pane-title-leaf-id.ts +++ b/src/renderer/src/lib/runtime-pane-title-leaf-id.ts @@ -45,6 +45,18 @@ function collectLeafIdsInReplayCreationOrder( return leafIdsInReplayCreationOrder } +export function collectRuntimePaneLeafIds( + node: TerminalPaneLayoutNode | null | undefined +): string[] { + if (!node) { + return [] + } + if (node.type === 'leaf') { + return [node.leafId] + } + return [...collectRuntimePaneLeafIds(node.first), ...collectRuntimePaneLeafIds(node.second)] +} + export function resolveRuntimePaneTitleLeafId( tabLayout: { root?: TerminalLayoutSnapshot['root'] } | undefined, runtimePaneId: string @@ -121,3 +133,27 @@ export function resolveRuntimePaneTitleLeafIdFromRoot( const leafIds = collectLeafIdsInReplayCreationOrder(root) return leafIds[numericPaneId - FIRST_PANE_ID] ?? null } + +/** + * Resolve a sparse live runtime pane slot through the tab's current PTY bindings. + * PaneManager ids survive closes with gaps, while the live PTY list retains order. + */ +export function resolveRuntimePaneTitleLeafIdFromSparseSlots(args: { + layout: Pick | undefined + paneId: number + liveSlotIds: number[] + ptyIds: string[] +}): string | null { + if (args.ptyIds.length !== args.liveSlotIds.length) { + return null + } + const paneIndex = args.liveSlotIds.indexOf(args.paneId) + const ptyId = paneIndex === -1 ? undefined : args.ptyIds[paneIndex] + if (!ptyId) { + return null + } + const boundLeafIds = Object.entries(args.layout?.ptyIdsByLeafId ?? {}) + .filter(([, boundPtyId]) => boundPtyId === ptyId) + .map(([leafId]) => leafId) + return boundLeafIds.length === 1 ? boundLeafIds[0] : null +}