diff --git a/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts b/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts new file mode 100644 index 000000000..14235654d --- /dev/null +++ b/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from 'vitest' +import type { AgentHookEventPayload } from '../../shared/agent-hook-listener' +import type { ParsedAgentStatusPayload } from '../../shared/agent-status-types' +import { createHookStatusSessionTabsInvalidator } from './hook-status-session-tabs-invalidation' + +function working( + overrides: Partial = {}, + payload: Partial = {} +): AgentHookEventPayload { + return { + paneKey: 'tab:leaf', + connectionId: null, + payload: { state: 'working', prompt: 'fix the tests', agentType: 'claude', ...payload }, + ...overrides + } +} + +describe('createHookStatusSessionTabsInvalidator', () => { + it('invalidates the first time a pane reports', () => { + const changed = createHookStatusSessionTabsInvalidator() + + expect(changed(working())).toBe(true) + }) + + it('stays quiet while the same status keeps being pinged', () => { + const changed = createHookStatusSessionTabsInvalidator() + changed(working()) + + expect(changed(working())).toBe(false) + }) + + it.each([ + ['state', { state: 'waiting' as const }], + ['prompt', { prompt: 'ship it' }], + ['agentType', { agentType: 'codex' }], + ['toolName', { toolName: 'Bash' }], + ['interactivePrompt', { interactivePrompt: '{"questions":[]}' }], + ['interrupted', { interrupted: true }] + ])('invalidates when %s changes', (_field, payload) => { + const changed = createHookStatusSessionTabsInvalidator() + changed(working()) + + expect(changed(working({}, payload))).toBe(true) + }) + + it('ignores resume-identity rows, which the provider-session path owns', () => { + const changed = createHookStatusSessionTabsInvalidator() + + expect(changed(working({ providerSessionOnly: true }))).toBe(false) + }) + + it('tracks panes independently', () => { + const changed = createHookStatusSessionTabsInvalidator() + changed(working()) + + expect(changed(working({ paneKey: 'tab:other' }))).toBe(true) + expect(changed(working())).toBe(false) + }) + + it('re-arms a forgotten pane so an identical relaunch still invalidates', () => { + const changed = createHookStatusSessionTabsInvalidator() + changed(working()) + changed.forgetPane('tab:leaf') + + expect(changed(working())).toBe(true) + }) + + it("names an SSH host's panes so a disconnect can republish each of them", () => { + const changed = createHookStatusSessionTabsInvalidator() + changed(working({ connectionId: 'conn-1' })) + changed(working({ paneKey: 'tab:remote', connectionId: 'conn-1' })) + changed(working({ paneKey: 'tab:local' })) + + expect(changed.forgetConnection('conn-1').sort()).toEqual(['tab:leaf', 'tab:remote']) + expect(changed(working({ paneKey: 'tab:local' }))).toBe(false) + }) +}) diff --git a/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts b/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts new file mode 100644 index 000000000..59b259fa8 --- /dev/null +++ b/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts @@ -0,0 +1,52 @@ +import type { AgentHookEventPayload } from '../../shared/agent-hook-listener' +import type { ParsedAgentStatusPayload } from '../../shared/agent-status-types' + +type KnownStatus = { connectionId: string | null; payload: ParsedAgentStatusPayload } + +/** Reports whether a hook status event changed anything the `session.tabs` + * projection publishes, so a repeated same-state ping costs no snapshot rebuild. + * Mirrors `retainAgentRowSnapshot`'s change set so both carriers invalidate alike. */ +export function createHookStatusSessionTabsInvalidator(): { + (event: AgentHookEventPayload): boolean + forgetPane: (paneKey: string) => void + forgetConnection: (connectionId: string) => string[] +} { + const known = new Map() + const invalidator = (event: AgentHookEventPayload): boolean => { + // Why: resume-identity rows carry transport placeholders, not status; the + // provider-session invalidator owns their republish. + if (event.providerSessionOnly === true) { + return false + } + const previous = known.get(event.paneKey)?.payload + const next = event.payload + known.set(event.paneKey, { connectionId: event.connectionId, payload: next }) + return ( + !previous || + previous.state !== next.state || + previous.prompt !== next.prompt || + (previous.agentType ?? null) !== (next.agentType ?? null) || + (previous.toolName ?? null) !== (next.toolName ?? null) || + (previous.interactivePrompt ?? null) !== (next.interactivePrompt ?? null) || + (previous.interrupted ?? false) !== (next.interrupted ?? false) + ) + } + // Why: a cleared pane must re-arm, else the memo swallows the first event of the + // next agent when it happens to match the one that just went away. + invalidator.forgetPane = (paneKey: string): void => { + known.delete(paneKey) + } + // Why: an SSH disconnect clears a whole host's rows at once and names no pane, so + // the caller needs the pane list back to republish each affected workspace. + invalidator.forgetConnection = (connectionId: string): string[] => { + const forgotten: string[] = [] + for (const [paneKey, status] of known) { + if (status.connectionId === connectionId) { + known.delete(paneKey) + forgotten.push(paneKey) + } + } + return forgotten + } + return invalidator +} diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 71fa1a6a0..9bf8fc43f 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -570,6 +570,7 @@ export class AgentHookServer { private onAgentStatus: ((payload: EnrichedAgentHookEventPayload) => void) | null = null private onClaudeStatusLine: ((event: ClaudeStatusLineRateLimits) => void) | null = null private onPaneStatusCleared: PaneStatusClearListener | null = null + private paneStatusClearListeners = new Set() private statusChangeListeners = new Set() private providerSessionChangeListeners = new Set() // Why: setListener is a single slot owned by the main-window fanout; the @@ -654,6 +655,29 @@ export class AgentHookServer { this.onPaneStatusCleared = listener } + /** Multi-subscriber tap on pane status clears. Unlike `setPaneStatusClearListener` + * (a single slot the main window owns and drops on close) this survives window + * teardown and exists at all under headless serve, which never opens one. */ + subscribePaneStatusClear(listener: PaneStatusClearListener): () => void { + this.paneStatusClearListeners.add(listener) + return () => { + this.paneStatusClearListeners.delete(listener) + } + } + + private emitPaneStatusCleared(clear: AgentStatusClearIpcPayload): void { + this.onPaneStatusCleared?.(clear) + for (const listener of this.paneStatusClearListeners) { + // Why: callers are pane/connection teardown paths; one throwing subscriber must + // not strand the rest, matching every other fan-out here. + try { + listener(clear) + } catch (err) { + console.error('[agent-hooks] pane-status-clear listener threw', err) + } + } + } + /** Snapshot of cached statuses in IPC shape. Used by `agentStatus:getSnapshot` after tabs hydrate so the * dashboard catches up on hook events that fired during startup. */ getStatusSnapshot(): AgentStatusIpcPayload[] { @@ -1633,7 +1657,7 @@ export class AgentHookServer { this.scheduleStatusPersist() this.notifyStatusChangeListeners() for (const paneKey of clearedStatusPaneKeys) { - this.onPaneStatusCleared?.({ paneKey }) + this.emitPaneStatusCleared({ paneKey }) } } } @@ -2177,7 +2201,7 @@ export class AgentHookServer { this.notifyStatusChangeListeners() } // Why: always send the cutoff even with no matched entry — another host may have overwritten this pane's row. - this.onPaneStatusCleared?.({ + this.emitPaneStatusCleared({ transient: true, connectionId: normalizedConnectionId, clearedAt @@ -2319,7 +2343,7 @@ export class AgentHookServer { this.runtimeObservedStatusPaneKeys.delete(resolvedPaneKey) this.scheduleStatusPersist() this.notifyStatusChangeListeners() - this.onPaneStatusCleared?.({ paneKey: resolvedPaneKey }) + this.emitPaneStatusCleared({ paneKey: resolvedPaneKey }) } } diff --git a/src/main/index.ts b/src/main/index.ts index 7ed25c33d..91bb5aea8 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -231,6 +231,7 @@ import { import { StarNagService } from './star-nag/service' import { agentHookServer, type AgentHookProviderSessionIdentity } from './agent-hooks/server' import { createHookProviderSessionInvalidator } from './agent-hooks/hook-provider-session-invalidation' +import { createHookStatusSessionTabsInvalidator } from './agent-hooks/hook-status-session-tabs-invalidation' import { wslHookRelayManager } from './agent-hooks/wsl-hook-relay-manager' import { maybeAutoRenameBranchOnFirstWork } from './agent-hooks/first-work-branch-rename' import { rememberBranchRenameFailureOutput } from './agent-hooks/branch-rename-failure-output' @@ -2110,7 +2111,9 @@ void app.whenReady().then(async () => { undefined })) for (const worktreeId of collectChangedProviderSessionWorktrees(ownedIdentities)) { - runtime?.notifyMobileSessionTabsChanged(worktreeId) + // Why not `notifyMobileSessionTabsChanged` alone: it re-emits at the unchanged + // `snapshotVersion`, which every client drops on its monotonic gate. + runtime?.touchMobileSessionTabsForWorktree(worktreeId, { immediate: true }) } } const unsubscribeStatusChanges = agentHookServer.subscribeStatusChanges((statuses) => { @@ -2122,9 +2125,32 @@ void app.whenReady().then(async () => { publishProviderSessionChanges(sessions) } ) + // Why: hook rows are the only carrier of live agent state on a headless host, and + // nothing else republishes `session.tabs` when one changes — so a paired client + // would keep the pane's last projection until an unrelated PTY touch came along. + const hookStatusChangedSessionTabs = createHookStatusSessionTabsInvalidator() + const unsubscribeHookStatusSessionTabs = agentHookServer.subscribeEnrichedStatus((enriched) => { + if (hookStatusChangedSessionTabs(enriched)) { + runtime?.touchMobileSessionTabsForPane(enriched.paneKey, enriched.worktreeId ?? null) + } + }) + // Teardown: agent exit, pane close, and the SSH transient-disconnect batch all land + // here. Without it the live state published above becomes a zombie question card. + const unsubscribeHookStatusClear = agentHookServer.subscribePaneStatusClear((clear) => { + const clearedPaneKeys = + 'paneKey' in clear + ? [clear.paneKey] + : hookStatusChangedSessionTabs.forgetConnection(clear.connectionId) + for (const paneKey of clearedPaneKeys) { + hookStatusChangedSessionTabs.forgetPane(paneKey) + runtime?.touchMobileSessionTabsForPane(paneKey) + } + }) unsubscribeAgentAwakeStatusChanges = () => { unsubscribeStatusChanges() unsubscribeProviderSessionChanges() + unsubscribeHookStatusSessionTabs() + unsubscribeHookStatusClear() } // Why: telemetry must init before any IPC handler/renderer can call track(); it's a no-op in dev and while TELEMETRY_ENABLED is false, so it's safe early. initTelemetry(store) 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 new file mode 100644 index 000000000..47715ea7c --- /dev/null +++ b/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts @@ -0,0 +1,333 @@ +// #11761: on a headless `orca serve` host the HTTP agent hook is the only carrier of +// live agent state, so `session.tabs` must project the hook row's status fields — not +// just its identity — while still refusing rows that only prove an agent once existed. +import { describe, expect, it, vi } from 'vitest' +import { OrcaRuntimeService } from './orca-runtime' +import { AGENT_STATUS_STALE_AFTER_MS } from '../../shared/agent-status-types' +import type { AgentStatusIpcPayload } from '../../shared/agent-status-types' +import { makePaneKey } from '../../shared/stable-pane-id' + +vi.mock('electron', () => ({ + BrowserWindow: { fromId: vi.fn(() => null) }, + webContents: { fromId: vi.fn(() => null) }, + ipcMain: { on: vi.fn(), removeListener: vi.fn() }, + app: { getPath: vi.fn(() => '/tmp') } +})) + +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +// A leaf this runtime never minted: pane lookup recovers a reminted tab id by leaf id, +// so only an unknown leaf id is genuinely unresolvable. +const UNKNOWN_LEAF_ID = '22222222-2222-4222-8222-222222222222' +const TAB_ID = 'ask-tab' +const WORKTREE_ID = 'wt-1' +const PANE_KEY = makePaneKey(TAB_ID, LEAF_ID) +const PTY_ID = 'pty-ask' +const ASK_PROMPT = JSON.stringify({ + questions: [ + { + question: 'Tabs or spaces?', + header: 'Style', + multiSelect: false, + options: [{ label: 'Tabs' }, { label: 'Spaces' }] + } + ] +}) +const PROVIDER_SESSION = { + key: 'session_id' as const, + id: 'ac1f6b90-2f77-4f0e-9c5e-1d2f6a4b8c31', + transcriptPath: '/transcripts/ac1f6b90.jsonl' +} + +function hookRow(overrides: Partial = {}): AgentStatusIpcPayload { + const now = Date.now() + return { + paneKey: PANE_KEY, + state: 'waiting', + prompt: 'Tabs or spaces?', + agentType: 'claude', + toolName: 'AskUserQuestion', + interactivePrompt: ASK_PROMPT, + connectionId: null, + receivedAt: now, + stateStartedAt: now, + tabId: TAB_ID, + worktreeId: WORKTREE_ID, + providerSession: PROVIDER_SESSION, + ...overrides + } +} + +async function createRuntimeWithHookRows( + rows: AgentStatusIpcPayload[] +): Promise { + const runtime = new OrcaRuntimeService(null, undefined, { + getAgentStatusSnapshot: () => rows + }) + const internals = runtime as unknown as { + resolveTerminalWorkspaceLaunchScope: (selector: string) => Promise + } + vi.spyOn(internals, 'resolveTerminalWorkspaceLaunchScope').mockResolvedValue({ + id: WORKTREE_ID, + path: '/repo/app', + connectionId: null, + repo: null, + folderWorkspace: null + }) + runtime.setPtyController({ + spawn: vi.fn().mockResolvedValue({ id: PTY_ID }), + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + await runtime.createTerminal(`id:${WORKTREE_ID}`, { + tabId: TAB_ID, + leafId: LEAF_ID, + launchAgent: 'claude', + title: 'Terminal' + }) + return runtime +} + +async function projectAgentStatus( + rows: AgentStatusIpcPayload[], + preparePane?: (runtime: OrcaRuntimeService) => void +): Promise | undefined> { + const runtime = await createRuntimeWithHookRows(rows) + preparePane?.(runtime) + const result = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + const tab = result.tabs[0] + return tab?.type === 'terminal' + ? (tab.agentStatus as unknown as Record | undefined) + : undefined +} + +/** Observe a pane title the way production does, so the recency stamps under test are + * the ones the OSC path actually writes (a sequence number *and* a wall clock). */ +function observePaneTitle(runtime: OrcaRuntimeService, title: string): void { + runtime.onPtyData(PTY_ID, `\x1b]0;${title}\x07`, Date.now()) +} + +function lastOscTitleEpochMs(runtime: OrcaRuntimeService): number { + const pty = ( + runtime as unknown as { ptysById: Map } + ).ptysById.get(PTY_ID) + return pty?.lastOscTitleEpochMs ?? 0 +} + +describe('headless hook agent-status projection (#11761)', () => { + it('carries the hook state, tool and interactivePrompt to paired clients', async () => { + const agentStatus = await projectAgentStatus([hookRow()]) + + expect(agentStatus).toEqual( + expect.objectContaining({ + agentType: 'claude', + state: 'waiting', + prompt: 'Tabs or spaces?', + toolName: 'AskUserQuestion', + interactivePrompt: ASK_PROMPT, + providerSession: PROVIDER_SESSION + }) + ) + }) + + it('publishes no hook transport identity to clients', async () => { + const agentStatus = await projectAgentStatus([ + hookRow({ launchToken: 'lt-secret', promptInteractionKey: 'turn-1' }) + ]) + + expect(Object.keys(agentStatus ?? {}).sort()).toEqual([ + 'agentType', + 'interactivePrompt', + 'paneKey', + 'prompt', + 'providerSession', + 'state', + 'stateHistory', + 'stateStartedAt', + 'tabId', + 'terminalHandle', + 'terminalTitle', + 'toolName', + 'updatedAt', + 'worktreeId' + ]) + }) + + it('falls back to identity-only done once the hook row goes stale', async () => { + const stale = Date.now() - AGENT_STATUS_STALE_AFTER_MS - 1_000 + const agentStatus = await projectAgentStatus([ + hookRow({ receivedAt: stale, stateStartedAt: stale }) + ]) + + expect(agentStatus).toEqual( + expect.objectContaining({ state: 'done', prompt: '', providerSession: PROVIDER_SESSION }) + ) + expect(agentStatus).not.toHaveProperty('interactivePrompt') + }) + + // Resume-identity rows carry transport placeholders, not status. `agentType` + // deliberately admits pi's flavour of them — `live` must not, either flavour. + it.each(['claude', 'pi'])('never fabricates live state from a %s resume row', async (agent) => { + const agentStatus = await projectAgentStatus([ + hookRow({ agentType: agent, providerSessionOnly: true }) + ]) + + expect(agentStatus).toEqual(expect.objectContaining({ state: 'done', prompt: '' })) + expect(agentStatus).not.toHaveProperty('toolName') + expect(agentStatus).not.toHaveProperty('interactivePrompt') + }) + + // #12346: a row hydrated from last-status.json may describe a turn that ended while + // no receiver was up, so its recent `receivedAt` proves nothing. Publishing it would + // resurrect the question card on every restart, with no agent left to answer it. + it('refuses a hydrated unconfirmed row while keeping its resume identity', async () => { + const agentStatus = await projectAgentStatus([hookRow({ restoredUnconfirmed: true })]) + + expect(agentStatus).toEqual( + expect.objectContaining({ state: 'done', prompt: '', providerSession: PROVIDER_SESSION }) + ) + expect(agentStatus).not.toHaveProperty('interactivePrompt') + }) + + // #7970: a retained OSC 9999 row is the pane's own report and keeps precedence. + it('prefers a retained OSC 9999 row over the hook row', async () => { + const runtime = await createRuntimeWithHookRows([hookRow()]) + runtime.onPtyData( + PTY_ID, + '\x1b]9999;{"state":"working","prompt":"fix the tests","agentType":"claude"}\x07', + 100 + ) + + const result = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + const tab = result.tabs[0] + + expect(tab?.type === 'terminal' && tab.agentStatus).toEqual( + expect.objectContaining({ state: 'working', prompt: 'fix the tests' }) + ) + }) + + // #1437: `toolName` is inherited across hook events, so it cannot reopen a pane + // the shell has reclaimed — only a pending question may survive the suppression. + it('keeps a shell-reclaimed pane identity-only when the hook row has just a toolName', async () => { + const agentStatus = await projectAgentStatus( + [hookRow({ state: 'working', toolName: 'Bash', interactivePrompt: undefined })], + (runtime) => { + observePaneTitle(runtime, 'bash') + } + ) + + expect(agentStatus).toEqual(expect.objectContaining({ state: 'done', prompt: '' })) + expect(agentStatus).not.toHaveProperty('toolName') + }) + + it('keeps a pending question visible even under a non-agent title', async () => { + const agentStatus = await projectAgentStatus([hookRow()], (runtime) => { + observePaneTitle(runtime, 'bash') + }) + + expect(agentStatus).toEqual( + expect.objectContaining({ state: 'waiting', interactivePrompt: ASK_PROMPT }) + ) + }) + + // The title path is refreshed live; an older hook `done` must not erase it. + it('keeps the title-derived working state when the hook row predates the title', async () => { + const now = Date.now() + const agentStatus = await projectAgentStatus( + [ + hookRow({ + state: 'done', + prompt: '', + toolName: undefined, + interactivePrompt: undefined, + receivedAt: now - 60_000, + stateStartedAt: now - 60_000 + }) + ], + (runtime) => { + observePaneTitle(runtime, '⠋ Claude') + } + ) + + expect(agentStatus).toEqual(expect.objectContaining({ state: 'working', prompt: '' })) + }) + + // The other direction of the same guard: once the hook reports after the last + // spinner frame, its state is the newest evidence and must be published. + it('publishes a hook row received after the latest title observation', async () => { + const rows = [hookRow()] + const runtime = await createRuntimeWithHookRows(rows) + observePaneTitle(runtime, '⠋ Claude') + const reportedAt = lastOscTitleEpochMs(runtime) + 1_000 + rows[0] = hookRow({ + state: 'done', + prompt: '', + toolName: undefined, + interactivePrompt: undefined, + receivedAt: reportedAt, + stateStartedAt: reportedAt + }) + + const result = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + const tab = result.tabs[0] + + // `updatedAt` pins the row's provenance: the identity-only fallback would + // publish `working` (the stale spinner title) stamped with its own clock. + expect(tab?.type === 'terminal' && tab.agentStatus).toEqual( + expect.objectContaining({ state: 'done', updatedAt: reportedAt }) + ) + }) +}) + +// Nothing else republishes `session.tabs` when only a hook row changed, and a +// re-emit at an unchanged `snapshotVersion` is dropped by the client's gate. +describe('hook-driven session tabs republish (#11761)', () => { + it('bumps the snapshot version and emits through the coalescer', async () => { + const rows = [hookRow({ state: 'working', toolName: undefined, interactivePrompt: undefined })] + const runtime = await createRuntimeWithHookRows(rows) + const before = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + const events: { snapshotVersion: number }[] = [] + const unsubscribe = runtime.onMobileSessionTabsChanged((snapshot) => events.push(snapshot)) + + rows[0] = hookRow() + runtime.touchMobileSessionTabsForPane(PANE_KEY, WORKTREE_ID) + + await vi.waitFor(() => expect(events).toHaveLength(1)) + expect(events[0]!.snapshotVersion).toBeGreaterThan(before.snapshotVersion) + unsubscribe() + }) + + // Proven by version arithmetic rather than a timed silence: a pane that wrongly + // resolved to this workspace would bump the version a second time. + it('ignores a pane with no resolvable workspace', async () => { + const runtime = await createRuntimeWithHookRows([hookRow()]) + const before = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + const events: { snapshotVersion: number }[] = [] + const unsubscribe = runtime.onMobileSessionTabsChanged((snapshot) => events.push(snapshot)) + + runtime.touchMobileSessionTabsForPane(makePaneKey('gone-tab', UNKNOWN_LEAF_ID)) + runtime.touchMobileSessionTabsForPane(PANE_KEY, WORKTREE_ID) + + await vi.waitFor(() => expect(events).toHaveLength(1)) + expect(events[0]!.snapshotVersion).toBe(before.snapshotVersion + 1) + unsubscribe() + }) + + // Without this the live state published above would outlive the agent as a + // question card no client could ever dismiss. + it('retires the question card after the pane status is cleared', async () => { + const rows = [hookRow()] + const runtime = await createRuntimeWithHookRows(rows) + await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + const events: { tabs: { type: string; agentStatus?: unknown }[] }[] = [] + const unsubscribe = runtime.onMobileSessionTabsChanged((snapshot) => events.push(snapshot)) + + // What a pane-status clear leaves behind: the pane keeps no hook row at all. + rows.length = 0 + runtime.touchMobileSessionTabsForPane(PANE_KEY, WORKTREE_ID) + + await vi.waitFor(() => expect(events).toHaveLength(1)) + expect(events[0]!.tabs[0]!.agentStatus).toBeUndefined() + unsubscribe() + }) +}) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 849cfdc9c..f2f836c86 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -38,6 +38,7 @@ import { TerminalKittyKeyboardModeTracker } from '../../shared/terminal-kitty-ke import { AGENT_STATUS_STALE_AFTER_MS, isFreshNonDoneAgentStatus, + pickParsedAgentStatusPayload, type AgentStatusIpcPayload, type ParsedAgentStatusPayload, type AgentStatusOrchestrationContext, @@ -1248,6 +1249,10 @@ type RuntimePtyWorktreeRecord = { lastAgentStatus: AgentStatus | null lastOscTitle: string | null lastOscTitleAt: number | null + // Why a second stamp: `lastOscTitleAt` is a title-observation sequence number, + // comparable only to other title stamps. Anything that must date a live title + // against an off-pane clock (hook `receivedAt`) needs wall-clock ms. + lastOscTitleEpochMs: number | null managementTitle: string | null managementTitleAt: number | null controllerTitle: string | null @@ -1474,6 +1479,13 @@ type RuntimeAgentRowSnapshot = { updatedAt: number } +/** A hook row narrowed to what `session.tabs` publishes, shaped like the retained OSC + * snapshot so one projection branch can consume either carrier. */ +type HookLiveAgentRow = Pick< + RuntimeAgentRowSnapshot, + 'payload' | 'updatedAt' | 'stateStartedAt' | 'worktreeId' +> + type RuntimeHeadlessTerminal = { emulator: HeadlessEmulator // Why: serialize can race with newer writes appended to writeChain; return @@ -6500,22 +6512,46 @@ export class OrcaRuntimeService { if (!hasPtyBackedTab) { continue } - this.mobileSessionTabsByWorktree.set(worktreeId, { - ...snapshot, - snapshotVersion: snapshot.snapshotVersion + 1 - }) - if (options.immediate) { - // Why: readiness/lifecycle changes are structural and must not wait - // behind the title/status coalescing window. - this.notifyMobileSessionTabsChanged(worktreeId) - } else { - // Why: title/status flips several times a second under spinner-in-title - // agents. Coalesce the emit instead of fanning out every version. - this.mobileSessionTabsNotifyCoalescer.schedule(worktreeId) - } + this.touchMobileSessionTabsForWorktree(worktreeId, options) } } + /** Bump the snapshot version and emit, coalesced unless `immediate`. + * Why the bump: clients gate mirrored snapshots on a strictly increasing + * `snapshotVersion`, so a re-emit at the same version is silently dropped. */ + touchMobileSessionTabsForWorktree( + worktreeId: string, + options: { immediate?: boolean } = {} + ): void { + const snapshot = this.mobileSessionTabsByWorktree.get(worktreeId) + if (!snapshot) { + return + } + this.mobileSessionTabsByWorktree.set(worktreeId, { + ...snapshot, + snapshotVersion: snapshot.snapshotVersion + 1 + }) + if (options.immediate) { + // Why: readiness/lifecycle changes are structural and must not wait + // behind the title/status coalescing window. + this.notifyMobileSessionTabsChanged(worktreeId) + return + } + // Why: title/status flips several times a second under spinner-in-title + // agents. Coalesce the emit instead of fanning out every version. + this.mobileSessionTabsNotifyCoalescer.schedule(worktreeId) + } + + /** Republish the workspace snapshot after a pane's hook status changed. + * Hook rows feed the headless `agentStatus` projection, which nothing else touches. */ + touchMobileSessionTabsForPane(paneKey: string, worktreeId?: string | null): void { + const resolved = worktreeId ?? this.getTerminalWorktreeIdForPaneKey(paneKey) + if (!resolved) { + return + } + this.touchMobileSessionTabsForWorktree(resolved) + } + private mobileSessionSnapshotHasSurface( worktreeId: string, parentTabId: string, @@ -10031,6 +10067,7 @@ export class OrcaRuntimeService { const observedAt = this.nextTitleObservationSequence() pty.lastOscTitle = normalizedTitle pty.lastOscTitleAt = observedAt + pty.lastOscTitleEpochMs = Date.now() pty.lastAgentStatus = agentStatus this.setPtyManagementTitleFromObservedTitle(pty, normalizedTitle, observedAt) ptyRecordChanged = prevTitle !== normalizedTitle || prevStatus !== agentStatus @@ -10112,6 +10149,7 @@ export class OrcaRuntimeService { if (pty) { pty.lastOscTitle = null pty.lastOscTitleAt = null + pty.lastOscTitleEpochMs = null pty.lastAgentStatus = null pty.managementTitle = null pty.managementTitleAt = null @@ -28366,6 +28404,7 @@ export class OrcaRuntimeService { lastAgentStatus: null, lastOscTitle: null, lastOscTitleAt: null, + lastOscTitleEpochMs: null, managementTitle: null, managementTitleAt: null, controllerTitle: null, @@ -29783,11 +29822,15 @@ export class OrcaRuntimeService { ) : null const ptyTitleClassification = classifyAgentTitle(ptyTitle) - if (ptyTitle !== null && ptyTitleClassification !== 'agent') { + const nonAgentTitle = ptyTitle !== null && ptyTitleClassification !== 'agent' + if (nonAgentTitle) { // Why: non-agent title = shell reclaimed the pane; suppress to clear stuck spinners (#1437), though a live hook signal survives. const hasLiveHookSignal = retained?.payload.interactivePrompt != null || retained?.payload.toolName != null || + // Why: a pending question is never inherited across hook events (unlike + // `toolName`), so it proves the agent is parked on a selector right now. + hookRow.live?.payload.interactivePrompt != null || // Why: headless serve has no renderer to retain an OSC row, so a fresh hook // agentType is the only live signal a hook-only pane can offer — and an agent // that reports over HTTP need never set a title this gate would recognize. @@ -29811,18 +29854,19 @@ export class OrcaRuntimeService { ownerAgent ) // Why: OSC 9999 hook payload carries real state/prompt/agent; without preferring it, hook-only transitions never surfaced (#7970). - if (retained) { + const liveRow = retained ?? this.resolveHookLiveAgentRow(hookRow.live, pty, nonAgentTitle) + if (liveRow) { return { agentStatus: normalizeCompatibleAgentStatusEntryForOwner( { - ...retained.payload, + ...liveRow.payload, paneKey, - updatedAt: retained.updatedAt, - stateStartedAt: retained.stateStartedAt, + updatedAt: liveRow.updatedAt, + stateStartedAt: liveRow.stateStartedAt, stateHistory: [], ...(terminalHandle ? { terminalHandle } : {}), - ...((pty?.worktreeId ?? retained.worktreeId) - ? { worktreeId: pty?.worktreeId ?? retained.worktreeId } + ...((pty?.worktreeId ?? liveRow.worktreeId) + ? { worktreeId: pty?.worktreeId ?? liveRow.worktreeId } : {}), tabId: tab.parentTabId, terminalTitle, @@ -29832,8 +29876,9 @@ export class OrcaRuntimeService { ) } } - // A hook-only pane has no PTY status to date the row from; `done` with a - // now-stamp is the honest projection — the hook proves identity, not liveness. + // Last resort: the pane's hook evidence is identity only (resume rows, stale + // rows, or a row the freshness gate rejected). `done` with a now-stamp is the + // honest projection — and it is what retires the card once the agent exits. const now = pty?.lastOutputAt ?? Date.now() const agentType = ownerAgent ?? undefined return { @@ -29859,6 +29904,32 @@ export class OrcaRuntimeService { } } + /** Live hook status to publish for a pane with no retained OSC row, or null when the + * pane's hook evidence only proves identity. + * + * Why the freshness rule: `pty.lastAgentStatus` is title-derived and refreshed live, + * so an unconditional hook precedence would let a 29-minute-old `done` erase a pane + * that is visibly working. A pending `interactivePrompt` outranks title evidence at + * any age — the agent is parked on a selector until it answers — and it is also the + * only signal allowed to survive the #1437 non-agent-title suppression. */ + private resolveHookLiveAgentRow( + live: HookLiveAgentRow | null, + pty: RuntimePtyWorktreeRecord | null, + nonAgentTitle: boolean + ): HookLiveAgentRow | null { + if (!live) { + return null + } + if (live.payload.interactivePrompt != null) { + return live + } + // Why only this stamp: it is the sole wall-clock date on the pane's live title, + // so it is the only one comparable to a hook `receivedAt`. The sibling + // `titleUpdatedAt`/`lastOscTitleAt`/`paneTitleUpdatedAt` fields are observation + // sequence numbers, and comparing them here can only ever misfire. + return !nonAgentTitle && live.updatedAt >= (pty?.lastOscTitleEpochMs ?? 0) ? live : null + } + /** Hook-reported identity for this pane, newest wins per field. * * `providerSession` is deliberately unbounded: it is resume identity, not live @@ -29869,15 +29940,24 @@ export class OrcaRuntimeService { * `agentType` is bounded by the same staleness window the retained OSC path uses, * because it is the signal that claims an agent owns the pane at all. A user who * exits the agent leaves `pty.lastAgentStatus` behind forever, so an unbounded - * read would keep offering native chat for what is now a plain shell. */ + * read would keep offering native chat for what is now a plain shell. + * + * `live` is the newest fresh row's status fields — bounded like `agentType` because + * it asserts liveness, and unlike `agentType` it excludes `providerSessionOnly` rows + * with no Pi exception: those carry resume identity, and their status-shaped fields + * are documented transport placeholders that must never reach a client. It also drops + * `restoredUnconfirmed` rows, which `isFreshNonDoneAgentStatus` already treats as + * never-fresh; they still count as `agentType` identity evidence. */ private getHookAgentRowForPane(rows: readonly AgentStatusIpcPayload[]): { providerSession: AgentProviderSessionMetadata | null providerSessionAgentType: string | null providerSessionReceivedAt: number | null agentType: string | null + live: HookLiveAgentRow | null } { let session: AgentStatusIpcPayload | null = null let agent: AgentStatusIpcPayload | null = null + let live: AgentStatusIpcPayload | null = null const agentTypeFreshAfter = Date.now() - AGENT_STATUS_STALE_AFTER_MS // Why pane key only: the sibling `terminalHandle` arm this used to carry never // matched. `toAgentStatusIpcPayload` does not emit the field on the hook path @@ -29896,12 +29976,31 @@ export class OrcaRuntimeService { ) { agent = entry } + if ( + entry.providerSessionOnly !== true && + // Why: a row hydrated from last-status.json describes a turn that may have ended + // while no receiver was up (#12346), so its `receivedAt` cannot prove liveness — + // publishing it would resurrect a zombie question card across a restart. + entry.restoredUnconfirmed !== true && + entry.receivedAt >= agentTypeFreshAfter && + (!live || entry.receivedAt > live.receivedAt) + ) { + live = entry + } } return { providerSession: session?.providerSession ?? null, providerSessionAgentType: session?.agentType ?? null, providerSessionReceivedAt: session?.receivedAt ?? null, - agentType: agent?.agentType ?? null + agentType: agent?.agentType ?? null, + live: live + ? { + payload: pickParsedAgentStatusPayload(live), + updatedAt: live.receivedAt, + stateStartedAt: live.stateStartedAt ?? live.receivedAt, + ...(live.worktreeId ? { worktreeId: live.worktreeId } : {}) + } + : null } } diff --git a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx index 26aea01d0..15009c5e5 100644 --- a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx @@ -4,6 +4,8 @@ import '@testing-library/jest-dom/vitest' import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { NativeChatMessage } from '../../../../shared/native-chat-types' +import { applyCommandMarkerBoundaries } from './native-chat-pending' import type { NativeChatInteractiveSend } from './use-native-chat-interactive-send' const INITIAL_PROMPT = JSON.stringify({ @@ -19,8 +21,9 @@ const INITIAL_PROMPT = JSON.stringify({ const storeState = { agentStatusByPaneKey: { 'tab-1:leaf-1': { - interactivePrompt: INITIAL_PROMPT, - toolName: 'AskUserQuestion' + interactivePrompt: INITIAL_PROMPT as string | undefined, + toolName: 'AskUserQuestion' as string | undefined, + state: undefined as string | undefined } } } @@ -42,11 +45,17 @@ function renderCard(canSend = true): ReturnType { return render(cardElement(canSend)) } -function cardElement(canSend = true): React.JSX.Element { +function cardElement( + canSend = true, + messages?: readonly NativeChatMessage[], + onShowingQuestionChange?: (showing: boolean) => void +): React.JSX.Element { return ( { beforeEach(() => { vi.clearAllMocks() storeState.agentStatusByPaneKey['tab-1:leaf-1'].interactivePrompt = INITIAL_PROMPT + storeState.agentStatusByPaneKey['tab-1:leaf-1'].state = undefined }) afterEach(() => { @@ -166,3 +209,82 @@ describe('NativeChatInteractiveCard answer lifecycle', () => { expect(screen.getByText('Tabs or spaces?')).toBeInTheDocument() }) }) + +// A headless host, a relay gap, or a replay can leave the pane with no live +// `interactivePrompt` while the transcript still holds the unresolved call (#11761). +// Which asks the transcript still counts as pending (orphaned calls, turn boundaries) +// is the shared parser's contract — covered in `src/shared/native-chat-ask.test.ts`. +describe('NativeChatInteractiveCard transcript fallback', () => { + beforeEach(() => { + vi.clearAllMocks() + storeState.agentStatusByPaneKey['tab-1:leaf-1'].interactivePrompt = undefined + storeState.agentStatusByPaneKey['tab-1:leaf-1'].state = undefined + }) + + afterEach(() => { + cleanup() + }) + + it('renders a pending transcript ask and reports the composer replacement', () => { + const onShowingQuestionChange = vi.fn() + render(cardElement(true, [askCallMessage('Tabs or spaces?')], onShowingQuestionChange)) + + expect(screen.getByText('Tabs or spaces?')).toBeInTheDocument() + expect(onShowingQuestionChange).toHaveBeenCalledWith(true) + }) + + it('prefers live status over the transcript when both carry a prompt', () => { + storeState.agentStatusByPaneKey['tab-1:leaf-1'].interactivePrompt = INITIAL_PROMPT + render(cardElement(true, [askCallMessage('Stale transcript question?')])) + + expect(screen.getByText('Tabs or spaces?')).toBeInTheDocument() + expect(screen.queryByText('Stale transcript question?')).not.toBeInTheDocument() + }) + + it('still renders while the mirrored status says the agent is working', () => { + // Why no state gate: the mirrored status channel is exactly what fails in the + // reported topology, so keying the fallback on it would suppress the card. + storeState.agentStatusByPaneKey['tab-1:leaf-1'].state = 'working' + render(cardElement(true, [askCallMessage('Tabs or spaces?')])) + + expect(screen.getByText('Tabs or spaces?')).toBeInTheDocument() + }) + + it('stays dismissed after answering while the transcript call is still pending', () => { + mocks.sendAnswer.mockReturnValue({ settleAfterMs: 500, waitsForVerifiedDelivery: true }) + const messages = [askCallMessage('Tabs or spaces?')] + const rendered = render(cardElement(true, messages)) + + let settleDelivery: ((delivered: boolean) => void) | undefined + mocks.sendAnswer.mockImplementation((_prompt, _selections, onDeliverySettled) => { + settleDelivery = onDeliverySettled + return { settleAfterMs: 500, waitsForVerifiedDelivery: true } + }) + chooseSpacesAndSubmit() + act(() => settleDelivery?.(true)) + rendered.rerender(cardElement(true, messages)) + + expect(screen.queryByText('Tabs or spaces?')).not.toBeInTheDocument() + }) + + it('clears once the FIFO tool result lands', () => { + const rendered = render(cardElement(true, [askCallMessage('Tabs or spaces?')])) + expect(screen.getByText('Tabs or spaces?')).toBeInTheDocument() + + rendered.rerender(cardElement(true, [askCallMessage('Tabs or spaces?'), askResultMessage()])) + expect(screen.queryByText('Tabs or spaces?')).not.toBeInTheDocument() + }) + + // The view passes the command-boundary-trimmed messages, so an ask abandoned via + // `/clear` cannot come back as a permanent card sitting over the composer. + it('drops an ask abandoned by /clear', () => { + const abandoned = { ...askCallMessage('Tabs or spaces?'), timestamp: 100 } + const trimmed = applyCommandMarkerBoundaries( + [abandoned as unknown as NativeChatMessage], + [{ id: 'clear-1', command: '/clear', sentAt: 200 }] + ) + render(cardElement(true, trimmed)) + + expect(screen.queryByText('Tabs or spaces?')).not.toBeInTheDocument() + }) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx index 3c8973e6d..c02df5c13 100644 --- a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx +++ b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx @@ -1,5 +1,7 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { useAppStore } from '../../store' +import { extractPendingAsk } from '../../../../shared/native-chat-ask' +import type { NativeChatMessage } from '../../../../shared/native-chat-types' import { parseInteractivePrompt } from './native-chat-interactive-prompt' import { nativeChatCardDismissKey } from './native-chat-dismiss-key' import { NativeChatQuestionCard } from './NativeChatQuestionCard' @@ -19,17 +21,27 @@ import type { NativeChatInteractiveSend } from './use-native-chat-interactive-se * answered prompt by content key and hide the card until a genuinely different * prompt arrives. The dismissal resets once the prompt clears, so a later * (even identical) prompt shows again instead of staying hidden. + * + * The transcript is the second source (mobile parity again): a question that the + * live status never delivered — headless host, relay gap, replay, reconnect — + * still has its unresolved tool call in the messages we already parsed. Without + * it the composer stays mounted over a pane parked on a selector, and the next + * send commits the highlighted option instead of the typed message (#11761). */ export function NativeChatInteractiveCard({ paneKey, send, canSend, + messages, onShowingQuestionChange, answerInputRef }: { paneKey: string send: NativeChatInteractiveSend canSend: boolean + /** Transcript to fall back on when live status carries no prompt. Pass the + * command-boundary-trimmed messages so an ask abandoned via `/clear` stays gone. */ + messages?: readonly NativeChatMessage[] /** Reports whether a question card is on screen so the view can replace the * composer with it (the card's free-text row is the answer input). */ onShowingQuestionChange?: (showing: boolean) => void @@ -45,10 +57,14 @@ export function NativeChatInteractiveCard({ const interactiveToolName = useAppStore((s) => s.agentStatusByPaneKey[paneKey]?.toolName ?? null) const { sendAnswer, sendRaw, cancelPending, cancel } = send - const card = useMemo( - () => parseInteractivePrompt(interactivePrompt, interactiveToolName ?? undefined), - [interactivePrompt, interactiveToolName] - ) + const card = useMemo(() => { + const statusCard = parseInteractivePrompt(interactivePrompt, interactiveToolName ?? undefined) + if (statusCard || !messages) { + return statusCard + } + const prompt = extractPendingAsk(messages) + return prompt ? { kind: 'question' as const, prompt } : null + }, [interactivePrompt, interactiveToolName, messages]) const cardKey = useMemo(() => nativeChatCardDismissKey(card), [card]) const [dismissedKey, setDismissedKey] = useState(null) // A question answer is a paced multi-step write (body→Enter per question); keep diff --git a/src/renderer/src/components/native-chat/NativeChatView.tsx b/src/renderer/src/components/native-chat/NativeChatView.tsx index 90bf3be22..33a8ddecb 100644 --- a/src/renderer/src/components/native-chat/NativeChatView.tsx +++ b/src/renderer/src/components/native-chat/NativeChatView.tsx @@ -284,15 +284,13 @@ function NativeChatResolvedView({ ? sessionWithLaunchPrompt : { ...sessionWithLaunchPrompt, messages } }, [sessionWithLaunchPrompt, commandMarkers]) - const launchPromptVisible = - launchPromptMessage !== null && - sessionAfterCommandBoundaries.messages.some((message) => message.id === launchPromptMessage.id) const failedLaunchPromptMessageIds = useMemo(() => { - if (!paneLaunchPrompt?.failed || !launchPromptVisible || !launchPromptMessage) { + const id = paneLaunchPrompt?.failed ? launchPromptMessage?.id : null + if (!id || !sessionAfterCommandBoundaries.messages.some((message) => message.id === id)) { return undefined } - return new Set([launchPromptMessage.id]) - }, [paneLaunchPrompt?.failed, launchPromptMessage, launchPromptVisible]) + return new Set([id]) + }, [paneLaunchPrompt?.failed, launchPromptMessage?.id, sessionAfterCommandBoundaries.messages]) // The streaming preview bubble (if any) sits after the transcript but before // the optimistic user echoes — same order mobile uses. @@ -431,6 +429,7 @@ function NativeChatResolvedView({ paneKey={paneKey} send={interactiveSend} canSend={canSend} + messages={sessionAfterCommandBoundaries.messages} onShowingQuestionChange={setQuestionActive} answerInputRef={questionAnswerInputRef} /> diff --git a/src/shared/agent-status-types.ts b/src/shared/agent-status-types.ts index 1f2c36800..0d902ecb0 100644 --- a/src/shared/agent-status-types.ts +++ b/src/shared/agent-status-types.ts @@ -181,6 +181,31 @@ export type AgentStatusPayload = { */ export type ParsedAgentStatusPayload = Omit & { prompt: string } +/** + * Narrow an `AgentStatusIpcPayload` (or any superset) down to the status fields alone. + * Why: the IPC shape is flattened, so a spread cannot be narrowed structurally — copying + * a hook row into a client-visible projection would otherwise ship `launchToken`, + * `connectionId`, `promptInteractionKey` and `providerSessionOnly` to every paired client. + */ +export function pickParsedAgentStatusPayload( + row: ParsedAgentStatusPayload +): ParsedAgentStatusPayload { + return { + state: row.state, + prompt: row.prompt, + ...(row.agentType !== undefined ? { agentType: row.agentType } : {}), + ...(row.model !== undefined ? { model: row.model } : {}), + ...(row.toolName !== undefined ? { toolName: row.toolName } : {}), + ...(row.toolInput !== undefined ? { toolInput: row.toolInput } : {}), + ...(row.interactivePrompt !== undefined ? { interactivePrompt: row.interactivePrompt } : {}), + ...(row.lastAssistantMessage !== undefined + ? { lastAssistantMessage: row.lastAssistantMessage } + : {}), + ...(row.interrupted !== undefined ? { interrupted: row.interrupted } : {}), + ...(row.subagents !== undefined ? { subagents: row.subagents } : {}) + } +} + /** * Wire shape for agent-status IPC. Both `agentStatus:set` and `agentStatus:getSnapshot` * produce this shape so renderer call sites share a single `setAgentStatus` path. diff --git a/src/shared/native-chat-ask.test.ts b/src/shared/native-chat-ask.test.ts index e02f2bb9b..7b1754ad4 100644 --- a/src/shared/native-chat-ask.test.ts +++ b/src/shared/native-chat-ask.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import type { NativeChatBlock, NativeChatMessage } from './native-chat-types' +import { + NATIVE_CHAT_INTERRUPTED_STATUS_TEXT, + type NativeChatBlock, + type NativeChatMessage +} from './native-chat-types' import { extractPendingAsk, parseAskFromStatus } from './native-chat-ask' function message(id: string, blocks: NativeChatBlock[]): NativeChatMessage { @@ -14,6 +18,26 @@ function result(): NativeChatBlock { return { type: 'tool-result', output: 'ok' } } +/** The row the transcript decoders emit for an interrupted turn. */ +function interrupted(id: string): NativeChatMessage { + return { + id, + role: 'system', + blocks: [{ type: 'text', text: NATIVE_CHAT_INTERRUPTED_STATUS_TEXT }], + timestamp: 1, + source: 'transcript' + } +} + +function userTurn(id: string, text: string): NativeChatMessage { + return { id, role: 'user', blocks: [{ type: 'text', text }], timestamp: 1, source: 'transcript' } +} + +/** Claude delivers tool results on their own turn, which decodes as role 'tool'. */ +function toolTurn(id: string): NativeChatMessage { + return { id, role: 'tool', blocks: [result()], timestamp: 1, source: 'transcript' } +} + const QUESTIONS_INPUT = { questions: [{ question: 'Deploy?', options: [{ label: 'Yes' }, { label: 'No' }] }] } @@ -51,6 +75,66 @@ describe('extractPendingAsk', () => { expect(pending?.questions[0]?.question).toBe('Deploy?') }) + it('does not strand an answered ask behind a tool call orphaned by an interrupt', () => { + // ESC on a running tool: Claude writes its interrupt record instead of a + // tool result, so that call's FIFO slot never resolves (#11761). + const pending = extractPendingAsk([ + message('m1', [call('Bash', { command: 'sleep 999' })]), + interrupted('m2'), + message('m3', [call('AskUserQuestion', QUESTIONS_INPUT)]), + message('m4', [result()]) + ]) + expect(pending).toBeNull() + }) + + it('drops an ask abandoned by an interrupt', () => { + const pending = extractPendingAsk([ + message('m1', [call('AskUserQuestion', QUESTIONS_INPUT)]), + interrupted('m2') + ]) + expect(pending).toBeNull() + }) + + it('keeps an ask that is still awaiting its result after an earlier interrupt', () => { + const pending = extractPendingAsk([ + message('m1', [call('Bash', { command: 'sleep 999' })]), + interrupted('m2'), + message('m3', [call('AskUserQuestion', QUESTIONS_INPUT)]) + ]) + expect(pending?.questions[0]?.question).toBe('Deploy?') + }) + + it('drops an ask the user typed past instead of answering', () => { + // Real transcripts hold asks that never get a result because the user + // escaped the selector and sent a new prompt — the question is over. + const pending = extractPendingAsk([ + message('m1', [call('AskUserQuestion', QUESTIONS_INPUT)]), + userTurn('m2', 'never mind, do this instead'), + message('m3', [{ type: 'text', text: 'on it' }]) + ]) + expect(pending).toBeNull() + }) + + it('does not strand an answered ask behind an orphan left by a plain-text interrupt', () => { + // Claude also writes the interrupt as a bare user turn (no + // `interruptedMessageId`), which decodes as a user message, not a status row. + const pending = extractPendingAsk([ + message('m1', [call('Bash', { command: 'sleep 999' })]), + userTurn('m2', '[Request interrupted by user]'), + message('m3', [call('AskUserQuestion', QUESTIONS_INPUT)]), + toolTurn('m4') + ]) + expect(pending).toBeNull() + }) + + it('resolves an ask whose result arrives on its own tool-role turn', () => { + const pending = extractPendingAsk([ + message('m1', [call('AskUserQuestion', QUESTIONS_INPUT)]), + toolTurn('m2') + ]) + expect(pending).toBeNull() + }) + it('ignores malformed question payloads', () => { expect( extractPendingAsk([ diff --git a/src/shared/native-chat-ask.ts b/src/shared/native-chat-ask.ts index b602e5918..132afc270 100644 --- a/src/shared/native-chat-ask.ts +++ b/src/shared/native-chat-ask.ts @@ -4,7 +4,7 @@ import type { AskQuestion, InteractiveQuestionParser } from './native-chat-ask-types' -import type { NativeChatMessage } from './native-chat-types' +import { isInterruptedStatusMessage, type NativeChatMessage } from './native-chat-types' export type { AskOption, AskPrompt, AskQuestion, InteractiveQuestionParser } @@ -98,6 +98,16 @@ export function extractPendingAsk(messages: readonly NativeChatMessage[]): AskPr let pending: AskPrompt | null = null const outstanding: (AskPrompt | null)[] = [] for (const message of messages) { + // A new user turn (or an interrupt row) ends the turn that owns whatever + // calls are still in flight: their results never arrive, and `tool_use_id` + // is dropped at decode time so those orphans can never be matched by id. + // Without this reset one orphan shifts the result FIFO for the rest of the + // transcript and strands an answered ask as a permanent card over the + // composer (#11761). Claude's tool-result turns decode as role 'tool'. + if (message.role === 'user' || isInterruptedStatusMessage(message)) { + outstanding.length = 0 + pending = null + } for (const block of message.blocks) { if (block.type === 'tool-call') { const parsed = parseToolInput(block.name, block.input) diff --git a/src/shared/native-chat-types.ts b/src/shared/native-chat-types.ts index f88ff0137..96d01f641 100644 --- a/src/shared/native-chat-types.ts +++ b/src/shared/native-chat-types.ts @@ -131,6 +131,18 @@ export function isToolResultBlock(block: NativeChatBlock): block is NativeChatTo return block.type === 'tool-result' } +/** The provider-authored interrupt row the transcript decoders emit (Claude's + * `interruptedMessageId` record, Codex's `turn_aborted`). The turn it ends + * never delivers results for the tool calls it left in flight. */ +export function isInterruptedStatusMessage(message: NativeChatMessage): boolean { + return ( + message.role === 'system' && + message.blocks.some( + (block) => block.type === 'text' && block.text === NATIVE_CHAT_INTERRUPTED_STATUS_TEXT + ) + ) +} + export function isImageRefBlock(block: NativeChatBlock): block is NativeChatImageRefBlock { return block.type === 'image-ref' } diff --git a/tests/e2e/native-chat-ask-user-question-card.spec.ts b/tests/e2e/native-chat-ask-user-question-card.spec.ts new file mode 100644 index 000000000..a4cc2d7c4 --- /dev/null +++ b/tests/e2e/native-chat-ask-user-question-card.spec.ts @@ -0,0 +1,161 @@ +import { randomUUID } from 'node:crypto' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import type { Page } from '@stablyai/playwright-test' +import { test, expect } from './helpers/orca-app' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { waitForActivePaneHookDescriptor, waitForActiveTerminalManager } from './helpers/terminal' +import type { GlobalSettings } from '../../src/shared/types' + +const QUESTION = 'Tabs or spaces?' + +async function enableNativeChatSetting(page: Page): Promise { + await page.evaluate(async () => { + const nextSettings = await window.api.settings.set({ experimentalNativeChat: true }) + window.__store?.setState({ settings: nextSettings as GlobalSettings }) + }) +} + +// Why (#11761): reproduces the paired-headless topology from the client's side — +// live status arrives carrying agent identity and a working state, but with no +// `interactivePrompt`/`toolName`, which is exactly what the host projection +// dropped. The pending ask exists only in the transcript. +async function seedStatusWithoutAskPayload( + page: Page, + args: { paneKey: string; worktreeId: string; sessionId: string; transcriptPath: string } +): Promise { + await page.evaluate(({ paneKey, worktreeId, sessionId, transcriptPath }) => { + window.__store + ?.getState() + .setAgentStatus( + paneKey, + { state: 'working', prompt: 'AskUserQuestion card proof', agentType: 'claude' }, + 'Claude', + undefined, + { worktreeId }, + { providerSession: { key: 'session_id', id: sessionId, transcriptPath } } + ) + }, args) +} + +async function toggleTerminalTabToChatView( + page: Page, + args: { tabId: string; worktreeId: string } +): Promise { + await page.evaluate(({ tabId, worktreeId }) => { + const store = window.__store + if (!store) { + throw new Error('Store unavailable') + } + const state = store.getState() + const unifiedTab = (state.unifiedTabsByWorktree[worktreeId] ?? []).find( + (tab) => tab.contentType === 'terminal' && tab.entityId === tabId + ) + if (!unifiedTab) { + throw new Error('Unified terminal tab not found for chat toggle') + } + state.toggleTabViewMode(unifiedTab.id) + }, args) +} + +/** A transcript whose last assistant turn leaves an AskUserQuestion unanswered. */ +function pendingAskTranscript(args: { sessionId: string; userText: string }): string { + const userTime = new Date() + const assistantTime = new Date(userTime.getTime() + 2_000) + const lines = [ + { + sessionId: args.sessionId, + uuid: `${args.sessionId}-user`, + timestamp: userTime.toISOString(), + type: 'user', + message: { role: 'user', content: [{ type: 'text', text: args.userText }] } + }, + { + sessionId: args.sessionId, + uuid: `${args.sessionId}-assistant`, + timestamp: assistantTime.toISOString(), + type: 'assistant', + message: { + model: 'claude-opus-4', + content: [ + { type: 'text', text: 'Before I reformat the file I need one decision from you.' }, + { + type: 'tool_use', + name: 'AskUserQuestion', + input: { + questions: [ + { + question: QUESTION, + header: 'Style', + multiSelect: false, + options: [{ label: 'Tabs' }, { label: 'Spaces' }] + } + ] + } + } + ] + } + } + ] + return `${lines.map((line) => JSON.stringify(line)).join('\n')}\n` +} + +test.describe('Desktop chat AskUserQuestion card (#11761)', () => { + test('renders the answerable question card from the transcript when live status carries no ask', async ({ + orcaPage + }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage, 30_000) + + const descriptor = await waitForActivePaneHookDescriptor(orcaPage) + const [tabId] = descriptor.paneKey.split(':') + const sessionId = `e2e-ask-card-${randomUUID()}` + + const scratchDir = mkdtempSync(path.join(os.tmpdir(), 'orca-e2e-ask-card-')) + const transcriptPath = path.join(scratchDir, `${sessionId}.jsonl`) + const screenshotDir = path.join(process.cwd(), 'validation-screenshots', 'ask-user-question') + mkdirSync(screenshotDir, { recursive: true }) + + try { + const userText = 'Reformat the config file for me' + writeFileSync(transcriptPath, pendingAskTranscript({ sessionId, userText })) + + await enableNativeChatSetting(orcaPage) + await seedStatusWithoutAskPayload(orcaPage, { + paneKey: descriptor.paneKey, + worktreeId: descriptor.worktreeId, + sessionId, + transcriptPath + }) + await toggleTerminalTabToChatView(orcaPage, { tabId, worktreeId: descriptor.worktreeId }) + + await expect(orcaPage.locator('[data-native-chat-root="true"]')).toBeVisible({ + timeout: 15_000 + }) + await expect(orcaPage.getByText(userText)).toBeVisible({ timeout: 30_000 }) + + // The pre-fix build leaves the composer mounted here and never renders a + // card, so this assertion is what actually gates the regression. + await expect(orcaPage.getByText(QUESTION)).toBeVisible({ timeout: 10_000 }) + await expect(orcaPage.getByRole('button', { name: /Spaces/ })).toBeVisible() + await orcaPage.screenshot({ path: path.join(screenshotDir, '01-question-card.png') }) + + // The submit button reads "Skip" until an option is chosen; picking one is + // what proves the card is answerable rather than merely rendered. + await orcaPage.getByRole('button', { name: /Spaces/ }).click() + await expect(orcaPage.getByRole('button', { name: 'Send answer' })).toBeVisible() + await orcaPage.screenshot({ path: path.join(screenshotDir, '02-option-selected.png') }) + + await orcaPage.getByRole('button', { name: 'Send answer' }).click() + // The card owns the composer slot, so its disappearance is the visible + // signal that the answer was accepted and chat input came back. + await expect(orcaPage.getByText(QUESTION)).toHaveCount(0, { timeout: 20_000 }) + await orcaPage.screenshot({ path: path.join(screenshotDir, '03-answered.png') }) + } finally { + rmSync(scratchDir, { recursive: true, force: true }) + } + }) +})