From 7675da363e3f004a1e5fd0d2d08e8dcbac29142c Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:31:26 -0700 Subject: [PATCH] fix(remote): stop paired-tab resurrection, ghost agent rows, and frozen visible panes (STA-4593) (#15459) --- ...obile-close-preserved-resurrection.test.ts | 517 ++++++++++++ src/main/runtime/orca-runtime.ts | 95 +++ .../pty-connection-test-pane-fixtures.ts | 1 + ...on-visible-pane-output-pause-latch.test.ts | 243 ++++++ .../terminal-pane/pty-connection.ts | 5 + ...ync-host-tab-retraction-ghost-rows.test.ts | 734 ++++++++++++++++++ .../src/runtime/web-session-tabs-sync.ts | 128 ++- src/renderer/src/store/slices/agent-status.ts | 251 +++--- .../src/store/slices/pane-foreground-agent.ts | 15 +- .../retired-terminal-tab-state-sweep.ts | 96 +++ src/renderer/src/store/slices/terminals.ts | 19 +- 11 files changed, 1974 insertions(+), 130 deletions(-) create mode 100644 src/main/runtime/orca-runtime-mobile-close-preserved-resurrection.test.ts create mode 100644 src/renderer/src/components/terminal-pane/pty-connection-visible-pane-output-pause-latch.test.ts create mode 100644 src/renderer/src/runtime/web-session-tabs-sync-host-tab-retraction-ghost-rows.test.ts create mode 100644 src/renderer/src/store/slices/retired-terminal-tab-state-sweep.ts diff --git a/src/main/runtime/orca-runtime-mobile-close-preserved-resurrection.test.ts b/src/main/runtime/orca-runtime-mobile-close-preserved-resurrection.test.ts new file mode 100644 index 00000000000..5780b353944 --- /dev/null +++ b/src/main/runtime/orca-runtime-mobile-close-preserved-resurrection.test.ts @@ -0,0 +1,517 @@ +/** + * STA-4593 incident: closing paired-remote tabs "worked briefly" but the tabs + * returned seconds later and after workspace switches, on a host whose PTYs + * outlive their tabs. + * + * Invariant under test: a user close committed through closeMobileSessionTab + * stays closed — no later host publication may re-add the tab while its PTY + * lingers connected. + * + * Causal boundary: shouldPreserveHeadlessMobileSessionTab preserves any + * renderer-omitted tab whose PTY is connected && runtimeSessionOwned. Every + * paired-created terminal is runtimeSessionOwned, and the close path clears + * that flag only AFTER the renderer relay acknowledges — but the renderer's + * own prune publication normally lands BEFORE the ack (retire, publish, then + * reply). That publication runs the preserved-merge while the flag is still + * set and the PTY exit has not fired yet, so the merge resurrects the tab the + * renderer just retired, and nothing prunes it again until some future + * renderer publication. + * + * The harness is the close-continuity pattern: a real OrcaRuntimeService with + * injected store/notifier/pty controller. Interleavings are controlled with a + * deferred relay ack; time is never the oracle. + */ +import { describe, expect, it, vi } from 'vitest' +import { getDefaultWorkspaceSession } from '../../shared/constants' +import { makePaneKey } from '../../shared/stable-pane-id' +import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' +import { OrcaRuntimeService } from './orca-runtime' + +const REPO_ID = 'repo-preserved-resurrection' +const WORKTREE_PATH = '/tmp/preserved-resurrection' +const WORKTREE_ID = `${REPO_ID}::${WORKTREE_PATH}` + +const TAB_A = 'tab-reviewer-a' +const TAB_B = 'tab-main-b' +const LEAF_A = '11111111-1111-4111-8111-111111111111' +const LEAF_B = '22222222-2222-4222-8222-222222222222' +const PTY_A = 'pty-reviewer-a' +const PTY_B = 'pty-main-b' +const INC_A = '33333333-3333-4333-8333-333333333333' +const INC_B = '44444444-4444-4444-8444-444444444444' + +const TAB_P = 'tab-split-parent' +// LEAF_C: the persisted session's alternative leafId for PTY_A's surface — +// renderer and headless sources can derive different leafIds for one surface. +const LEAF_C = '55555555-5555-4555-8555-555555555555' +const PTY_BY_LEAF: Record = { [LEAF_A]: PTY_A, [LEAF_B]: PTY_B, [LEAF_C]: PTY_A } +const INC_BY_LEAF: Record = { [LEAF_A]: INC_A, [LEAF_B]: INC_B, [LEAF_C]: INC_A } + +type TabSpec = { tabId: string; leafId: string; ptyId: string } +const TAB_SPECS: Record = { + [TAB_A]: { tabId: TAB_A, leafId: LEAF_A, ptyId: PTY_A }, + [TAB_B]: { tabId: TAB_B, leafId: LEAF_B, ptyId: PTY_B } +} + +function makeSession(tabIds: readonly string[]): WorkspaceSessionState { + return { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [WORKTREE_ID]: tabIds.map((tabId, index) => ({ + id: tabId, + ptyId: TAB_SPECS[tabId]!.ptyId, + worktreeId: WORKTREE_ID, + title: tabId, + customTitle: null, + color: null, + sortOrder: index, + createdAt: index + 1 + })) + }, + terminalLayoutsByTabId: Object.fromEntries( + tabIds.map((tabId) => [ + tabId, + { + root: { type: 'leaf' as const, leafId: TAB_SPECS[tabId]!.leafId }, + activeLeafId: TAB_SPECS[tabId]!.leafId, + expandedLeafId: null, + ptyIdsByLeafId: { [TAB_SPECS[tabId]!.leafId]: TAB_SPECS[tabId]!.ptyId } + } + ]) + ), + terminalPtyIncarnationsByPaneKey: { + [makePaneKey(TAB_A, LEAF_A)]: INC_A, + [makePaneKey(TAB_B, LEAF_B)]: INC_B + } + } +} + +function makeDeferred() { + let resolve!: () => void + const promise = new Promise((settle) => { + resolve = settle + }) + return { promise, resolve } +} + +function createHarness() { + let session = makeSession([TAB_A, TAB_B]) + const repo = { + id: REPO_ID, + path: WORKTREE_PATH, + displayName: 'preserved-resurrection', + badgeColor: '#000000', + addedAt: 1 + } + const store = { + getRepos: () => [repo], + getRepo: (id: string) => (id === REPO_ID ? repo : undefined), + getAllWorktreeMeta: () => ({}), + getWorktreeMeta: () => undefined, + getSettings: () => ({ workspaceDir: '/tmp/workspaces' }), + getProjects: () => [], + getWorkspaceSession: () => session, + setWorkspaceSession: (next: WorkspaceSessionState) => { + session = next + }, + flushOrThrow: () => {} + } + const relayAck = makeDeferred() + const closeTerminal = vi.fn() + const closeTerminalTab = vi.fn(() => relayAck.promise) + const kill = vi.fn(() => true) + const runtime = new OrcaRuntimeService(store as never) + runtime.setNotifier({ closeTerminal, closeTerminalTab } as never) + runtime.setPtyController({ + write: () => true, + kill, + stopAndWait: vi.fn(async () => true), + listProcesses: vi.fn(async () => [ + { id: PTY_A, incarnationId: INC_A, cwd: WORKTREE_PATH, title: 'reviewer shell' }, + { id: PTY_B, incarnationId: INC_B, cwd: WORKTREE_PATH, title: 'main shell' } + ]), + getForegroundProcess: async () => null + } as never) + runtime.attachWindow(1) + + const rendererGraph = (tabIds: readonly string[]) => ({ + tabs: tabIds.map((tabId) => ({ + tabId, + worktreeId: WORKTREE_ID, + title: tabId, + activeLeafId: TAB_SPECS[tabId]!.leafId, + layout: { type: 'leaf' as const, leafId: TAB_SPECS[tabId]!.leafId } + })), + leaves: tabIds.map((tabId, index) => ({ + tabId, + worktreeId: WORKTREE_ID, + leafId: TAB_SPECS[tabId]!.leafId, + paneRuntimeId: index + 7, + ptyId: TAB_SPECS[tabId]!.ptyId + })) + }) + + const publishRendererSnapshot = (tabIds: readonly string[], snapshotVersion: number) => { + runtime.syncWindowGraph(1, { + ...rendererGraph(tabIds), + mobileSessionTabs: [ + { + worktree: WORKTREE_ID, + publicationEpoch: 'renderer:preserved-resurrection', + snapshotVersion, + activeGroupId: 'group-1', + activeTabId: tabIds.length ? `${tabIds[0]}::${TAB_SPECS[tabIds[0]!]!.leafId}` : null, + activeTabType: tabIds.length ? ('terminal' as const) : null, + tabs: tabIds.map((tabId, index) => ({ + type: 'terminal' as const, + id: `${tabId}::${TAB_SPECS[tabId]!.leafId}`, + parentTabId: tabId, + leafId: TAB_SPECS[tabId]!.leafId, + ptyId: TAB_SPECS[tabId]!.ptyId, + title: tabId, + isActive: index === 0 + })) + } + ] + }) + } + + for (const spec of Object.values(TAB_SPECS)) { + runtime.registerPty(spec.ptyId, WORKTREE_ID, null, { + tabId: spec.tabId, + leafId: spec.leafId, + incarnationId: spec.tabId === TAB_A ? INC_A : INC_B + }) + // Why: byte-identical to a paired create — ensurePtyBackedMobileSurfaceForRendererTab + // marks every paired-created PTY runtimeSessionOwned + paired-session-owned, + // and nothing clears either flag until close/exit. + const record = ( + runtime as unknown as { + ptysById: Map + } + ).ptysById.get(spec.ptyId)! + record.runtimeSessionOwned = true + ;( + runtime as unknown as { pairedRendererSessionOwnedPtyIds: Set } + ).pairedRendererSessionOwnedPtyIds.add(spec.ptyId) + } + publishRendererSnapshot([TAB_A, TAB_B], 1) + + return { + runtime, + relayAck, + closeTerminalTab, + kill, + publishRendererSnapshot, + retirePersistedTab: (tabId: string) => { + session = makeSession([TAB_A, TAB_B].filter((id) => id !== tabId)) + }, + isRuntimeSessionOwned: (ptyId: string) => + ( + runtime as unknown as { ptysById: Map } + ).ptysById.get(ptyId)?.runtimeSessionOwned === true, + markCreatePending: (tabId: string) => { + ;( + runtime as unknown as { + pendingMobileTerminalCreatesByKey: Map + } + ).pendingMobileTerminalCreatesByKey.set(`${WORKTREE_ID}::${tabId}`, { + activate: true, + paired: true, + selectIfNoActiveTab: true + }) + } + } +} + +function makeSplitLayout(leafIds: readonly string[]) { + const root = + leafIds.length > 1 + ? { + type: 'split' as const, + direction: 'vertical' as const, + first: { type: 'leaf' as const, leafId: leafIds[0]! }, + second: { type: 'leaf' as const, leafId: leafIds[1]! }, + ratio: 0.5 + } + : { type: 'leaf' as const, leafId: leafIds[0]! } + return { + root, + activeLeafId: leafIds[0]!, + expandedLeafId: null, + ptyIdsByLeafId: Object.fromEntries(leafIds.map((leafId) => [leafId, PTY_BY_LEAF[leafId]!])) + } +} + +/** One parent tab split across `leafIds` — closing a leaf rewrites the layout, not the tab. */ +function makeSplitSession(leafIds: readonly string[]): WorkspaceSessionState { + return { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [WORKTREE_ID]: [ + { + id: TAB_P, + ptyId: PTY_BY_LEAF[leafIds[0]!]!, + worktreeId: WORKTREE_ID, + title: TAB_P, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + terminalLayoutsByTabId: { [TAB_P]: makeSplitLayout(leafIds) }, + terminalPtyIncarnationsByPaneKey: Object.fromEntries( + leafIds.map((leafId) => [makePaneKey(TAB_P, leafId), INC_BY_LEAF[leafId]!]) + ) + } +} + +function createSplitHarness() { + let session = makeSplitSession([LEAF_A, LEAF_B]) + const repo = { + id: REPO_ID, + path: WORKTREE_PATH, + displayName: 'preserved-resurrection', + badgeColor: '#000000', + addedAt: 1 + } + const store = { + getRepos: () => [repo], + getRepo: (id: string) => (id === REPO_ID ? repo : undefined), + getAllWorktreeMeta: () => ({}), + getWorktreeMeta: () => undefined, + getSettings: () => ({ workspaceDir: '/tmp/workspaces' }), + getProjects: () => [], + getWorkspaceSession: () => session, + setWorkspaceSession: (next: WorkspaceSessionState) => { + session = next + }, + flushOrThrow: () => {} + } + const runtime = new OrcaRuntimeService(store as never) + runtime.setNotifier({ closeTerminal: vi.fn(), closeTerminalTab: vi.fn(async () => {}) } as never) + runtime.setPtyController({ + write: () => true, + kill: vi.fn(() => true), + stopAndWait: vi.fn(async () => true), + listProcesses: vi.fn(async () => [ + { id: PTY_A, incarnationId: INC_A, cwd: WORKTREE_PATH, title: 'left pane' }, + { id: PTY_B, incarnationId: INC_B, cwd: WORKTREE_PATH, title: 'right pane' } + ]), + getForegroundProcess: async () => null + } as never) + runtime.attachWindow(1) + + const publishSplitSnapshot = (leafIds: readonly string[], snapshotVersion: number) => { + const layout = makeSplitLayout(leafIds) + runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: TAB_P, + worktreeId: WORKTREE_ID, + title: TAB_P, + activeLeafId: layout.activeLeafId, + layout: layout.root + } + ], + leaves: leafIds.map((leafId, index) => ({ + tabId: TAB_P, + worktreeId: WORKTREE_ID, + leafId, + paneRuntimeId: index + 7, + ptyId: PTY_BY_LEAF[leafId]! + })), + mobileSessionTabs: [ + { + worktree: WORKTREE_ID, + publicationEpoch: 'renderer:preserved-resurrection', + snapshotVersion, + activeGroupId: 'group-1', + activeTabId: `${TAB_P}::${layout.activeLeafId}`, + activeTabType: 'terminal' as const, + tabs: leafIds.map((leafId, index) => ({ + type: 'terminal' as const, + id: `${TAB_P}::${leafId}`, + parentTabId: TAB_P, + leafId, + ptyId: PTY_BY_LEAF[leafId]!, + parentLayout: layout, + title: `${TAB_P} ${leafId}`, + isActive: index === 0 + })) + } + ] + }) + } + + for (const leafId of [LEAF_A, LEAF_B]) { + runtime.registerPty(PTY_BY_LEAF[leafId]!, WORKTREE_ID, null, { + tabId: TAB_P, + leafId, + incarnationId: INC_BY_LEAF[leafId]! + }) + const record = ( + runtime as unknown as { ptysById: Map } + ).ptysById.get(PTY_BY_LEAF[leafId]!)! + record.runtimeSessionOwned = true + ;( + runtime as unknown as { pairedRendererSessionOwnedPtyIds: Set } + ).pairedRendererSessionOwnedPtyIds.add(PTY_BY_LEAF[leafId]!) + } + publishSplitSnapshot([LEAF_A, LEAF_B], 1) + + return { + runtime, + publishSplitSnapshot, + isRuntimeSessionOwned: (ptyId: string) => + ( + runtime as unknown as { ptysById: Map } + ).ptysById.get(ptyId)?.runtimeSessionOwned === true, + retirePersistedLeaf: (leafId: string) => { + session = makeSplitSession([LEAF_A, LEAF_B].filter((id) => id !== leafId)) + }, + relabelPersistedLeftLeaf: () => { + session = makeSplitSession([LEAF_C, LEAF_B]) + } + } +} + +async function listTerminalSurfaceIds(runtime: OrcaRuntimeService): Promise { + const listed = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + return listed.tabs + .filter((tab) => tab.type === 'terminal') + .map((tab) => tab.id) + .sort() +} + +async function listParentTabIds(runtime: OrcaRuntimeService): Promise { + const listed = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + return [ + ...new Set( + listed.tabs.map((tab) => (tab.type === 'terminal' ? tab.parentTabId : tab.id)).sort() + ) + ] +} + +describe('a committed paired close stays closed while its PTY lingers', () => { + it('control: a renderer prune that lands after the relay ack retires the tab for good', async () => { + const harness = createHarness() + + const closing = harness.runtime.closeMobileSessionTab(`id:${WORKTREE_ID}`, TAB_A, { + reason: 'user' + }) + await vi.waitFor(() => expect(harness.closeTerminalTab).toHaveBeenCalledWith(TAB_A)) + harness.retirePersistedTab(TAB_A) + harness.relayAck.resolve() + await expect(closing).resolves.toMatchObject({ closed: true }) + // The renderer's prune publication arrives only after the ack (and after + // the close cleared the ownership flags). + harness.publishRendererSnapshot([TAB_B], 2) + + expect(await listParentTabIds(harness.runtime)).toEqual([TAB_B]) + }) + + it('red: the preserved-merge resurrects a pre-ack renderer prune only until the close cleans up', async () => { + const harness = createHarness() + + const closing = harness.runtime.closeMobileSessionTab(`id:${WORKTREE_ID}`, TAB_A, { + reason: 'user' + }) + await vi.waitFor(() => expect(harness.closeTerminalTab).toHaveBeenCalledWith(TAB_A)) + // Production ordering: the renderer durably retires the tab and publishes + // the pruned graph BEFORE replying to the relay. The kill it dispatched has + // not produced an exit yet, so the PTY record is still connected. + harness.retirePersistedTab(TAB_A) + harness.publishRendererSnapshot([TAB_B], 2) + harness.relayAck.resolve() + await expect(closing).resolves.toMatchObject({ closed: true }) + + const republished = await listParentTabIds(harness.runtime) + expect( + republished, + 'the preserved-merge resurrected a tab whose user close was committed' + ).toEqual([TAB_B]) + }) + + it('red: a host-side close (no paired close RPC) must not be resurrected by the preserved-merge', async () => { + const harness = createHarness() + + // The host closes the tab itself — host user gesture or `orca terminal` + // lifecycle driven by an agent on the host. No closeMobileSessionTab runs, + // so no post-relay cleanup exists. The renderer durably retires the tab and + // publishes the pruned graph; the PTY it killed has not exited yet (or the + // kill failed — WSL/Windows), so its record is still connected and still + // carries the paired-create ownership flags. + harness.retirePersistedTab(TAB_A) + harness.publishRendererSnapshot([TAB_B], 2) + + const republished = await listParentTabIds(harness.runtime) + expect(republished, 'the preserved-merge resurrected a tab the host renderer closed').toEqual([ + TAB_B + ]) + }) + + it('control: a create still in flight is not retirement — its tab and ownership survive the omission', async () => { + const harness = createHarness() + + // The renderer's stale publication omits TAB_A while its paired create has + // not settled: it is missing from persistence AND from the publication, the + // exact input pattern durable retirement keys on. The pending-create mark + // is the only discriminator, and losing it would strand the create-recovery + // rescue: the freshly created remote tab would vanish. + harness.markCreatePending(TAB_A) + harness.retirePersistedTab(TAB_A) + harness.publishRendererSnapshot([TAB_B], 2) + + expect( + await listParentTabIds(harness.runtime), + 'a create-in-flight tab was retired by a stale publication' + ).toEqual([TAB_B, TAB_A]) + expect(harness.isRuntimeSessionOwned(PTY_A)).toBe(true) + }) +}) + +describe('a durably closed split leaf stays closed while its sibling survives', () => { + it('red: the preserved-merge must not resurrect a retired leaf of a persisted parent', async () => { + const harness = createSplitHarness() + expect(await listTerminalSurfaceIds(harness.runtime)).toEqual([ + `${TAB_P}::${LEAF_A}`, + `${TAB_P}::${LEAF_B}` + ]) + + // The host closes the left pane only: the parent tab persists with the + // surviving leaf, and the killed PTY has not exited yet (kill lag, or a + // failed kill on WSL/Windows), so it stays connected + paired-create owned. + harness.retirePersistedLeaf(LEAF_A) + harness.publishSplitSnapshot([LEAF_B], 2) + + expect( + await listTerminalSurfaceIds(harness.runtime), + 'the preserved-merge resurrected a split leaf the host renderer closed' + ).toEqual([`${TAB_P}::${LEAF_B}`]) + expect(harness.isRuntimeSessionOwned(PTY_A)).toBe(false) + // The surviving sibling keeps its ownership: the release must be leaf-scoped. + expect(harness.isRuntimeSessionOwned(PTY_B)).toBe(true) + }) + + it('control: a persisted layout that still binds the PTY under a different leafId is not retirement', async () => { + const harness = createSplitHarness() + + // The persisted session relabels PTY_A's surface to another leafId (renderer + // and headless sources can derive different leafIds for one surface), and a + // stale publication omits it. The still-bound PTY id is the only signal that + // the leaf lives on; releasing on the leafId mismatch alone would retire a + // live pane and let the merge drop it. + harness.relabelPersistedLeftLeaf() + harness.publishSplitSnapshot([LEAF_B], 2) + + expect( + await listTerminalSurfaceIds(harness.runtime), + 'a live relabeled leaf was retired by a stale publication' + ).toEqual([`${TAB_P}::${LEAF_A}`, `${TAB_P}::${LEAF_B}`]) + expect(harness.isRuntimeSessionOwned(PTY_A)).toBe(true) + expect(harness.isRuntimeSessionOwned(PTY_B)).toBe(true) + }) +}) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 54051d63aaa..429c3327036 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -1088,6 +1088,7 @@ import { } from '../../shared/worktree/retired-name-registry' import { normalizeSparseDirectories } from '../ipc/sparse-checkout-directories' import type { PtyBindingSourceExpectation, Store } from '../persistence' +import { collectLayoutLeafIdsInOrder } from '../persistence/restoring-sessions/terminal-layout-normalization' import type { StatsCollector } from '../stats/collector' import { computeValidatedBranchName, @@ -7617,6 +7618,99 @@ export class OrcaRuntimeService { } } + private getMobileTerminalLeafPtyIds(tab: RuntimeMobileSessionTerminalTab): string[] { + return [tab.ptyId, tab.parentLayout?.ptyIdsByLeafId?.[tab.leafId]].filter( + (ptyId): ptyId is string => typeof ptyId === 'string' && ptyId.length > 0 + ) + } + + private clearRuntimeSessionOwnershipForMobileTerminalLeaf( + worktreeId: string, + tab: RuntimeMobileSessionTerminalTab + ): void { + for (const ptyId of this.getMobileTerminalLeafPtyIds(tab)) { + const pty = this.ptysById.get(ptyId) + if (pty?.worktreeId === worktreeId && pty.tabId === tab.parentTabId) { + pty.runtimeSessionOwned = false + this.setPairedRendererSessionOwnership(pty.ptyId, false) + } + } + } + + // Why: only positive evidence that the persisted parent dropped this leaf may + // release it — a parent with no persisted layout is no evidence of a split. + private persistedParentStillBindsMobileTerminalLeaf( + session: WorkspaceSessionState, + persistedParent: TerminalTab, + tab: RuntimeMobileSessionTerminalTab + ): boolean { + const layout = session.terminalLayoutsByTabId?.[tab.parentTabId] + if (!layout) { + return true + } + if ( + typeof layout.ptyIdsByLeafId?.[tab.leafId] === 'string' || + collectLayoutLeafIdsInOrder(layout.root).includes(tab.leafId) + ) { + return true + } + // Why: renderer and headless sources can derive different leafIds for one + // surface, so a still-bound PTY id outranks a leafId that no longer matches. + const leafPtyIds = new Set(this.getMobileTerminalLeafPtyIds(tab)) + if (leafPtyIds.size === 0) { + return true + } + return [persistedParent.ptyId, ...Object.values(layout.ptyIdsByLeafId ?? {})].some( + (ptyId) => typeof ptyId === 'string' && leafPtyIds.has(ptyId) + ) + } + + // Why: omitted from the publication AND from persistence = durably closed, so release + // ownership — else a lagging or failed kill preserves the tab back into every merge. + // A create still in flight has not been retired, only not published yet. + private releaseRuntimeSessionOwnershipForRendererRetiredTabs( + incoming: RuntimeMobileSessionTabsSnapshot, + existing: RuntimeMobileSessionTabsSnapshot | undefined + ): void { + if (!existing || this.isHeadlessBuiltMobileSessionPublicationBase(existing.publicationEpoch)) { + return + } + const worktreeId = existing.worktree + const session = this.getWorkspaceSessionForWorktree(worktreeId) + const persistedTabs = session?.tabsByWorktree?.[worktreeId] + if (!session || !persistedTabs) { + return + } + const persistedTabsById = new Map(persistedTabs.map((tab) => [tab.id, tab])) + const incomingIdentityKeys = new Set( + incoming.tabs.flatMap((tab) => this.getMobileSessionSnapshotTabIdentityKeys(tab)) + ) + for (const tab of existing.tabs) { + if (tab.type !== 'terminal') { + continue + } + if ( + this.pendingMobileTerminalCreatesByKey.has(`${worktreeId}::${tab.parentTabId}`) || + this.getMobileSessionSnapshotTabIdentityKeys(tab).some((id) => + incomingIdentityKeys.has(id) + ) || + !this.hasLiveRuntimeSessionOwnedPtyBinding(worktreeId, tab) + ) { + continue + } + const persistedParent = persistedTabsById.get(tab.parentTabId) + if (!persistedParent) { + this.clearRuntimeSessionOwnershipForMobileTab(worktreeId, existing, tab.parentTabId) + continue + } + // Why: a split parent outlives its retired leaf, so releasing the parent's + // PTYs would retire the surviving sibling with it. + if (!this.persistedParentStillBindsMobileTerminalLeaf(session, persistedParent, tab)) { + this.clearRuntimeSessionOwnershipForMobileTerminalLeaf(worktreeId, tab) + } + } + } + // Why: a tab needs authoritative runtime teardown (kill + de-persist + prune) // only when the renderer can't durably tear it down: either it's serve/SSH // (preserved + re-hydrated, would resurrect) or the renderer graph never @@ -32996,6 +33090,7 @@ export class OrcaRuntimeService { this.reconcileNativeChatLaunchDraftResolutionTombstones(snapshot) const launchDraftFencedSnapshot = this.applyNativeChatLaunchDraftResolutionFence(snapshot) const fencedSnapshot = this.applyMobileSessionRetirementFences(launchDraftFencedSnapshot) + this.releaseRuntimeSessionOwnershipForRendererRetiredTabs(fencedSnapshot, existing) const nextSnapshot = this.mergePreservedHeadlessMobileSessionTabs(fencedSnapshot, existing) // Why: clients drop same-epoch frames whose version isn't strictly newer, // and main-local touches may already have emitted a higher version than diff --git a/src/renderer/src/components/terminal-pane/pty-connection-test-pane-fixtures.ts b/src/renderer/src/components/terminal-pane/pty-connection-test-pane-fixtures.ts index 3d720043ad8..acca657bfd7 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-test-pane-fixtures.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-test-pane-fixtures.ts @@ -11,6 +11,7 @@ export function leafIdForPane(paneId: number): string { export type ConnectCallbacks = { onReattachDetermined?: () => void onConnect?: () => void + onStreamRecovered?: () => void onData?: ( data: string, meta?: { seq?: number; rawLength?: number; background?: boolean; droppedOutput?: boolean } diff --git a/src/renderer/src/components/terminal-pane/pty-connection-visible-pane-output-pause-latch.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-visible-pane-output-pause-latch.test.ts new file mode 100644 index 00000000000..8cf68aa4188 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-visible-pane-output-pause-latch.test.ts @@ -0,0 +1,243 @@ +/** + * STA-4593 incident: typing into a visible remote Codex pane produced no + * visible output until the user switched workspaces away and back. The host + * DROPS output while a stream is paused yet keeps accepting input, and every + * hide→show cycle heals the pane with an unconditional snapshot repaint — + * which is why the switch "fixed" it and why nothing ever logged an error. + * + * Invariant under test: a VISIBLE pane must never be left with a paused output + * intent. The pause bit must be re-derived from live visibility whenever the + * chance to sync it was missed. + * + * Causal boundary: syncHiddenRendererPtyDelivery issues setOutputPaused only + * when canUseHiddenOutputSnapshot(ptyId) — which requires + * transport.getPtyId() === ptyId. During a stream rebind getPtyId() is + * transiently null, so a reveal that lands in that window is a silent no-op: + * desiredOutputPaused stays true, the transport replays it verbatim onto the + * recovered stream, the host drops all output, and the stall watchdog never + * arms on a paused stream. Nothing re-derives the bit on onConnect or + * onStreamRecovered. + * + * Rig: the hidden-backlog-snapshot suite's mock-transport harness; fake + * visibility flips drive the real connectPanePty binding. + */ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { flushAsyncTicks } from './pty-connection-test-async' +import { + createMockTransport, + createPane, + createManager, + type ConnectCallbacks, + type MockTransport +} from './pty-connection-test-pane-fixtures' +import { buildPaneConnectionDeps } from './pty-connection-test-deps' +import { createInitialStoreState } from './pty-connection-test-store-fixtures' +import type { StoreState } from './pty-connection-test-store-state' +import { + installTerminalTestGlobals, + restoreTerminalTestGlobals +} from './pty-connection-test-environment' + +const { scheduleRuntimeGraphSync, shouldSeedCacheTimerOnInitialTitle } = vi.hoisted(() => ({ + scheduleRuntimeGraphSync: vi.fn(), + shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false) +})) + +let mockStoreState: StoreState +let transportFactoryQueue: MockTransport[] = [] +let storeSubscribers: ((state: StoreState) => void)[] = [] + +vi.mock('@/runtime/sync-runtime-graph', () => ({ + scheduleRuntimeGraphSync +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: StoreState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) + +vi.mock('@/lib/agent-status', async (importOriginal) => { + const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment') + return buildAgentStatusModuleMock(await importOriginal>()) +}) + +vi.mock('./cache-timer-seeding', () => ({ + shouldSeedCacheTimerOnInitialTitle +})) + +vi.mock('react', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useCallback: unknown>(fn: T): T => fn + } +}) + +vi.mock('./pty-transport', () => ({ + createIpcPtyTransport: vi.fn(() => { + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + }) +})) + +vi.mock('./remote-runtime-pty-transport', () => ({ + createRemoteRuntimePtyTransport: vi.fn(() => { + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + }) +})) + +vi.mock('./pty-dispatcher', async (importOriginal) => { + const actual = await importOriginal>() + return { + ...actual, + getEagerPtyBufferHandle: vi.fn(() => undefined) + } +}) + +const REMOTE_PTY_ID = 'remote:env-1@@terminal-1' + +function createDeps(overrides: Record = {}) { + return buildPaneConnectionDeps(() => mockStoreState, overrides) +} + +type Binding = { syncProcessTracking: () => void; dispose: () => void } + +async function connectVisibleRemotePane(): Promise<{ + transport: MockTransport + binding: Binding + deps: ReturnType + callbacks: () => ConnectCallbacks + pauseCalls: () => boolean[] +}> { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport(REMOTE_PTY_ID) + transport.setOutputPaused = vi.fn(() => true) + transport.serializeBuffer = vi.fn() + const captured: { callbacks: ConnectCallbacks | null } = { callbacks: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + captured.callbacks = callbacks + return REMOTE_PTY_ID + }) + transportFactoryQueue.push(transport) + const deps = createDeps({ isVisibleRef: { current: true } }) + const binding = connectPanePty( + createPane(1) as never, + createManager(1) as never, + deps as never + ) as Binding + await flushAsyncTicks(6) + expect(captured.callbacks).not.toBeNull() + return { + transport, + binding, + deps, + callbacks: () => captured.callbacks!, + pauseCalls: () => + (transport.setOutputPaused as ReturnType).mock.calls.map( + ([paused]) => paused as boolean + ) + } +} + +describe('remote pane output pause vs visibility', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + installTerminalTestGlobals() + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + }) + + it('control: a hide/show cycle with a stable stream ends unpaused', async () => { + const { binding, deps, pauseCalls } = await connectVisibleRemotePane() + + ;(deps.isVisibleRef as { current: boolean }).current = false + binding.syncProcessTracking() + expect(pauseCalls().at(-1)).toBe(true) + ;(deps.isVisibleRef as { current: boolean }).current = true + binding.syncProcessTracking() + + expect(pauseCalls().at(-1)).toBe(false) + binding.dispose() + }) + + it('red: a reveal that lands mid-rebind must still end with the visible pane unpaused', async () => { + const { transport, binding, deps, callbacks, pauseCalls } = await connectVisibleRemotePane() + + // Hide the pane: the transport is asked to pause host output. + ;(deps.isVisibleRef as { current: boolean }).current = false + binding.syncProcessTracking() + expect(pauseCalls().at(-1)).toBe(true) + + // The stream drops and a resubscribe is in flight: the transport has no + // bound PTY id during the rebind window. + transport.getPtyId.mockImplementation(() => null) + + // The user switches back while the rebind is in flight. The sync cannot + // reach the host right now — the defect is that nothing ever retries it. + ;(deps.isVisibleRef as { current: boolean }).current = true + binding.syncProcessTracking() + + // The rebind completes: the transport re-derives the same pane and reports + // the recovered stream. Production replays desiredOutputPaused onto the new + // stream verbatim, so unless this moment re-derives pause-from-visibility, + // the host keeps dropping every byte for a pane the user is looking at — + // while its input keeps being accepted, and the paused stream never arms + // the stall watchdog. + transport.getPtyId.mockImplementation(() => REMOTE_PTY_ID) + callbacks().onStreamRecovered?.() + callbacks().onConnect?.() + await flushAsyncTicks(6) + + expect( + pauseCalls().at(-1), + `a visible pane was left output-paused after a mid-rebind reveal (setOutputPaused calls: ${JSON.stringify(pauseCalls())})` + ).toBe(false) + binding.dispose() + }) + + it('red: a content-bearing recovery fires only onConnect, which must re-derive the pause bit alone', async () => { + const { transport, binding, deps, callbacks, pauseCalls } = await connectVisibleRemotePane() + + ;(deps.isVisibleRef as { current: boolean }).current = false + binding.syncProcessTracking() + expect(pauseCalls().at(-1)).toBe(true) + + transport.getPtyId.mockImplementation(() => null) + ;(deps.isVisibleRef as { current: boolean }).current = true + binding.syncProcessTracking() + + // When the recovery subscribe's push snapshot carried content — the common + // case — the transport fires ONLY onConnect (onStreamRecovered is gated on + // an empty snapshot). onConnect must therefore re-derive the bit by itself. + transport.getPtyId.mockImplementation(() => REMOTE_PTY_ID) + callbacks().onConnect?.() + await flushAsyncTicks(6) + + expect( + pauseCalls().at(-1), + `onConnect alone did not unpause a visible pane (setOutputPaused calls: ${JSON.stringify(pauseCalls())})` + ).toBe(false) + binding.dispose() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index abcebbdc417..abfd27b9ba8 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -5848,10 +5848,15 @@ export function connectPanePty( onConnect: (): void => { if (isCurrent()) { reportRemoteRendererSerializerReady() + // Why: a visibility flip during a rebind can't reach the host (no bound + // pty id), and the transport replays the stale pause bit onto the new + // stream — re-derive it from live visibility once the stream is bound. + syncHiddenRendererPtyDelivery() } }, onStreamRecovered: (): void => { if (isCurrent()) { + syncHiddenRendererPtyDelivery() markHiddenOutputRestoreNeeded() } }, diff --git a/src/renderer/src/runtime/web-session-tabs-sync-host-tab-retraction-ghost-rows.test.ts b/src/renderer/src/runtime/web-session-tabs-sync-host-tab-retraction-ghost-rows.test.ts new file mode 100644 index 00000000000..79d15c68fe8 --- /dev/null +++ b/src/renderer/src/runtime/web-session-tabs-sync-host-tab-retraction-ghost-rows.test.ts @@ -0,0 +1,734 @@ +/** + * STA-4593 incident: a paired client accumulated ~18 sidebar agent rows while the + * host reported 4 agent records. The host had closed (retracted) the reviewer + * tabs; every retraction stranded the client's agent-status row. + * + * Invariant under test: when a host snapshot retracts a mirrored terminal tab, + * the client owes that tab the same renderer-state sweep a local closeTab runs — + * no live agentStatusByPaneKey row, no retainedAgentsByPaneKey promotion, no + * sidebar row for a tab that no longer exists anywhere. + * + * Causal boundaries pinned here (both in the paired apply path): + * - Mechanism A: buildMirroredAgentStatusPatch's delete loop skips client-owned + * pane keys (web-session-tabs-sync.ts, ownership gate), and a retracted tab's + * pane keys are only visitable for exactly one snapshot — so a client-owned + * 'working' row outlives its tab forever and renders via the + * worktree-attributed sidebar fallback. + * - Mechanism C: nothing on the paired path plants + * recentlyClosedAgentStatusTabIds, so the retention sync promotes the + * vanished 'done' row into retainedAgentsByPaneKey permanently. + * + * Everything is injected at the seam: real snapshot mirror, real retention sync + * logic, real sidebar row builder, fake clocks. Time is never the oracle. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + RuntimeMobileSessionTabsRemovedResult, + RuntimeMobileSessionTabsResult +} from '../../../shared/runtime-types' +import { makePaneKey } from '../../../shared/stable-pane-id' +import { toWebTerminalSurfaceTabId } from '../../../shared/terminal-surface-id' +import { getDefaultSettings } from '../../../shared/constants' +import type { AppState } from '../store/types' +import { createTestStore, makeWorktree, seedStore } from '../store/slices/store-test-helpers' +import { + markRendererOwnedAgentStatusWrite, + registerRendererOwnedAgentStatusPane, + resetRendererOwnedAgentStatusPanesForTests +} from '../components/terminal-pane/renderer-owned-agent-status-registry' +import { + applyFreshWebSessionTabsSnapshot, + applyFreshWebSessionTabsSnapshots, + resetWebSessionTabsSnapshotFreshnessForTests +} from './web-session-tabs-sync' +import { + buildRetainedAgentsSyncSnapshot, + collectRetainedAgentsOnDisappear +} from '../components/dashboard/useRetainedAgents' +import { buildWorktreeAgentRows } from '../components/sidebar/worktree-agent-rows' +import { + selectLiveAgentStatusEntriesForWorktree, + selectRetainedAgentEntriesForWorktree +} from '../components/sidebar/worktree-agent-row-selectors' +import { + selectLivePtyIdsForWorktree, + selectRuntimePaneTitlesForWorktree +} from '../components/sidebar/worktree-card-status-inputs' + +// Why: web-session-tabs-sync imports the app-level store singleton; this +// harness drives a createTestStore instance instead, like its sibling suites. +vi.mock('../store', () => ({ + useAppStore: { + setState: vi.fn(), + getState: vi.fn(() => ({})), + subscribe: vi.fn(() => () => {}) + } +})) + +const WT = 'repo1::/path/wt1' +const ENV = 'web-env-1' +const HOST_EPOCH = 'host-epoch-1' +const T0 = 1_700_000_000_000 + +const KEEP_TAB = 'host-tab-keep' +const RETRACTED_TAB = 'host-tab-reviewer' +const KEEP_LEAF = '11111111-1111-4111-8111-111111111111' +const RETRACTED_LEAF = '22222222-2222-4222-8222-222222222222' + +type TestStore = ReturnType +type RetainedSyncAgents = ReturnType['currentAgents'] + +function mirrorTabId(hostTabId: string): string { + return toWebTerminalSurfaceTabId(hostTabId) +} + +function mirrorPaneKey(hostTabId: string, leafId: string): string { + return makePaneKey(mirrorTabId(hostTabId), leafId) +} + +const GHOST_PANE_KEY = mirrorPaneKey(RETRACTED_TAB, RETRACTED_LEAF) + +/** hostAgentStatus=true models a host-hook pane; otherwise status comes from client bytes. */ +function makeHostSnapshot(args: { + snapshotVersion: number + hostTabIds: readonly string[] + hostNow: number + hostAgentStatusTabIds?: readonly string[] + hostAgentStatusState?: 'working' | 'done' +}): RuntimeMobileSessionTabsResult { + const leafByTab: Record = { + [KEEP_TAB]: KEEP_LEAF, + [RETRACTED_TAB]: RETRACTED_LEAF + } + return { + worktree: WT, + publicationEpoch: HOST_EPOCH, + snapshotVersion: args.snapshotVersion, + activeGroupId: 'host-group-1', + activeTabId: `${args.hostTabIds[0]}::${leafByTab[args.hostTabIds[0]!]}`, + activeTabType: 'terminal', + tabs: args.hostTabIds.map((hostTabId, index) => ({ + type: 'terminal' as const, + id: `${hostTabId}::${leafByTab[hostTabId]}`, + title: hostTabId === RETRACTED_TAB ? 'Review origin/main' : 'Codex', + parentTabId: hostTabId, + leafId: leafByTab[hostTabId]!, + isActive: index === 0, + launchAgent: 'codex', + status: 'ready' as const, + terminal: `terminal-${index + 1}`, + ...(args.hostAgentStatusTabIds?.includes(hostTabId) + ? { + agentStatus: { + state: args.hostAgentStatusState ?? ('working' as const), + prompt: `work on ${hostTabId}`, + updatedAt: args.hostNow, + stateStartedAt: args.hostNow - 60_000, + agentType: 'codex', + paneKey: makePaneKey(hostTabId, leafByTab[hostTabId]!), + tabId: hostTabId, + worktreeId: WT, + stateHistory: [] + } + } + : {}) + })) + } +} + +/** The synthetic tombstone buildMissingWebSessionTabsRemovals publishes when an + * environment stops listing a worktree — it empties the whole worktree mirror, + * including tabs a still-live sibling environment publishes. The host's own + * notifyMobileSessionTabsRemoved tombstone differs only in epoch shape. */ +function makeWorktreeRemovalFrame( + publicationEpoch = 'visibility-inventory-removal' +): RuntimeMobileSessionTabsRemovedResult { + return { + worktree: WT, + publicationEpoch, + snapshotVersion: 0, + removed: true, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabs: [] + } +} + +/** Mirrors applyWebSessionTabsStorePatch: build the patch from live state, then set it. */ +function applyHostSnapshot( + store: TestStore, + snapshot: RuntimeMobileSessionTabsResult, + now: number, + opts?: { expectChange?: boolean } +): void { + vi.setSystemTime(now) + const state = store.getState() + const patch = applyFreshWebSessionTabsSnapshot(state, snapshot, ENV, now) + if (opts?.expectChange !== false) { + expect(patch, 'host snapshot must pass the freshness gate').not.toBe(state) + } + store.setState(patch as Partial) +} + +/** + * Byte-identical replay of pty-connection for a paired-runtime pane + * (shouldOwnAgentStatusInRenderer = runtimeEnvironmentId !== null): claim the + * pane at transport creation, prove it on each byte-derived write. Returns the + * release that pty-connection's unmount disposer holds. + */ +function replayClientByteStatus( + store: TestStore, + hostTabId: string, + leafId: string, + state: 'working' | 'done', + clientNow: number +): () => void { + vi.setSystemTime(clientNow) + const paneKey = mirrorPaneKey(hostTabId, leafId) + const release = registerRendererOwnedAgentStatusPane(paneKey, ENV) + markRendererOwnedAgentStatusWrite(paneKey) + store + .getState() + .setAgentStatus( + paneKey, + { state, prompt: `review on ${hostTabId}`, agentType: 'codex' }, + 'codex', + undefined, + { + tabId: mirrorTabId(hostTabId), + worktreeId: WT + } + ) + return release +} + +/** Byte-identical replay of useRetainedAgentsSync's effect body, minus React. */ +function replayRetainedAgentsSync( + store: TestStore, + previousAgents: RetainedSyncAgents, + now: number +): RetainedSyncAgents { + const state = store.getState() + const { currentAgents, existingWorktreeIds, tabIndex } = buildRetainedAgentsSyncSnapshot({ + repos: state.repos, + worktreesByRepo: state.worktreesByRepo, + folderWorkspaces: state.folderWorkspaces, + tabsByWorktree: state.tabsByWorktree, + agentStatusByPaneKey: state.agentStatusByPaneKey, + now + }) + const { toRetain, consumedSuppressedPaneKeys } = collectRetainedAgentsOnDisappear({ + previousAgents, + currentAgents, + retainedAgentsByPaneKey: state.retainedAgentsByPaneKey, + retentionSuppressedPaneKeys: state.retentionSuppressedPaneKeys, + recentlyClosedAgentStatusTabIds: state.recentlyClosedAgentStatusTabIds, + recentlyRetiredAgentStatusPaneKeys: state.recentlyRetiredAgentStatusPaneKeys, + tabIndex + }) + store.getState().retainAgents(toRetain) + store.getState().pruneRetainedAgents(existingWorktreeIds) + if (consumedSuppressedPaneKeys.length > 0) { + store.getState().clearRetentionSuppressedPaneKeys(consumedSuppressedPaneKeys) + } + return currentAgents +} + +function seedPairedClientStore(): TestStore { + const store = createTestStore() + seedStore(store, { + settings: { ...getDefaultSettings('/tmp'), tabAutoGenerateTitle: true }, + worktreesByRepo: { repo1: [makeWorktree({ id: WT, repoId: 'repo1', path: '/path/wt1' })] }, + activeWorktreeId: WT + } as Partial) + return store +} + +type SidebarObservation = { + tabIds: string[] + rowPaneKeys: string[] + retainedPaneKeys: string[] +} + +/** Byte-identical to useWorktreeAgentRows' inputs, minus React. */ +function observeSidebar(store: TestStore, now: number): SidebarObservation { + const state = store.getState() + const tabs = state.tabsByWorktree[WT] ?? [] + const rows = buildWorktreeAgentRows({ + tabs, + entries: selectLiveAgentStatusEntriesForWorktree(state, WT), + retained: selectRetainedAgentEntriesForWorktree(state, WT), + runtimePaneTitlesByTabId: selectRuntimePaneTitlesForWorktree(state, WT), + ptyIdsByTabId: selectLivePtyIdsForWorktree(state, WT), + terminalLayoutsByTabId: Object.fromEntries( + tabs.map((tab) => [tab.id, state.terminalLayoutsByTabId[tab.id]]) + ), + now + }) + return { + tabIds: tabs.map((tab) => tab.id), + rowPaneKeys: rows.filter((row) => row.rowSource !== 'subagent').map((row) => row.paneKey), + retainedPaneKeys: Object.keys(state.retainedAgentsByPaneKey) + } +} + +describe('a host-retracted paired tab leaves no ghost agent row behind', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(T0) + resetWebSessionTabsSnapshotFreshnessForTests() + resetRendererOwnedAgentStatusPanesForTests() + }) + + afterEach(() => { + vi.useRealTimers() + resetRendererOwnedAgentStatusPanesForTests() + }) + + it('control: retraction sweeps a host-authoritative pane the client never owned', () => { + const store = seedPairedClientStore() + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 1, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 - 1_000, + hostAgentStatusTabIds: [KEEP_TAB, RETRACTED_TAB] + }), + T0 + ) + expect(store.getState().agentStatusByPaneKey[GHOST_PANE_KEY]).toBeDefined() + // A stranded migration entry renders the same ghost sidebar row through the + // migration path, so the sweep owes it the same retirement. + for (const [ptyId, hostTabId, leafId] of [ + ['pty-migration-ghost', RETRACTED_TAB, RETRACTED_LEAF], + ['pty-migration-keep', KEEP_TAB, KEEP_LEAF] + ] as const) { + store.getState().setMigrationUnsupportedPty({ + ptyId, + paneKey: mirrorPaneKey(hostTabId, leafId), + tabId: mirrorTabId(hostTabId), + worktreeId: WT, + reason: 'legacy-numeric-pane-key', + source: 'local', + updatedAt: T0 + }) + } + + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 2, + hostTabIds: [KEEP_TAB], + hostNow: T0 + 1_000, + hostAgentStatusTabIds: [KEEP_TAB] + }), + T0 + 2_000 + ) + + expect(store.getState().agentStatusByPaneKey[GHOST_PANE_KEY]).toBeUndefined() + expect(store.getState().migrationUnsupportedByPtyId['pty-migration-ghost']).toBeUndefined() + // Negative safety: the surviving tab keeps its row and its migration entry. + expect(store.getState().agentStatusByPaneKey[mirrorPaneKey(KEEP_TAB, KEEP_LEAF)]).toBeDefined() + expect(store.getState().migrationUnsupportedByPtyId['pty-migration-keep']).toBeDefined() + }) + + it('Mechanism A: a client-owned working row must not outlive its retracted tab', () => { + const store = seedPairedClientStore() + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 1, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 - 1_000 + }), + T0 + ) + // The reviewer pane's status is written from parsed bytes; on a paired + // runtime every such pane registers renderer ownership. + const release = replayClientByteStatus(store, RETRACTED_TAB, RETRACTED_LEAF, 'working', T0) + expect(store.getState().agentStatusByPaneKey[GHOST_PANE_KEY]?.state).toBe('working') + + // Host closes the reviewer tab (e.g. `orca terminal` lifecycle on the host). + applyHostSnapshot( + store, + makeHostSnapshot({ snapshotVersion: 2, hostTabIds: [KEEP_TAB], hostNow: T0 + 1_000 }), + T0 + 2_000 + ) + // Production ordering: the pane's unmount disposer releases ownership only + // AFTER the store set that removed the tab. + release() + // Any later host activity: the retracted tab's pane keys are no longer + // reachable by the mirror's delete loop. This snapshot repeats the previous + // one verbatim, so an empty patch is the correct apply — with or without the + // sweep — and only the pane keys' unreachability is under test here. + applyHostSnapshot( + store, + makeHostSnapshot({ snapshotVersion: 3, hostTabIds: [KEEP_TAB], hostNow: T0 + 3_000 }), + T0 + 4_000, + { expectChange: false } + ) + + const observed = observeSidebar(store, T0 + 4_000) + const evidence = JSON.stringify( + { observed, ghost: store.getState().agentStatusByPaneKey[GHOST_PANE_KEY] }, + null, + 2 + ) + // Two independent signals: the mirrored tab inventory dropped the tab... + expect(observed.tabIds, evidence).toEqual([mirrorTabId(KEEP_TAB)]) + // ...so no live status row may survive under it, and no sidebar row may render for it. + expect( + store.getState().agentStatusByPaneKey[GHOST_PANE_KEY], + `a live agent row outlived the tab the host retracted\n${evidence}` + ).toBeUndefined() + expect( + observed.rowPaneKeys.filter((paneKey) => paneKey === GHOST_PANE_KEY), + `the sidebar renders an agent row for a tab that no longer exists\n${evidence}` + ).toEqual([]) + }) + + it('Mechanism C: a done agent whose tab the host retracted must not become a permanent retained row', () => { + const store = seedPairedClientStore() + let previousAgents: RetainedSyncAgents = new Map() + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 1, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 - 1_000 + }), + T0 + ) + const release = replayClientByteStatus(store, RETRACTED_TAB, RETRACTED_LEAF, 'done', T0) + // The retention sync observed the done row while its tab was still live. + previousAgents = replayRetainedAgentsSync(store, previousAgents, T0 + 500) + expect([...previousAgents.keys()]).toContain(GHOST_PANE_KEY) + + applyHostSnapshot( + store, + makeHostSnapshot({ snapshotVersion: 2, hostTabIds: [KEEP_TAB], hostNow: T0 + 1_000 }), + T0 + 2_000 + ) + // The named half of the contract: the paired retraction plants the same + // closed-tab marker closeTab plants, not just any retention suppressor. + expect( + store.getState().recentlyClosedAgentStatusTabIds[mirrorTabId(RETRACTED_TAB)], + 'the paired retraction did not plant the closed-tab suppressor' + ).toBeDefined() + release() + // The tab's disappearance triggers the next retention sync tick. + previousAgents = replayRetainedAgentsSync(store, previousAgents, T0 + 2_500) + + const observed = observeSidebar(store, T0 + 3_000) + const evidence = JSON.stringify( + { observed, retained: store.getState().retainedAgentsByPaneKey[GHOST_PANE_KEY] }, + null, + 2 + ) + // A host retraction is a close; closes suppress retention. The paired path + // must plant the same closed-tab marker closeTab plants. + expect( + store.getState().retainedAgentsByPaneKey[GHOST_PANE_KEY], + `a host-retracted done agent was promoted into permanent retained state\n${evidence}` + ).toBeUndefined() + expect( + observed.rowPaneKeys.filter((paneKey) => paneKey === GHOST_PANE_KEY), + `the sidebar renders a retained ghost row for a tab that no longer exists\n${evidence}` + ).toEqual([]) + }) + + it('control: the retention suppressor works when the closed-tab marker is planted', () => { + const store = seedPairedClientStore() + let previousAgents: RetainedSyncAgents = new Map() + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 1, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 - 1_000 + }), + T0 + ) + const release = replayClientByteStatus(store, RETRACTED_TAB, RETRACTED_LEAF, 'done', T0) + previousAgents = replayRetainedAgentsSync(store, previousAgents, T0 + 500) + + // The local close path's suppressor-aware removal: sweeps the row AND + // plants recentlyClosedAgentStatusTabIds so retention cannot resurrect it. + store.getState().dropAgentStatusByTabPrefix(mirrorTabId(RETRACTED_TAB), { worktreeId: WT }) + applyHostSnapshot( + store, + makeHostSnapshot({ snapshotVersion: 2, hostTabIds: [KEEP_TAB], hostNow: T0 + 1_000 }), + T0 + 2_000 + ) + release() + previousAgents = replayRetainedAgentsSync(store, previousAgents, T0 + 2_500) + + expect(store.getState().retainedAgentsByPaneKey[GHOST_PANE_KEY]).toBeUndefined() + expect(store.getState().agentStatusByPaneKey[GHOST_PANE_KEY]).toBeUndefined() + }) + + it('a worktree tombstone frame must not sweep a client-owned row it merely un-mirrors', () => { + const store = seedPairedClientStore() + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 1, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 - 1_000 + }), + T0 + ) + const release = replayClientByteStatus(store, RETRACTED_TAB, RETRACTED_LEAF, 'working', T0) + expect(store.getState().agentStatusByPaneKey[GHOST_PANE_KEY]?.state).toBe('working') + + // One environment stops listing the worktree. The tombstone clears the whole + // mirror, but it is not a host close: the tab is still open on the host, and + // another paired environment may still be publishing it. + applyHostSnapshot(store, makeWorktreeRemovalFrame(), T0 + 1_000) + expect(store.getState().tabsByWorktree[WT] ?? []).toEqual([]) + + expect( + store.getState().agentStatusByPaneKey[GHOST_PANE_KEY], + 'a tombstone frame swept a client-owned row whose tab is still open on the host' + ).toBeDefined() + expect( + store.getState().recentlyClosedAgentStatusTabIds[mirrorTabId(RETRACTED_TAB)], + 'a tombstone frame planted a close suppressor for a tab nobody closed' + ).toBeUndefined() + + // The worktree comes back on the next real publication: the tab returns, and + // its client-owned status row must return with it (STA-3107 status authority). + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 2, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 + 2_000 + }), + T0 + 3_000 + ) + expect(store.getState().tabsByWorktree[WT]?.map((tab) => tab.id)).toContain( + mirrorTabId(RETRACTED_TAB) + ) + expect(store.getState().agentStatusByPaneKey[GHOST_PANE_KEY]?.state).toBe('working') + release() + }) + + it("the host's own removal tombstone (removed:) must not sweep either", () => { + const store = seedPairedClientStore() + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 1, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 - 1_000 + }), + T0 + ) + const release = replayClientByteStatus(store, RETRACTED_TAB, RETRACTED_LEAF, 'working', T0) + + // notifyMobileSessionTabsRemoved publishes removed:true under a + // removed: epoch — only the `removed` flag identifies it. + applyHostSnapshot(store, makeWorktreeRemovalFrame('removed:k7q2xz'), T0 + 1_000) + expect(store.getState().tabsByWorktree[WT] ?? []).toEqual([]) + + expect( + store.getState().agentStatusByPaneKey[GHOST_PANE_KEY], + 'a host removal tombstone swept a client-owned row it merely un-mirrors' + ).toBeDefined() + expect( + store.getState().recentlyClosedAgentStatusTabIds[mirrorTabId(RETRACTED_TAB)], + 'a host removal tombstone planted a close suppressor for a tab nobody closed' + ).toBeUndefined() + release() + }) + + it('a re-mirrored tab id regains its client-owned status channel', () => { + const store = seedPairedClientStore() + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 1, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 - 1_000 + }), + T0 + ) + const release = replayClientByteStatus(store, RETRACTED_TAB, RETRACTED_LEAF, 'working', T0) + + // A transient omission — host restart subset frame, cross-host collision + // replacement — retracts the tab and plants the closed-tab marker. + applyHostSnapshot( + store, + makeHostSnapshot({ snapshotVersion: 2, hostTabIds: [KEEP_TAB], hostNow: T0 + 1_000 }), + T0 + 2_000 + ) + release() + expect( + store.getState().recentlyClosedAgentStatusTabIds[mirrorTabId(RETRACTED_TAB)] + ).toBeDefined() + + // The host publishes the same tab id again (post-restart republication or + // the collision repair replay). Mirrored ids are stable, and the close-intent + // filter holds genuinely closing tabs out of snapshots — so presence is + // authoritative: the marker must lift, or the returning tab is + // agent-status-dead for the rest of the session. + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 3, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 + 3_000 + }), + T0 + 4_000 + ) + expect(store.getState().tabsByWorktree[WT]?.map((tab) => tab.id)).toContain( + mirrorTabId(RETRACTED_TAB) + ) + expect( + store.getState().recentlyClosedAgentStatusTabIds[mirrorTabId(RETRACTED_TAB)], + 'a re-mirrored tab id kept its closed-tab marker' + ).toBeUndefined() + + // The returning pane's byte-derived status must land again. + const release2 = replayClientByteStatus( + store, + RETRACTED_TAB, + RETRACTED_LEAF, + 'working', + T0 + 4_500 + ) + expect( + store.getState().agentStatusByPaneKey[GHOST_PANE_KEY]?.state, + 'a returning mirrored tab was left permanently unable to acquire agent status' + ).toBe('working') + release2() + }) + + it('a retract-and-return flap must not strand a suppressor that eats a later real retention', () => { + const store = seedPairedClientStore() + let previousAgents: RetainedSyncAgents = new Map() + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 1, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 - 1_000 + }), + T0 + ) + const release = replayClientByteStatus(store, RETRACTED_TAB, RETRACTED_LEAF, 'working', T0) + previousAgents = replayRetainedAgentsSync(store, previousAgents, T0 + 500) + + // The flap: a transient retraction and the return land inside ONE retention + // interval (React batches same-task store commits into a single effect pass). + // The pane stays mounted throughout, so the sweep found the client-owned row + // live and planted its one-shot suppressor with no disappearance to consume + // it — the pane's next live write is what must lift it (setAgentStatus's + // fresh-status suppressor lift), or it eats a later real retention. + applyHostSnapshot( + store, + makeHostSnapshot({ snapshotVersion: 2, hostTabIds: [KEEP_TAB], hostNow: T0 + 1_000 }), + T0 + 2_000 + ) + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 3, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 + 2_500 + }), + T0 + 3_000 + ) + // The still-mounted pane finishes its run cleanly and rewrites its row. + vi.setSystemTime(T0 + 3_500) + markRendererOwnedAgentStatusWrite(GHOST_PANE_KEY) + store + .getState() + .setAgentStatus( + GHOST_PANE_KEY, + { state: 'done', prompt: 'review finished', agentType: 'codex' }, + 'codex', + undefined, + { tabId: mirrorTabId(RETRACTED_TAB), worktreeId: WT } + ) + previousAgents = replayRetainedAgentsSync(store, previousAgents, T0 + 4_000) + expect(store.getState().agentStatusByPaneKey[GHOST_PANE_KEY]?.state).toBe('done') + + // Much later the pane unmounts and the host's next snapshot omits its status: + // the mirror's delete loop removes the no-longer-client-owned done row with no + // suppressor — the exact live→gone transition retention exists to catch. + release() + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 4, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 + 600_000 + }), + T0 + 601_000 + ) + expect( + store.getState().agentStatusByPaneKey[GHOST_PANE_KEY], + 'precondition: the released done row must be removed by the mirror delete loop' + ).toBeUndefined() + previousAgents = replayRetainedAgentsSync(store, previousAgents, T0 + 601_500) + + expect( + store.getState().retainedAgentsByPaneKey[GHOST_PANE_KEY], + 'a stranded retraction suppressor ate a legitimate done-agent retention' + ).toBeDefined() + }) + + it('Mechanism A holds through the batch apply path a reconnect load uses', () => { + const store = seedPairedClientStore() + applyHostSnapshot( + store, + makeHostSnapshot({ + snapshotVersion: 1, + hostTabIds: [KEEP_TAB, RETRACTED_TAB], + hostNow: T0 - 1_000 + }), + T0 + ) + const release = replayClientByteStatus(store, RETRACTED_TAB, RETRACTED_LEAF, 'working', T0) + expect(store.getState().agentStatusByPaneKey[GHOST_PANE_KEY]?.state).toBe('working') + + // A reconnect delivers everything missed as ONE batch: the retraction, then + // a later snapshot that touches agent status again — the batch's final + // record republication must not undo the sweep. + vi.setSystemTime(T0 + 2_000) + const state = store.getState() + const patch = applyFreshWebSessionTabsSnapshots( + state, + [ + makeHostSnapshot({ snapshotVersion: 2, hostTabIds: [KEEP_TAB], hostNow: T0 + 1_000 }), + makeHostSnapshot({ + snapshotVersion: 3, + hostTabIds: [KEEP_TAB], + hostNow: T0 + 1_500, + hostAgentStatusTabIds: [KEEP_TAB] + }) + ], + ENV, + T0 + 2_000 + ) + expect(patch, 'the batch must pass the freshness gate').not.toBe(state) + store.setState(patch as Partial) + release() + + const observed = observeSidebar(store, T0 + 3_000) + expect(store.getState().tabsByWorktree[WT]?.map((tab) => tab.id)).toEqual([ + mirrorTabId(KEEP_TAB) + ]) + expect( + store.getState().agentStatusByPaneKey[GHOST_PANE_KEY], + 'the batch apply left a ghost row the singular apply sweeps' + ).toBeUndefined() + expect(observed.rowPaneKeys.filter((paneKey) => paneKey === GHOST_PANE_KEY)).toEqual([]) + // Negative safety: the batch's later snapshot still landed its host row. + expect(store.getState().agentStatusByPaneKey[mirrorPaneKey(KEEP_TAB, KEEP_LEAF)]).toBeDefined() + }) +}) diff --git a/src/renderer/src/runtime/web-session-tabs-sync.ts b/src/renderer/src/runtime/web-session-tabs-sync.ts index 7642abd3f57..14028c25c38 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.ts @@ -30,6 +30,10 @@ import type { Tab, TabGroup, TabGroupLayoutNode } from '../../../shared/tab-type import type { TerminalLayoutSnapshot, TerminalTab } from '../../../shared/terminal-tab-types' import type { OpenFile } from '../store/slices/editor' import { isTerminalLeafId, makePaneKey, parsePaneKey } from '../../../shared/stable-pane-id' +import { + buildRetiredTerminalTabStateSweepPatch, + type RetiredTerminalTabSweepState +} from '../store/slices/retired-terminal-tab-state-sweep' import { getRemoteRuntimePtyEnvironmentId, toRemoteRuntimePtyId } from './runtime-terminal-stream' import { sanitizeTerminalLayoutPaneTitlesForLabels } from '@/lib/terminal-pane-title-sanitization' import { terminalLayoutEqual } from '@/lib/terminal-layout-equality' @@ -232,7 +236,21 @@ export type WebSessionTabsSyncState = Pick< | 'unreadTerminalTabs' | 'sortEpoch' > & - Partial> + Partial< + Pick< + AppState, + | 'acknowledgedAgentsByPaneKey' + | 'agentLaunchConfigByPaneKey' + | 'automaticAgentResumeClaimsByTabId' + | 'migrationUnsupportedByPtyId' + | 'paneForegroundAgentByPaneKey' + | 'pendingStartupByTabId' + | 'recentlyClosedAgentStatusTabIds' + | 'recentlyRetiredAgentStatusPaneKeys' + | 'retainedAgentsByPaneKey' + | 'retentionSuppressedPaneKeys' + > + > type WebSessionTabsBatchRecordKey = | 'activeBrowserTabIdByWorktree' @@ -464,6 +482,17 @@ function removeWebSessionTabsEnvironment(environmentId: string, worktreeId: stri } } +const VISIBILITY_INVENTORY_REMOVAL_EPOCH = 'visibility-inventory-removal' + +// Why: a tombstone empties the whole worktree mirror — including tabs a still-live sibling environment publishes — so it is a +// visibility fact, never evidence that the host closed anything. +function isWebSessionTabsWorktreeRemovalFrame(snapshot: RuntimeMobileSessionTabsResult): boolean { + return ( + (snapshot as { removed?: unknown }).removed === true || + snapshot.publicationEpoch === VISIBILITY_INVENTORY_REMOVAL_EPOCH + ) +} + // Why: omission means removal only because `listAllMobileSessionTabs` publishes every worktree it knows unfiltered; if a host ever // scopes that map, this turns live worktrees into tombstones, so the fence below is deliberately short-lived. function buildMissingWebSessionTabsRemovals( @@ -484,7 +513,7 @@ function buildMissingWebSessionTabsRemovals( trackedWorktree, snapshot: { worktree: trackedWorktree.worktree, - publicationEpoch: 'visibility-inventory-removal', + publicationEpoch: VISIBILITY_INVENTORY_REMOVAL_EPOCH, snapshotVersion: 0, removed: true, activeGroupId: null, @@ -1180,7 +1209,77 @@ function updateBatchAgentPaneKey( } } -/** Generates a state patch for mirrored agent statuses, merging host entries with client overrides. */ +// Why: the closed-tab marker has no TTL, and setAgentStatus hard-drops writes for a +// marked tab id — for a stable mirrored id that returns, that is a permanent silent +// blackhole unless presence in a snapshot lifts it. +function buildRemirroredClosedTabMarkerLiftPatch( + recentlyClosedAgentStatusTabIds: WebSessionTabsSyncState['recentlyClosedAgentStatusTabIds'], + mirroredTerminalIds: ReadonlySet +): Partial | null { + let next: WebSessionTabsSyncState['recentlyClosedAgentStatusTabIds'] | null = null + for (const tabId of mirroredTerminalIds) { + if (tabId in (recentlyClosedAgentStatusTabIds ?? {})) { + next ??= { ...recentlyClosedAgentStatusTabIds } + delete next[tabId] + } + } + return next ? { recentlyClosedAgentStatusTabIds: next } : null +} + +/** + * A host retraction owes the retracted tab closeTab's renderer-state sweep: without it, + * client-owned rows (STA-3107-exempt in the mirror's delete loop) and retention promotions + * outlive the tab forever (STA-4593). + */ +function buildRetractedMirroredTabSweepPatch( + state: WebSessionTabsSyncState, + worktreeId: string, + nextTabsByWorktree: WebSessionTabsSyncState['tabsByWorktree'], + agentStatusPatch: Pick< + WebSessionTabsSyncState, + 'agentStatusByPaneKey' | 'agentStatusEpoch' | 'sortEpoch' + > | null, + removedTerminalResourceIds: readonly string[], + batchContext?: WebSessionTabsBatchContext +): Partial | null { + // Why: only a mirrored id the host stopped publishing is a retraction — a local or provisional + // tab in this list is being renamed into its mirror, and a rename must keep its rows. + const retractedTabIds = removedTerminalResourceIds.filter(isMirroredTerminalSurfaceId) + if (retractedTabIds.length === 0) { + return null + } + const sweepState: RetiredTerminalTabSweepState = { + acknowledgedAgentsByPaneKey: state.acknowledgedAgentsByPaneKey ?? {}, + agentLaunchConfigByPaneKey: state.agentLaunchConfigByPaneKey ?? {}, + agentStatusByPaneKey: agentStatusPatch?.agentStatusByPaneKey ?? state.agentStatusByPaneKey, + agentStatusEpoch: agentStatusPatch?.agentStatusEpoch ?? state.agentStatusEpoch, + migrationUnsupportedByPtyId: state.migrationUnsupportedByPtyId ?? {}, + paneForegroundAgentByPaneKey: state.paneForegroundAgentByPaneKey ?? {}, + recentlyClosedAgentStatusTabIds: state.recentlyClosedAgentStatusTabIds ?? {}, + recentlyRetiredAgentStatusPaneKeys: state.recentlyRetiredAgentStatusPaneKeys ?? {}, + retainedAgentsByPaneKey: state.retainedAgentsByPaneKey ?? {}, + retentionSuppressedPaneKeys: state.retentionSuppressedPaneKeys ?? {}, + sortEpoch: agentStatusPatch?.sortEpoch ?? state.sortEpoch, + // Why: the drop's completed-orphan rule reads "keyed under a tab this worktree no longer has", + // so it must see the post-removal tab list, not the one the snapshot replaced. + tabsByWorktree: nextTabsByWorktree + } + const sweep = buildRetiredTerminalTabStateSweepPatch(sweepState, retractedTabIds, worktreeId) + if (!sweep?.agentStatusByPaneKey || !batchContext) { + return sweep ?? null + } + // Why: the batch republishes its own record copy at the end, which would undo the sweep. + const mutableState = state as unknown as Record + mutableState.agentStatusByPaneKey = sweep.agentStatusByPaneKey + batchContext.changedRecords.add('agentStatusByPaneKey') + for (const paneKey of Object.keys(sweepState.agentStatusByPaneKey)) { + if (!(paneKey in sweep.agentStatusByPaneKey)) { + updateBatchAgentPaneKey(paneKey, false, batchContext) + } + } + return sweep +} + function buildMirroredAgentStatusPatch( state: WebSessionTabsSyncState, currentTerminalTabs: readonly TerminalTab[], @@ -3264,9 +3363,32 @@ function applyWebSessionTabsSnapshotWithContext( now, batchContext ) + // Why: only a host snapshot that omits a tab is a retraction; a tombstone clears the mirror + // for every environment at once, and sweeping there erases rows a live sibling still owns. + const retractedTabSweepPatch = isWebSessionTabsWorktreeRemovalFrame(snapshot) + ? null + : buildRetractedMirroredTabSweepPatch( + state, + worktreeId, + nextTabsByWorktree, + agentStatusPatch, + removedTerminalResourceIds, + batchContext + ) + // Why: mirrored ids are stable, so a published-again id proves the tab is not closed — + // a lingering closed-tab marker would blackhole its byte-derived status for the whole + // session (host-restart subset frames, cross-host collision replays). The close-intent + // filter already holds genuinely closing tabs out of the snapshot. + const remirroredClosedTabLiftPatch = buildRemirroredClosedTabMarkerLiftPatch( + retractedTabSweepPatch?.recentlyClosedAgentStatusTabIds ?? + state.recentlyClosedAgentStatusTabIds, + mirroredTerminalIds + ) const patch: Partial = { ...agentStatusPatch, + ...retractedTabSweepPatch, + ...remirroredClosedTabLiftPatch, ...(nextOpenFiles !== state.openFiles ? { openFiles: nextOpenFiles } : {}), ...(nextTabsByWorktree !== state.tabsByWorktree ? { tabsByWorktree: nextTabsByWorktree } : {}), ...(nextBrowserTabsByWorktree !== state.browserTabsByWorktree diff --git a/src/renderer/src/store/slices/agent-status.ts b/src/renderer/src/store/slices/agent-status.ts index cb86af68d38..95cde4a3979 100644 --- a/src/renderer/src/store/slices/agent-status.ts +++ b/src/renderer/src/store/slices/agent-status.ts @@ -473,7 +473,7 @@ function getLeafIdFromPaneKey(paneKey: string): string | null { } function findCompletedOrphanPaneKeysForTabClose( - state: AppState, + state: AgentStatusTabPrefixDropState, worktreeId: string | undefined, prefix: string ): string[] { @@ -1295,6 +1295,143 @@ function buildAgentStatusBatchPatch( return patch as Partial } +/** The slice of app state the tab-prefix drop reduces over — narrow so callers + * outside the store (the paired snapshot apply) can build the same patch. */ +export type AgentStatusTabPrefixDropState = Pick< + AppState, + | 'acknowledgedAgentsByPaneKey' + | 'agentLaunchConfigByPaneKey' + | 'agentStatusByPaneKey' + | 'agentStatusEpoch' + | 'migrationUnsupportedByPtyId' + | 'recentlyClosedAgentStatusTabIds' + | 'recentlyRetiredAgentStatusPaneKeys' + | 'retainedAgentsByPaneKey' + | 'retentionSuppressedPaneKeys' + | 'sortEpoch' + | 'tabsByWorktree' +> + +/** Pure form of the dropAgentStatusByTabPrefix reducer: the paired snapshot + * apply folds the same sweep into a patch it assembles itself, so the two + * paths cannot drift. `retiredAliasPaneKeys` comes from the caller because + * retiring pane-authority aliases is a registry side effect, not a reduction. */ +export function buildAgentStatusTabPrefixDropPatch( + s: AgentStatusTabPrefixDropState, + tabIdPrefix: string, + retiredAliasPaneKeys: readonly string[], + opts?: DropAgentStatusByTabPrefixOptions +): { patch: Partial; hadLive: boolean } { + const prefix = `${tabIdPrefix}:` + let hadLive = false + const buildPatch = (): Partial => { + const completedOrphanKeys = findCompletedOrphanPaneKeysForTabClose(s, opts?.worktreeId, prefix) + const completedOrphanKeySet = new Set(completedOrphanKeys) + const liveKeys = [ + ...Object.keys(s.agentStatusByPaneKey).filter((k) => k.startsWith(prefix)), + ...completedOrphanKeys + ] + const launchConfigKeys = Object.keys(s.agentLaunchConfigByPaneKey).filter( + (k) => k.startsWith(prefix) || completedOrphanKeySet.has(k) + ) + const retainedKeys = Object.keys(s.retainedAgentsByPaneKey).filter( + (k) => k.startsWith(prefix) || completedOrphanKeySet.has(k) + ) + const migrationUnsupported = pruneMigrationUnsupportedEntries( + s.migrationUnsupportedByPtyId, + (entry) => entry.paneKey?.startsWith(prefix) ?? false + ) + // See removeAgentStatus for ack-cleanup rationale; ack entries are owned by the pane lifecycle regardless of live/retained state. + let nextAck = s.acknowledgedAgentsByPaneKey + const ackKeys = Object.keys(nextAck).filter( + (k) => k.startsWith(prefix) || completedOrphanKeySet.has(k) + ) + if (ackKeys.length > 0) { + nextAck = { ...nextAck } + for (const k of ackKeys) { + delete nextAck[k] + } + } + const nextClosedTabs = boundRecentlyClosedAgentStatusTabIds( + s.recentlyClosedAgentStatusTabIds, + tabIdPrefix + ) + const nextRetiredPaneKeys = boundRecentlyRetiredAgentStatusPaneKeys( + s.recentlyRetiredAgentStatusPaneKeys, + retiredAliasPaneKeys + ) + + if ( + liveKeys.length === 0 && + launchConfigKeys.length === 0 && + retainedKeys.length === 0 && + !migrationUnsupported.changed + ) { + if (nextAck !== s.acknowledgedAgentsByPaneKey) { + return { + acknowledgedAgentsByPaneKey: nextAck, + recentlyClosedAgentStatusTabIds: nextClosedTabs, + recentlyRetiredAgentStatusPaneKeys: nextRetiredPaneKeys + } + } + return { + recentlyClosedAgentStatusTabIds: nextClosedTabs, + recentlyRetiredAgentStatusPaneKeys: nextRetiredPaneKeys + } + } + hadLive = liveKeys.length > 0 + + const nextLive = liveKeys.length > 0 ? { ...s.agentStatusByPaneKey } : s.agentStatusByPaneKey + for (const key of liveKeys) { + delete nextLive[key] + } + const nextLaunchConfigs = + launchConfigKeys.length > 0 + ? { ...s.agentLaunchConfigByPaneKey } + : s.agentLaunchConfigByPaneKey + for (const key of launchConfigKeys) { + delete nextLaunchConfigs[key] + } + + const nextRetained = + retainedKeys.length > 0 ? { ...s.retainedAgentsByPaneKey } : s.retainedAgentsByPaneKey + for (const key of retainedKeys) { + delete nextRetained[key] + } + + // Why: a suppressor is only consumed on a live→gone transition, so plant one only for live paneKeys and skip already-suppressed and completed-orphan keys — otherwise it leaks (mirrors dropAgentStatus). + const suppressorAdds = liveKeys.filter( + (k) => !completedOrphanKeySet.has(k) && !(k in s.retentionSuppressedPaneKeys) + ) + let nextRetentionSuppressedPaneKeys = s.retentionSuppressedPaneKeys + if (suppressorAdds.length > 0) { + nextRetentionSuppressedPaneKeys = { ...s.retentionSuppressedPaneKeys } + for (const key of suppressorAdds) { + nextRetentionSuppressedPaneKeys[key] = true + } + } + + return { + agentStatusByPaneKey: nextLive, + agentLaunchConfigByPaneKey: nextLaunchConfigs, + retainedAgentsByPaneKey: nextRetained, + migrationUnsupportedByPtyId: migrationUnsupported.next, + retentionSuppressedPaneKeys: nextRetentionSuppressedPaneKeys, + recentlyClosedAgentStatusTabIds: nextClosedTabs, + recentlyRetiredAgentStatusPaneKeys: nextRetiredPaneKeys, + ...(nextAck !== s.acknowledgedAgentsByPaneKey + ? { acknowledgedAgentsByPaneKey: nextAck } + : {}), + // Why: mirrors removeAgentStatusByTabPrefix — only bump epochs when the live map changed; retained-only sweeps don't affect sort/freshness. + agentStatusEpoch: + hadLive || migrationUnsupported.changed ? s.agentStatusEpoch + 1 : s.agentStatusEpoch, + sortEpoch: hadLive || migrationUnsupported.changed ? s.sortEpoch + 1 : s.sortEpoch + } + } + const patch = buildPatch() + return { patch, hadLive } +} + export const createAgentStatusSlice: StateCreator = ( storeSet, storeGet @@ -2722,117 +2859,17 @@ export const createAgentStatusSlice: StateCreator { - const prefix = `${tabIdPrefix}:` const retiredAliasPaneKeys = retireAgentPaneAuthorityAliasesByOwnerTab(tabIdPrefix) let hadLive = false set((s) => { - const completedOrphanKeys = findCompletedOrphanPaneKeysForTabClose( + const dropped = buildAgentStatusTabPrefixDropPatch( s, - opts?.worktreeId, - prefix + tabIdPrefix, + retiredAliasPaneKeys, + opts ) - const completedOrphanKeySet = new Set(completedOrphanKeys) - const liveKeys = [ - ...Object.keys(s.agentStatusByPaneKey).filter((k) => k.startsWith(prefix)), - ...completedOrphanKeys - ] - const launchConfigKeys = Object.keys(s.agentLaunchConfigByPaneKey).filter( - (k) => k.startsWith(prefix) || completedOrphanKeySet.has(k) - ) - const retainedKeys = Object.keys(s.retainedAgentsByPaneKey).filter( - (k) => k.startsWith(prefix) || completedOrphanKeySet.has(k) - ) - const migrationUnsupported = pruneMigrationUnsupportedEntries( - s.migrationUnsupportedByPtyId, - (entry) => entry.paneKey?.startsWith(prefix) ?? false - ) - // See removeAgentStatus for ack-cleanup rationale; ack entries are owned by the pane lifecycle regardless of live/retained state. - let nextAck = s.acknowledgedAgentsByPaneKey - const ackKeys = Object.keys(nextAck).filter( - (k) => k.startsWith(prefix) || completedOrphanKeySet.has(k) - ) - if (ackKeys.length > 0) { - nextAck = { ...nextAck } - for (const k of ackKeys) { - delete nextAck[k] - } - } - const nextClosedTabs = boundRecentlyClosedAgentStatusTabIds( - s.recentlyClosedAgentStatusTabIds, - tabIdPrefix - ) - const nextRetiredPaneKeys = boundRecentlyRetiredAgentStatusPaneKeys( - s.recentlyRetiredAgentStatusPaneKeys, - retiredAliasPaneKeys - ) - - if ( - liveKeys.length === 0 && - launchConfigKeys.length === 0 && - retainedKeys.length === 0 && - !migrationUnsupported.changed - ) { - if (nextAck !== s.acknowledgedAgentsByPaneKey) { - return { - acknowledgedAgentsByPaneKey: nextAck, - recentlyClosedAgentStatusTabIds: nextClosedTabs, - recentlyRetiredAgentStatusPaneKeys: nextRetiredPaneKeys - } - } - return { - recentlyClosedAgentStatusTabIds: nextClosedTabs, - recentlyRetiredAgentStatusPaneKeys: nextRetiredPaneKeys - } - } - hadLive = liveKeys.length > 0 - - const nextLive = - liveKeys.length > 0 ? { ...s.agentStatusByPaneKey } : s.agentStatusByPaneKey - for (const key of liveKeys) { - delete nextLive[key] - } - const nextLaunchConfigs = - launchConfigKeys.length > 0 - ? { ...s.agentLaunchConfigByPaneKey } - : s.agentLaunchConfigByPaneKey - for (const key of launchConfigKeys) { - delete nextLaunchConfigs[key] - } - - const nextRetained = - retainedKeys.length > 0 ? { ...s.retainedAgentsByPaneKey } : s.retainedAgentsByPaneKey - for (const key of retainedKeys) { - delete nextRetained[key] - } - - // Why: a suppressor is only consumed on a live→gone transition, so plant one only for live paneKeys and skip already-suppressed and completed-orphan keys — otherwise it leaks (mirrors dropAgentStatus). - const suppressorAdds = liveKeys.filter( - (k) => !completedOrphanKeySet.has(k) && !(k in s.retentionSuppressedPaneKeys) - ) - let nextRetentionSuppressedPaneKeys = s.retentionSuppressedPaneKeys - if (suppressorAdds.length > 0) { - nextRetentionSuppressedPaneKeys = { ...s.retentionSuppressedPaneKeys } - for (const key of suppressorAdds) { - nextRetentionSuppressedPaneKeys[key] = true - } - } - - return { - agentStatusByPaneKey: nextLive, - agentLaunchConfigByPaneKey: nextLaunchConfigs, - retainedAgentsByPaneKey: nextRetained, - migrationUnsupportedByPtyId: migrationUnsupported.next, - retentionSuppressedPaneKeys: nextRetentionSuppressedPaneKeys, - recentlyClosedAgentStatusTabIds: nextClosedTabs, - recentlyRetiredAgentStatusPaneKeys: nextRetiredPaneKeys, - ...(nextAck !== s.acknowledgedAgentsByPaneKey - ? { acknowledgedAgentsByPaneKey: nextAck } - : {}), - // Why: mirrors removeAgentStatusByTabPrefix — only bump epochs when the live map changed; retained-only sweeps don't affect sort/freshness. - agentStatusEpoch: - hadLive || migrationUnsupported.changed ? s.agentStatusEpoch + 1 : s.agentStatusEpoch, - sortEpoch: hadLive || migrationUnsupported.changed ? s.sortEpoch + 1 : s.sortEpoch - } + hadLive = dropped.hadLive + return dropped.patch }) if (hadLive) { queueMicrotask(() => freshness.schedule()) diff --git a/src/renderer/src/store/slices/pane-foreground-agent.ts b/src/renderer/src/store/slices/pane-foreground-agent.ts index ebdd69e6865..36c6be7c070 100644 --- a/src/renderer/src/store/slices/pane-foreground-agent.ts +++ b/src/renderer/src/store/slices/pane-foreground-agent.ts @@ -64,7 +64,12 @@ export const createPaneForegroundAgentSlice: StateCreator< }) }, clearPaneForegroundAgentByTabPrefix: (tabIdPrefix) => { - set((s) => clearEntriesByTabPrefixes(s.paneForegroundAgentByPaneKey, [`${tabIdPrefix}:`]) ?? s) + set( + (s) => + buildPaneForegroundAgentTabPrefixClearPatch(s.paneForegroundAgentByPaneKey, [ + `${tabIdPrefix}:` + ]) ?? s + ) }, clearPaneForegroundAgentByWorktree: (worktreeId) => { // Why: entries carry no worktreeId, so this must run while the worktree's @@ -72,14 +77,16 @@ export const createPaneForegroundAgentSlice: StateCreator< // awaiting terminal teardown). set((s) => { const prefixes = (s.tabsByWorktree[worktreeId] ?? []).map((tab) => `${tab.id}:`) - return clearEntriesByTabPrefixes(s.paneForegroundAgentByPaneKey, prefixes) ?? s + return ( + buildPaneForegroundAgentTabPrefixClearPatch(s.paneForegroundAgentByPaneKey, prefixes) ?? s + ) }) } }) -function clearEntriesByTabPrefixes( +export function buildPaneForegroundAgentTabPrefixClearPatch( entries: Record, - tabPrefixes: string[] + tabPrefixes: readonly string[] ): Pick | null { if (tabPrefixes.length === 0) { return null diff --git a/src/renderer/src/store/slices/retired-terminal-tab-state-sweep.ts b/src/renderer/src/store/slices/retired-terminal-tab-state-sweep.ts new file mode 100644 index 00000000000..cfdefcf87e0 --- /dev/null +++ b/src/renderer/src/store/slices/retired-terminal-tab-state-sweep.ts @@ -0,0 +1,96 @@ +import type { AppState } from '../types' +import { forgetAgentHibernationTabOutput } from '@/lib/agent-hibernation-output-activity' +import { forgetForegroundTerminalTabs } from '@/lib/foreground-terminal-tabs' +import { forgetAgentStartupDeliveriesForTabs } from '@/lib/agent-startup-delivery-guards' +// Why: the store-free registry (not terminal-parked-tab-watchers, which imports @/store) so a slice can import this module during its own evaluation. +import { retireParkedTerminalTab } from '@/components/terminal-pane/terminal-parked-watcher-registry' +import { retireAgentPaneAuthorityAliasesByOwnerTab } from './agent-pane-authority' +import { + buildAgentStatusTabPrefixDropPatch, + type AgentStatusTabPrefixDropState +} from './agent-status' +import { buildPaneForegroundAgentTabPrefixClearPatch } from './pane-foreground-agent' + +export type RetiredTerminalTabSweepActions = Pick< + AppState, + 'dropAgentStatusByTabPrefix' | 'clearPaneForegroundAgentByTabPrefix' +> + +/** The state the sweep reduces over: the two store maps plus everything the + * agent-status drop reads. Narrow so a non-store caller can pass its own view. */ +export type RetiredTerminalTabSweepState = AgentStatusTabPrefixDropState & + Pick + +/** + * The suppressor-aware store maps plus three module registries a retired terminal tab strands. + * One unit with two callers on purpose: a second inline copy of this list is how a retirement + * path ends up sweeping none of it (STA-4593). NOT full closeTab parity — closeTab's own set() + * additionally clears pane-keyed maps outside this list (sleeping agent sessions, unread + * markers, pane timers), which a caller with a narrower set() still strands. Must run AFTER the + * tab is out of `tabsByWorktree` — the completed-orphan sweep keys on "tab this worktree no + * longer has". + */ +export function sweepRetiredTerminalTabState( + actions: RetiredTerminalTabSweepActions, + tabId: string, + worktreeId?: string | null +): void { + // Why: idempotent, and retirement has no earlier hook — closeTab still revokes these before its own + // provider teardown, where the ordering against pty exit is load-bearing. + retireParkedTerminalTab(tabId) + // Why: sweep tab agent status through its suppressor-aware removal path. + // Why the worktree: Pi can leave a completed row keyed under an already-missing tab id; passing it sweeps that orphan while preserving active pre-render child rows. + actions.dropAgentStatusByTabPrefix(tabId, worktreeId ? { worktreeId } : undefined) + // Why: retired pane keys never recur, so stranded foreground entries would accumulate for the renderer's whole lifetime. + actions.clearPaneForegroundAgentByTabPrefix(tabId) + // Why: retirement permanently retires the tab's panes (a reopen mints a fresh leafId), so drop hibernation output epochs to keep the module map from growing forever. + forgetAgentHibernationTabOutput(tabId) + // Why: same rationale — retired tab ids never recur, so drop the foreground last-seen and consumed agent-startup delivery guards. + forgetForegroundTerminalTabs([tabId]) + forgetAgentStartupDeliveriesForTabs([tabId]) +} + +/** + * The same sweep as a patch, for a caller that owns its own `set()` (the paired snapshot apply + * builds patches from a state the store has not seen yet). Registry side effects still fire. + * `state` must already exclude the retired tabs from `tabsByWorktree`. + */ +export function buildRetiredTerminalTabStateSweepPatch( + state: RetiredTerminalTabSweepState, + tabIds: readonly string[], + worktreeId?: string | null +): Partial | null { + if (tabIds.length === 0) { + return null + } + // Why: the registry side effects run while the patch is computed (possibly inside a set() + // updater) — safe because all are idempotent and the ids are genuinely retired, but a caller + // that discards the patch still mutates the registries. + let swept: RetiredTerminalTabSweepState = state + for (const tabId of tabIds) { + retireParkedTerminalTab(tabId) + const { patch } = buildAgentStatusTabPrefixDropPatch( + swept, + tabId, + retireAgentPaneAuthorityAliasesByOwnerTab(tabId), + worktreeId ? { worktreeId } : undefined + ) + const foreground = buildPaneForegroundAgentTabPrefixClearPatch( + swept.paneForegroundAgentByPaneKey, + [`${tabId}:`] + ) + swept = { ...swept, ...patch, ...foreground } + forgetAgentHibernationTabOutput(tabId) + } + forgetForegroundTerminalTabs(tabIds) + forgetAgentStartupDeliveriesForTabs(tabIds) + const changed: Record = {} + for (const key of Object.keys(swept) as (keyof RetiredTerminalTabSweepState)[]) { + if (!Object.is(swept[key], state[key])) { + changed[key] = swept[key] + } + } + return Object.keys(changed).length === 0 + ? null + : (changed as Partial) +} diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index b33c3f9ad99..4e570420a11 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -49,10 +49,8 @@ import { WINDOWS_GIT_BASH_SHELL } from '../../../../shared/windows-terminal-shel import type { AgentStartedTelemetry } from '../../lib/worktree-startup-payload' import type { AiVaultSessionTitle } from '../../../../shared/ai-vault-session-title' import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph' -import { forgetAgentHibernationTabOutput } from '@/lib/agent-hibernation-output-activity' -import { forgetForegroundTerminalTabs } from '@/lib/foreground-terminal-tabs' import { terminalLayoutEqual } from '@/lib/terminal-layout-equality' -import { forgetAgentStartupDeliveriesForTabs } from '@/lib/agent-startup-delivery-guards' +import { sweepRetiredTerminalTabState } from './retired-terminal-tab-state-sweep' import { clearTransientTerminalState, emptyLayoutSnapshot } from './terminal-helpers' import { collectReleasedLeafIds, @@ -1830,19 +1828,8 @@ export const createTerminalSlice: StateCreator : {}) } }) - // Why: sweep tab agent status through its suppressor-aware removal path. - // Why: Pi can leave a completed row keyed under an already-missing tab id; pass the worktree to sweep that orphan while preserving active pre-render child rows. - get().dropAgentStatusByTabPrefix( - tabId, - closingWorktreeId ? { worktreeId: closingWorktreeId } : undefined - ) - // Why: retired pane keys never recur, so stranded foreground entries would accumulate for the renderer's whole lifetime. - get().clearPaneForegroundAgentByTabPrefix(tabId) - // Why: closing a tab permanently retires its panes (reopen mints a fresh leafId), so drop hibernation output epochs to keep the module map from growing forever. - forgetAgentHibernationTabOutput(tabId) - // Why: same rationale — retired tab ids never recur, so drop the foreground last-seen and consumed agent-startup delivery guards. - forgetForegroundTerminalTabs([tabId]) - forgetAgentStartupDeliveriesForTabs([tabId]) + // Why shared with the paired snapshot apply: every path that removes a tab owes it the same sweep, and a second copy of the list is how one path silently misses a new entry. + sweepRetiredTerminalTabState(get(), tabId, closingWorktreeId) for (const tabs of Object.values(get().unifiedTabsByWorktree)) { const workspaceItem = tabs.find( (entry) => entry.contentType === 'terminal' && entry.entityId === tabId