From ba43f8539c0c2bfd896813c870d495f61fbcfd1c Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 24 Oct 2026 17:00:48 -0700 Subject: [PATCH] fix(runtime): a cold-restored terminal keeps its agent session without showing the old Done (STA-12043) (#33302) * fix(runtime): a pane's new process does not wear its previous run's Done (STA-12043) * fix(runtime): keep the superseded turn's segment and date process start at spawn request * fix(runtime): take the identity row's segment from the turn clients were shown --- src/main/ipc/pty/ipc/spawn-state.ts | 2 + .../runtime/spawn-commit-run-facts.test.ts | 13 ++ src/main/ipc/pty/runtime/spawn-state.ts | 2 + src/main/ipc/pty/spawn-commit-assembly.ts | 2 + src/main/ipc/pty/spawn-commit-publication.ts | 5 +- ...ntime-hook-agent-status-projection.test.ts | 8 +- ...le-agent-status-title-truthfulness.test.ts | 44 ++++++ ...untime-mobile-agent-status-builder.test.ts | 113 +++++++++++++++- .../runtime-mobile-agent-status-builder.ts | 85 +++++++++++- .../prune-mobile-session-tab-group-layout.ts | 3 +- src/main/runtime/terminal-run-facts.test.ts | 26 ++++ src/main/runtime/terminal-run-facts.ts | 26 ++++ ...nverifiable-agent-row-presentation.test.ts | 127 +++++++++++++++++ ...abs-agent-completion-notifications.test.ts | 15 ++ .../web-session-tabs-sync/store-patch.ts | 6 +- src/shared/agent-status-types.ts | 3 +- ...ed-serve-cold-restore-agent-status.spec.ts | 128 ++++++++++++++++++ 17 files changed, 595 insertions(+), 13 deletions(-) create mode 100644 src/renderer/src/runtime/host-unverifiable-agent-row-presentation.test.ts create mode 100644 tests/e2e/paired-serve-cold-restore-agent-status.spec.ts diff --git a/src/main/ipc/pty/ipc/spawn-state.ts b/src/main/ipc/pty/ipc/spawn-state.ts index 638a88b5cf3..15e6a3a7f2e 100644 --- a/src/main/ipc/pty/ipc/spawn-state.ts +++ b/src/main/ipc/pty/ipc/spawn-state.ts @@ -26,6 +26,7 @@ export type PtyIpcSpawnState = { spawnTiming: PtySpawnTiming codexHomeLaunchStartedAt: Date | undefined codexHomeLaunchStartedSequence: number | undefined + spawnRequestedAt: number reattachedCodexHomeRoutes: Map cwd: string | undefined prevalidatedCwd: string | undefined @@ -110,6 +111,7 @@ export function createPtyIpcSpawnState( spawnTiming: createPtySpawnTiming(), codexHomeLaunchStartedAt: undefined, codexHomeLaunchStartedSequence: undefined, + spawnRequestedAt: Date.now(), reattachedCodexHomeRoutes: new Map(), cwd: undefined, prevalidatedCwd: undefined, diff --git a/src/main/ipc/pty/runtime/spawn-commit-run-facts.test.ts b/src/main/ipc/pty/runtime/spawn-commit-run-facts.test.ts index 4235b1ba6dc..594bf63a5ff 100644 --- a/src/main/ipc/pty/runtime/spawn-commit-run-facts.test.ts +++ b/src/main/ipc/pty/runtime/spawn-commit-run-facts.test.ts @@ -76,6 +76,19 @@ describe('runtime spawn commit: run facts', () => { expect((await commit({ coldRestore })).freshSpawn).toBe(false) }) + // STA-12043: a hook the new process sends while the commit persists must not read as older. + it('dates a new process by its spawn request, not its commit', async () => { + const facts = new TerminalRunFactsRegister() + let requestedAt = 0 + await commit({}, facts, undefined, (ctx) => { + requestedAt = ctx.spawnRequestedAt + vi.spyOn(Date, 'now').mockReturnValue(requestedAt + 5_000) + }) + vi.restoreAllMocks() + + expect(facts.readProcessStartedAt(PTY_ID, INCARNATION_ID)).toBe(requestedAt) + }) + it('keeps a process run facts when the same incarnation commits again', async () => { const facts = new TerminalRunFactsRegister() await commit({}, facts) diff --git a/src/main/ipc/pty/runtime/spawn-state.ts b/src/main/ipc/pty/runtime/spawn-state.ts index 5cb5a746a6d..d764b1fc4a1 100644 --- a/src/main/ipc/pty/runtime/spawn-state.ts +++ b/src/main/ipc/pty/runtime/spawn-state.ts @@ -32,6 +32,7 @@ export type RuntimePtySpawnState = { args: RuntimePtySpawnArgs codexHomeLaunchStartedAt: Date | undefined codexHomeLaunchStartedSequence: number | undefined + spawnRequestedAt: number preAdoptedStablePane: AdoptStablePaneResult | null reattachedCodexHomeRoutes: Map cwd: string | undefined @@ -167,6 +168,7 @@ export function createRuntimePtySpawnState( args, codexHomeLaunchStartedAt: undefined, codexHomeLaunchStartedSequence: undefined, + spawnRequestedAt: Date.now(), preAdoptedStablePane: null, reattachedCodexHomeRoutes: new Map(), cwd: undefined, diff --git a/src/main/ipc/pty/spawn-commit-assembly.ts b/src/main/ipc/pty/spawn-commit-assembly.ts index 85abc005db7..9613dfc5b83 100644 --- a/src/main/ipc/pty/spawn-commit-assembly.ts +++ b/src/main/ipc/pty/spawn-commit-assembly.ts @@ -71,6 +71,8 @@ export type PtySpawnCommitState = { codexSelectionTarget: CodexAccountSelectionTarget codexHomeLaunchStartedAt: Date | undefined codexHomeLaunchStartedSequence: number | undefined + /** Before the provider started anything, so no output of this spawn's process predates it. */ + spawnRequestedAt?: number reattachedCodexHomeRoutes: Map launchCommand: string | undefined preSpawnHiddenMarkId: string | null diff --git a/src/main/ipc/pty/spawn-commit-publication.ts b/src/main/ipc/pty/spawn-commit-publication.ts index 7796244f3b9..b52b75268a6 100644 --- a/src/main/ipc/pty/spawn-commit-publication.ts +++ b/src/main/ipc/pty/spawn-commit-publication.ts @@ -36,7 +36,10 @@ export function publishCommittedPtySpawn( const { state } = input const { deps, args, result } = state const runtime = deps.runtime - runtime?.noteTerminalSpawnCommit?.(result, input.expectedSourceBinding) + runtime?.noteTerminalSpawnCommit?.( + { ...result, spawnRequestedAt: state.spawnRequestedAt }, + input.expectedSourceBinding + ) input.spawnTiming?.log(result.id, { daemon: state.isDaemonHostSpawn, reattach: result.isReattach ?? false diff --git a/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts b/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts index 07fea3c3f00..da822047a2b 100644 --- a/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts +++ b/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts @@ -347,14 +347,16 @@ describe('headless hook agent-status projection (#11761)', () => { } }) - it('lets a shell title retire a restored turn as done', async () => { + // STA-12043: the retired turn keeps its identity but no state, so no Done sits beside the shell. + it('lets a shell title retire a restored turn to identity only', async () => { const agentStatus = await projectAgentStatus( [hookRow({ state: 'working', interactivePrompt: undefined, restoredUnconfirmed: true })], (runtime) => observePaneTitle(runtime, 'bash') ) - expect(agentStatus === undefined || agentStatus.state === 'done').toBe(true) - expect(agentStatus).not.toHaveProperty('restoredUnconfirmed') + expect(agentStatus).toEqual( + expect.objectContaining({ state: 'done', prompt: '', restoredUnconfirmed: true }) + ) }) // #7970: a retained OSC 9999 row is the pane's own report and keeps precedence. diff --git a/src/main/runtime/orca-runtime-mobile-agent-status-title-truthfulness.test.ts b/src/main/runtime/orca-runtime-mobile-agent-status-title-truthfulness.test.ts index 7d19f23db22..cbccf5b68ea 100644 --- a/src/main/runtime/orca-runtime-mobile-agent-status-title-truthfulness.test.ts +++ b/src/main/runtime/orca-runtime-mobile-agent-status-title-truthfulness.test.ts @@ -7,6 +7,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { OrcaRuntimeService } from './orca-runtime' import type { AgentStatusIpcPayload } from '../../shared/agent-status-types' import { makePaneKey } from '../../shared/stable-pane-id' +import { projectSessionTabAgentStatus } from './rpc/methods/session-tab-agent-status-projection' vi.mock('electron', () => ({ BrowserWindow: { fromId: vi.fn(() => null) }, @@ -264,3 +265,46 @@ describe('mobile session tabs: identity-only fallback freshness', () => { ) }) }) + +// STA-12043: a restored host starts a new shell in the pane; the old run's Done is not this one's. +describe('mobile session tabs: a process started after its pane last reported', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(T0) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('publishes identity only, and legacy done to clients that cannot read it', async () => { + const completedBeforeRestart: AgentStatusIpcPayload = { + paneKey: PANE_KEY, + state: 'done', + prompt: 'ship it', + agentType: 'codex', + connectionId: null, + receivedAt: T0 - 120_000, + stateStartedAt: T0 - 120_000, + tabId: TAB_ID, + worktreeId: WORKTREE_ID, + providerSession: PROVIDER_SESSION + } + const runtime = await createRuntime([completedBeforeRestart]) + runtime.noteTerminalSpawnCommit({ id: PTY_ID }) + runtime.onPtyData(PTY_ID, '\x1b]0;zsh\x07', T0) + + const result = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + const tab = result.tabs[0] + expect(tab?.type === 'terminal' ? tab.agentStatus : null).toEqual( + expect.objectContaining({ restoredUnconfirmed: true, providerSession: PROVIDER_SESSION }) + ) + const legacy = projectSessionTabAgentStatus(result, 'runtime', []).tabs[0] + expect(legacy?.type === 'terminal' ? legacy.agentStatus : null).toEqual( + expect.objectContaining({ state: 'done', providerSession: PROVIDER_SESSION }) + ) + expect(legacy?.type === 'terminal' ? legacy.agentStatus : null).not.toHaveProperty( + 'restoredUnconfirmed' + ) + }) +}) diff --git a/src/main/runtime/runtime-mobile-agent-status-builder.test.ts b/src/main/runtime/runtime-mobile-agent-status-builder.test.ts index ef0480a35a6..fcd20f6b241 100644 --- a/src/main/runtime/runtime-mobile-agent-status-builder.test.ts +++ b/src/main/runtime/runtime-mobile-agent-status-builder.test.ts @@ -183,10 +183,121 @@ describe('restored turns on a synced leaf', () => { ) }) - it('retires to done once the leaf PTY paints a newer shell title', () => { + // STA-12043: Done beside a shell claimed a completion nobody saw; the turn keeps only its identity. + it('retires to identity only once the leaf PTY paints a newer shell title', () => { const receivedAt = Date.now() - 60_000 const result = project(leafWithTitle('bash'), receivedAt + 1_000, receivedAt) + expect(result.agentStatus).toEqual( + expect.objectContaining({ + state: 'done', + prompt: '', + restoredUnconfirmed: true, + updatedAt: receivedAt + 1_000, + agentType: 'claude' + }) + ) + }) +}) + +// STA-12043: after a host restore the pane runs a new shell, yet its hook rows are the old run's. +describe('hook evidence from before the PTY process started', () => { + const SESSION = { key: 'session_id' as const, id: 'pre-restart-session' } + + function completedRow(agentType: 'codex' | undefined, receivedAt: number): AgentStatusIpcPayload { + return { + paneKey: PANE_KEY, + state: 'done', + prompt: 'old prompt', + ...(agentType ? { agentType } : {}), + connectionId: null, + receivedAt, + stateStartedAt: receivedAt, + tabId: TAB_ID, + worktreeId: 'wt-1', + providerSession: SESSION + } + } + + function projectAfterStart( + row: AgentStatusIpcPayload, + title: string | null, + processStartedAt: number | null + ) { + return buildRuntimeMobileAgentStatus(null, LEAF_TAB, 'term-1', null, () => [row], { + getPaneKey: () => PANE_KEY, + getLeaf: () => leafWithTitle(title), + getTrackedTitle: () => null, + getPtyTitleObservedAtEpochMs: (ptyId) => + ptyId === PTY_ID && title && processStartedAt ? processStartedAt + 1 : null, + getPtyProcessStartedAtEpochMs: (ptyId) => (ptyId === PTY_ID ? processStartedAt : null) + }) + } + + it.each([ + ['codex', null], + ['codex', 'zsh'], + [undefined, null], + [undefined, 'zsh'] + ] as const)('keeps only identity for agent=%s title=%s', (agentType, title) => { + const receivedAt = Date.now() - 120_000 + const result = projectAfterStart( + completedRow(agentType, receivedAt), + title, + receivedAt + 60_000 + ) + + expect(result.agentStatus).toEqual( + expect.objectContaining({ + restoredUnconfirmed: true, + updatedAt: receivedAt + 60_000, + stateStartedAt: receivedAt, + providerSession: SESSION + }) + ) + if (agentType) { + expect(result.agentStatus?.agentType).toBe(agentType) + } + }) + + it('keeps the shown turn, not a later resume-identity row, as the segment', () => { + const receivedAt = Date.now() - 120_000 + const identityRow: AgentStatusIpcPayload = { + ...completedRow('codex', receivedAt + 30_000), + stateStartedAt: receivedAt + 30_000, + providerSessionOnly: true + } + const result = buildRuntimeMobileAgentStatus( + null, + LEAF_TAB, + 'term-1', + null, + () => [completedRow('codex', receivedAt), identityRow], + { + getPaneKey: () => PANE_KEY, + getLeaf: () => leafWithTitle(null), + getTrackedTitle: () => null, + getPtyProcessStartedAtEpochMs: () => receivedAt + 60_000 + } + ) + + expect(result.agentStatus).toEqual( + expect.objectContaining({ restoredUnconfirmed: true, stateStartedAt: receivedAt }) + ) + }) + + it('keeps the completed turn of a process main attached to', () => { + const receivedAt = Date.now() - 120_000 + const result = projectAfterStart(completedRow('codex', receivedAt), null, null) + + expect(result.agentStatus?.state).toBe('done') + expect(result.agentStatus).not.toHaveProperty('restoredUnconfirmed') + }) + + it('keeps a turn the current process reported', () => { + const startedAt = Date.now() - 120_000 + const result = projectAfterStart(completedRow('codex', startedAt + 1_000), null, startedAt) + expect(result.agentStatus?.state).toBe('done') expect(result.agentStatus).not.toHaveProperty('restoredUnconfirmed') }) diff --git a/src/main/runtime/runtime-mobile-agent-status-builder.ts b/src/main/runtime/runtime-mobile-agent-status-builder.ts index 0dcccef37e8..0cf243d585c 100644 --- a/src/main/runtime/runtime-mobile-agent-status-builder.ts +++ b/src/main/runtime/runtime-mobile-agent-status-builder.ts @@ -6,7 +6,8 @@ import { resolvePaneAgentOwnerRecord } from '../../shared/pane-agent-owner' import { pickHostTurnEnd, type AgentStatusEntry, - type AgentStatusIpcPayload + type AgentStatusIpcPayload, + type HostTurnEndFacts } from '../../shared/agent-status-types' import type { RuntimeMobileSessionTerminalTab } from '../../shared/runtime-types' import { @@ -29,6 +30,8 @@ type RuntimeMobileAgentStatusHost = { getTrackedTitle(ptyId: string | null): string | null /** Wall clock of the PTY's latest live OSC title; seeded restore titles leave it unset. */ getPtyTitleObservedAtEpochMs?(ptyId: string | null): number | null + /** When main started the PTY's current process; null when it attached to a running one. */ + getPtyProcessStartedAtEpochMs?(ptyId: string): number | null } export function buildRuntimeMobileAgentStatus( @@ -44,7 +47,8 @@ export function buildRuntimeMobileAgentStatus( // provider session — only the full hook payload does, and headless serve has no // renderer to publish `tab.agentStatus`. Without it mobile native chat has no // transcript to address and sits on the empty state forever. - const hookRow = selectRuntimeHookAgentRowForPane(getHookRowsForPane(paneKey)) + const hookRows = getHookRowsForPane(paneKey) + const hookRow = selectRuntimeHookAgentRowForPane(hookRows) // Why: the hook row is evidence in its own right. Returning early on a missing // PTY status/projected row put this check ahead of the only headless carrier, so // an agent that reported its session but never emitted a recognized title got no @@ -103,6 +107,65 @@ export function buildRuntimeMobileAgentStatus( ownerAgent, ownerOptions ) + // Why the superseded turn's segment: a client that saw it reads this as a replay, not a new + // completion to announce. Only updatedAt advances, so the client's copy cannot outrank it. + const identityOnly = ( + updatedAt: number, + segment: { stateStartedAt: number } & HostTurnEndFacts + ): { agentStatus: AgentStatusEntry } => { + const agentType = ownerAgent ?? hookRow.agentType ?? hookRow.providerSessionAgentType + return { + agentStatus: normalizeCompatibleAgentStatusEntryForOwner( + { + state: 'done', + prompt: '', + updatedAt, + stateStartedAt: segment.stateStartedAt, + ...pickHostTurnEnd(segment), + // Why: clients render an unverifiable row as no status yet keep its identity, so native + // chat still finds the session; clients that cannot read it get the legacy `done`. + restoredUnconfirmed: true, + paneKey, + ...(terminalHandle ? { terminalHandle } : {}), + ...(agentType ? { agentType } : {}), + ...((pty?.worktreeId ?? leaf?.worktreeId) + ? { worktreeId: pty?.worktreeId ?? leaf?.worktreeId } + : {}), + tabId: tab.parentTabId, + terminalTitle, + stateHistory: [], + ...providerSession + }, + ownerAgent, + ownerOptions + ) + } + } + // Why: evidence from before main started this process describes an earlier one, so a fresh + // shell after a restore must not wear that turn's state. An attached process has no start here, + // and a relay replay that restamps receivedAt later only keeps the status as it was. + const processPtyId = pty?.ptyId ?? leaf?.ptyId + const processStartedAt = processPtyId ? host.getPtyProcessStartedAtEpochMs?.(processPtyId) : null + const newestEvidenceAt = Math.max( + retained?.updatedAt ?? -Infinity, + ...hookRows.map((row) => row.receivedAt) + ) + // The segment is the turn a client was shown; a later resume-identity row is not a turn. + const shownSegment = + retained ?? + hookRow.live ?? + hookRow.restored ?? + newestHookRow(hookRows.filter((row) => row.providerSessionOnly !== true)) ?? + newestHookRow(hookRows) + if ( + processStartedAt != null && + shownSegment && + newestEvidenceAt < processStartedAt && + !pty?.lastAgentStatus && + !leaf?.lastAgentStatus + ) { + return identityOnly(processStartedAt, shownSegment) + } // Why: OSC 9999 hook payload carries real state/prompt/agent; without preferring it, hook-only transitions never surfaced (#7970). const liveRow = retained ?? resolveRuntimeHookLiveAgentRow(hookRow.live, pty, nonAgentTitle) if (liveRow) { @@ -139,12 +202,17 @@ export function buildRuntimeMobileAgentStatus( } // Why: a turn restored across a server restart is unverifiable, not finished; a synthetic // `done` told clients a running agent had stopped. Keep its own date (so it cannot outrank a - // client's row) and drop the question fields (#12346). Only a later shell title retires it. + // client's row) and drop the question fields (#12346). A later shell title retires the turn, + // leaving only its identity: Done beside a shell would claim a completion nobody saw. + const titleObservedAt = + pty?.lastOscTitleEpochMs ?? host.getPtyTitleObservedAtEpochMs?.(leaf?.ptyId ?? null) ?? 0 const shellTitleRetiredRestored = hookRow.restored !== null && terminalTitleBlocksExplicitAgentStatus(ptyTitle) && - (pty?.lastOscTitleEpochMs ?? host.getPtyTitleObservedAtEpochMs?.(leaf?.ptyId ?? null) ?? 0) >= - hookRow.restored.updatedAt + titleObservedAt >= hookRow.restored.updatedAt + if (shellTitleRetiredRestored && hookRow.restored && !pty?.lastAgentStatus) { + return identityOnly(titleObservedAt, hookRow.restored) + } const restored = !pty?.lastAgentStatus && !shellTitleRetiredRestored ? hookRow.restored : null if (restored) { const restoredAgentType = ownerAgent ?? restored.payload.agentType @@ -202,3 +270,10 @@ export function buildRuntimeMobileAgentStatus( } } } + +function newestHookRow(rows: readonly AgentStatusIpcPayload[]): AgentStatusIpcPayload | null { + return rows.reduce( + (newest, row) => (!newest || row.receivedAt > newest.receivedAt ? row : newest), + null + ) +} diff --git a/src/main/runtime/session-lifecycle/projection/prune-mobile-session-tab-group-layout.ts b/src/main/runtime/session-lifecycle/projection/prune-mobile-session-tab-group-layout.ts index faaf80b94af..be7d5be9ea4 100644 --- a/src/main/runtime/session-lifecycle/projection/prune-mobile-session-tab-group-layout.ts +++ b/src/main/runtime/session-lifecycle/projection/prune-mobile-session-tab-group-layout.ts @@ -178,7 +178,8 @@ export abstract class OrcaRuntimeWithPruneMobileSessionTabGroupLayout extends Or }, getTrackedTitle: (ptyId) => this.getUnpersistedTrackedTitleForPty(ptyId), getPtyTitleObservedAtEpochMs: (ptyId) => - ptyId ? (this.ptysById.get(ptyId)?.lastOscTitleEpochMs ?? null) : null + ptyId ? (this.ptysById.get(ptyId)?.lastOscTitleEpochMs ?? null) : null, + getPtyProcessStartedAtEpochMs: (ptyId) => this.terminalRunFacts.readProcessStartedAt(ptyId) } ) } diff --git a/src/main/runtime/terminal-run-facts.test.ts b/src/main/runtime/terminal-run-facts.test.ts index 1966a405949..3789e98ab60 100644 --- a/src/main/runtime/terminal-run-facts.test.ts +++ b/src/main/runtime/terminal-run-facts.test.ts @@ -46,6 +46,32 @@ describe('terminal run facts', () => { expect(facts.read('pty-1', 'inc-1')).toEqual({ freshSpawn: false, firstUserInputAt: null }) }) + it('dates only the processes main started, across a re-registration of the same one', () => { + const facts = new TerminalRunFactsRegister() + facts.recordSpawnCommit({ + id: 'pty-1', + incarnationId: 'inc-1', + coldRestore: {}, + spawnRequestedAt: 100 + }) + facts.recordSpawnCommit({ + id: 'pty-1', + incarnationId: 'inc-1', + isReattach: true, + spawnRequestedAt: 200 + }) + facts.recordSpawnCommit({ + id: 'pty-2', + incarnationId: 'inc-9', + isReattach: true, + spawnRequestedAt: 300 + }) + + expect(facts.readProcessStartedAt('pty-1', 'inc-1')).toBe(100) + expect(facts.readProcessStartedAt('pty-1', 'inc-0')).toBeNull() + expect(facts.readProcessStartedAt('pty-2', 'inc-9')).toBeNull() + }) + it('never reads a reattached process as fresh', () => { const facts = new TerminalRunFactsRegister() diff --git a/src/main/runtime/terminal-run-facts.ts b/src/main/runtime/terminal-run-facts.ts index e6753c58be9..dc919f8ee93 100644 --- a/src/main/runtime/terminal-run-facts.ts +++ b/src/main/runtime/terminal-run-facts.ts @@ -16,6 +16,8 @@ export type TerminalSpawnCommit = Parameters[0] id: string incarnationId?: string coldRestore?: object + /** Taken before the spawn, or a fast process's first report would read as older than it. */ + spawnRequestedAt?: number } /** A cold restore starts a new process for a pane that had one, so it is never fresh. */ @@ -25,6 +27,14 @@ type TerminalRunRecord = { incarnationId: string | null spawnOrigin: TerminalRunSpawnOrigin firstUserInputAt: number | null + /** When main committed a process it started (spawn, split or cold restore); null for one it + * attached to, whose start main never saw. */ + processStartedAt: number | null +} + +// Why not the binding origin: a split outranks a reattach there, but only an attach keeps a process. +function startedNewProcess(commit: TerminalSpawnCommit): boolean { + return commit.isReattach !== true && commit.agentSessionEnsure?.disposition !== 'adopted' } /** Main's per-process facts about one PTY run, keyed by the incarnation they describe. */ @@ -83,6 +93,11 @@ export class TerminalRunFactsRegister { this.runsByPtyId.set(commit.id, { incarnationId, spawnOrigin: origin === 'spawn' && commit.coldRestore !== undefined ? 'cold-restore' : origin, + processStartedAt: sameProcess + ? (previous?.processStartedAt ?? null) + : startedNewProcess(commit) + ? (commit.spawnRequestedAt ?? Date.now()) + : null, firstUserInputAt: sameProcess ? (previous?.firstUserInputAt ?? null) : pending?.incarnationId === incarnationId @@ -134,6 +149,17 @@ export class TerminalRunFactsRegister { } } + /** When the PTY's current process was started by main; null when main attached to it instead, + * so evidence older than that may still describe it. A run older than the PTY's live process + * only understates that start, so callers may omit the incarnation. */ + readProcessStartedAt(ptyId: string, incarnationId?: string | null): number | null { + const run = this.runsByPtyId.get(ptyId) + if (!run || (run.incarnationId && incarnationId && run.incarnationId !== incarnationId)) { + return null + } + return run.processStartedAt + } + delete(ptyId: string): void { this.runsByPtyId.delete(ptyId) this.lastInputAtByPtyId.delete(ptyId) diff --git a/src/renderer/src/runtime/host-unverifiable-agent-row-presentation.test.ts b/src/renderer/src/runtime/host-unverifiable-agent-row-presentation.test.ts new file mode 100644 index 00000000000..c4bfac13582 --- /dev/null +++ b/src/renderer/src/runtime/host-unverifiable-agent-row-presentation.test.ts @@ -0,0 +1,127 @@ +/** + * STA-12043, client half: once a restored host starts a new shell in a pane, it publishes the old + * run's row as unverifiable. The paired client must replace its copy of the old Done with no tab + * status, yet native chat must still find the conversation through the row's provider session. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { 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 { createTestStore, makeWorktree, seedStore } from '../store/slices/store-test-helpers' +import { applyFreshWebSessionTabsSnapshot } from './web-session-tabs-sync/snapshot-api' +import { resetWebSessionTabsSnapshotFreshnessForTests } from './web-session-tabs-sync/tracking-lifecycle' +import { resolveTerminalTabActivityStatus } from '../components/tab-bar/terminal-tab-activity-status' +import { resolveNativeChatSession } from '../components/native-chat/native-chat-pane-resolution' + +vi.mock('../store', () => ({ + useAppStore: { + setState: vi.fn(), + getState: vi.fn(() => ({})), + subscribe: vi.fn(() => () => {}) + } +})) + +const WT = 'repo1::/path/wt1' +const ENV = 'remote-env-1' +const HOST_TAB = 'host-tab-1' +const LEAF = '11111111-1111-4111-8111-111111111111' +const SESSION = { key: 'session_id' as const, id: 'pre-restart-session' } +const T0 = 1_700_000_000_000 + +function hostSnapshot( + snapshotVersion: number, + agentStatus: { updatedAt: number; restoredUnconfirmed?: true } +): RuntimeMobileSessionTabsResult { + return { + worktree: WT, + publicationEpoch: 'host-epoch-1', + snapshotVersion, + activeGroupId: 'host-group-1', + activeTabId: `${HOST_TAB}::${LEAF}`, + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: `${HOST_TAB}::${LEAF}`, + title: 'zsh', + parentTabId: HOST_TAB, + leafId: LEAF, + isActive: true, + status: 'ready', + terminal: 'terminal-1', + agentStatus: { + state: 'done', + prompt: '', + stateStartedAt: agentStatus.updatedAt, + agentType: 'codex', + paneKey: makePaneKey(HOST_TAB, LEAF), + tabId: HOST_TAB, + worktreeId: WT, + stateHistory: [], + providerSession: SESSION, + ...agentStatus + } + } + ] + } +} + +describe('a host row published unverifiable after a cold restore', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(T0) + resetWebSessionTabsSnapshotFreshnessForTests() + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('replaces the mirrored Done with no tab status and keeps the session', () => { + const store = createTestStore() + seedStore(store, { + settings: { ...getDefaultSettings('/tmp'), tabAutoGenerateTitle: true }, + worktreesByRepo: { repo1: [makeWorktree({ id: WT, repoId: 'repo1', path: '/path/wt1' })] }, + activeWorktreeId: WT + }) + const mirroredTabId = toWebTerminalSurfaceTabId(HOST_TAB) + const paneKey = makePaneKey(mirroredTabId, LEAF) + const tabStatus = (): string => { + const state = store.getState() + return resolveTerminalTabActivityStatus({ + tab: { id: mirroredTabId, title: 'zsh' }, + agentStatusByPaneKey: state.agentStatusByPaneKey + }) + } + + store.setState( + applyFreshWebSessionTabsSnapshot( + store.getState(), + hostSnapshot(1, { updatedAt: T0 }), + ENV, + T0 + ) + ) + expect(tabStatus()).toBe('done') + + vi.setSystemTime(T0 + 60_000) + store.setState( + applyFreshWebSessionTabsSnapshot( + store.getState(), + hostSnapshot(2, { updatedAt: T0 + 30_000, restoredUnconfirmed: true }), + ENV, + T0 + 60_000 + ) + ) + + expect(tabStatus()).not.toBe('done') + expect( + resolveNativeChatSession({ + paneKey, + agentStatusEntry: store.getState().agentStatusByPaneKey[paneKey], + ptyId: null + })?.sessionId + ).toBe(SESSION.id) + }) +}) diff --git a/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts b/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts index 16023b28167..3935a0208ec 100644 --- a/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts @@ -127,6 +127,21 @@ describe('paired session-tab agent completion notifications', () => { expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(4) }) + // STA-12043: a cold-restored host republishes the old turn as unverifiable under a new date. + it('announces no completion for an unverifiable done', () => { + applySnapshot(makeAgentSnapshot(1, NOW, undefined, 'done', NOW), true) + const announced = mocks.observeAgentHookCompletionForNotification.mock.calls.length + const restored = makeAgentSnapshot(2, NOW + 60_000, undefined, 'done', NOW + 60_000) + const [tab] = restored.tabs + if (tab?.type === 'terminal' && tab.agentStatus) { + tab.agentStatus.restoredUnconfirmed = true + } + + applySnapshot(restored, true) + + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(announced) + }) + it('announces one live stamped completion after a late-pair working seed', () => { const dispatchCompletion = vi.fn() const coordinator = createAgentCompletionCoordinator({ diff --git a/src/renderer/src/runtime/web-session-tabs-sync/store-patch.ts b/src/renderer/src/runtime/web-session-tabs-sync/store-patch.ts index a553d9cecd9..fbfcedef107 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/store-patch.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/store-patch.ts @@ -89,7 +89,11 @@ export function applyWebSessionTabsStorePatch( continue } const notificationStatus = clientOwnedNotification ? remapped : accepted - if (!notificationStatus) { + // Why: an unverifiable `done` restates an earlier turn or none, never a completion. + if ( + !notificationStatus || + (notificationStatus.state === 'done' && notificationStatus.restoredUnconfirmed) + ) { continue } const currentClientStateStartedAt = clientOwnedNotification diff --git a/src/shared/agent-status-types.ts b/src/shared/agent-status-types.ts index a0c353a6083..e31f2df067c 100644 --- a/src/shared/agent-status-types.ts +++ b/src/shared/agent-status-types.ts @@ -169,7 +169,8 @@ export type AgentStatusEntry = { programStatus?: ProgramStatusFacet /** True for a nonterminal state hydrated from last-status.json with no live hook since: * the transition may have been missed while no receiver was up, so freshness gates - * treat the row as stale immediately. Cleared by any accepted live event. */ + * treat the row as stale immediately. Cleared by any accepted live event. A host also + * publishes it on a row older than the pane's current process: identity only, no status. */ restoredUnconfirmed?: boolean } & AgentStatusRowFacets diff --git a/tests/e2e/paired-serve-cold-restore-agent-status.spec.ts b/tests/e2e/paired-serve-cold-restore-agent-status.spec.ts new file mode 100644 index 00000000000..3a69fbfa01e --- /dev/null +++ b/tests/e2e/paired-serve-cold-restore-agent-status.spec.ts @@ -0,0 +1,128 @@ +/** + * STA-12043: a serve host stopped with its terminal daemon and restored starts a new shell in each + * pane. That pane's last hook row is the old run's completed turn, so a phone must not see Done + * beside the new shell, yet must still find the conversation through its provider session. A phone + * that cannot read unverifiable rows keeps the legacy Done it always received. + */ +import { expect, test } from './helpers/orca-app' +import { launchPhoneMirrorTopology } from './helpers/phone-mirror-topology' +import { pairMobileClient, type PairedMobileClient } from './helpers/paired-mobile-client' +import { emitCodexHookStatus, readHookEndpoint } from './helpers/agent-hook-endpoint' +import { cleanupE2EDaemons } from './helpers/electron-process-shutdown' +import type { RuntimeWorktreePsSummary } from '../../src/shared/runtime-worktree-contracts' +import type { RuntimeMobileSessionTabsResult } from '../../src/shared/runtime-session-contracts' + +const SESSION_ID = '0199a1b2-c3d4-7e5f-8a9b-0c1d2e3f4a5b' + +type PaneStatus = { + ready: boolean + state: string | null + restoredUnconfirmed: boolean + sessionId: string | null +} + +async function readPaneStatus( + phone: PairedMobileClient, + worktreeId: string, + tabId: string +): Promise { + const listed = await phone.request('session.tabs.list', { + worktree: `id:${worktreeId}` + }) + if (!listed.ok) { + return `refused:${listed.error.code}` + } + const tab = listed.result.tabs.find( + (candidate) => candidate.type === 'terminal' && candidate.parentTabId === tabId + ) + if (tab?.type !== 'terminal') { + return 'missing' + } + return { + ready: tab.status === 'ready', + state: tab.agentStatus?.state ?? null, + restoredUnconfirmed: tab.agentStatus?.restoredUnconfirmed === true, + sessionId: tab.agentStatus?.providerSession?.id ?? null + } +} + +test('a cold-restored serve terminal keeps its agent session without showing Done', async ({ + testRepoPath +}, testInfo) => { + test.setTimeout(240_000) + const { host, phone, dispose } = await launchPhoneMirrorTopology( + { phoneTo: 'host', pinnedServePort: true }, + testInfo + ) + const legacyPhone = pairMobileClient(host.offer, []) + try { + await host.client.call('repo.add', { path: testRepoPath, kind: 'git' }) + const { worktrees } = await phone.call<{ worktrees: RuntimeWorktreePsSummary[] }>( + 'worktree.ps', + { limit: 1_000 } + ) + const worktreeId = worktrees.find((row) => row.path === testRepoPath)?.worktreeId ?? '' + expect(worktreeId, 'the phone lists the serve host workspace').not.toBe('') + + const created = await phone.call<{ tab: { id: string; parentTabId: string; leafId: string } }>( + 'session.tabs.createTerminal', + { worktree: `id:${worktreeId}`, activate: false, select: true, navigation: 'caller' } + ) + const { parentTabId: tabId, leafId } = created.tab + const paneKey = `${tabId}:${leafId}` + + const endpoint = await readHookEndpoint(host.app) + await emitCodexHookStatus(endpoint, { + paneKey, + worktreeId, + state: 'working', + prompt: 'say hi', + sessionId: SESSION_ID + }) + await emitCodexHookStatus(endpoint, { + paneKey, + worktreeId, + state: 'done', + lastAssistantMessage: 'hi', + sessionId: SESSION_ID + }) + await expect + .poll(() => readPaneStatus(phone, worktreeId, tabId)) + .toMatchObject({ state: 'done', restoredUnconfirmed: false, sessionId: SESSION_ID }) + + // A user opening the tab is the wake gesture that starts or reattaches the restored pane. + const openTab = () => + expect + .poll(async () => { + const activated = await phone.request('session.tabs.activate', { + worktree: `id:${worktreeId}`, + tabId: created.tab.id, + navigation: 'caller', + intent: 'user' + }) + return activated.ok + }) + .toBe(true) + + // The daemon kept the shell the turn ran in, so nothing says that turn is gone. + await host.restartServeProcess() + await openTab() + await expect + .poll(() => readPaneStatus(phone, worktreeId, tabId), { timeout: 60_000 }) + .toMatchObject({ ready: true, state: 'done', restoredUnconfirmed: false }) + + // Why the daemon too: a serve restart alone reattaches the same shell; the ticket's host lost it. + await host.restartServeProcess({ betweenProcesses: () => cleanupE2EDaemons(host.userDataDir) }) + + await openTab() + await expect + .poll(() => readPaneStatus(phone, worktreeId, tabId), { timeout: 60_000 }) + .toMatchObject({ ready: true, restoredUnconfirmed: true, sessionId: SESSION_ID }) + await expect + .poll(() => readPaneStatus(legacyPhone, worktreeId, tabId)) + .toMatchObject({ state: 'done', restoredUnconfirmed: false, sessionId: SESSION_ID }) + } finally { + legacyPhone.dispose() + await dispose() + } +})