From bba68b1bddf1276c8bd27ad4ca41efcbd4260321 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 8 Sep 2026 03:06:54 -0700 Subject: [PATCH 1/4] fix(pi): finish the dialog-wait signal on every surface (#19533) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(pi): carry modal waits to mobile and stop losing the dialog close Follow-ups to #18836, from its readiness review. - Paint pi's `!` needs-input state marker while a dialog is open, so the 80ms spinner frame stops repainting a working title over a mid-turn wait. Mobile and the CLI read the title, so they saw `working` where the desktop already showed `waiting`. - Keep the assistant reply that lands while a dialog is open. The modal guard cleared tool fields and the `message_end` capture with them, so a turn ending under a dialog left the preview on the previous message. - Report `ui_prompt_end` even when `ctx.isIdle()` throws on a runner the modal itself invalidated; the lost post stranded the pane on `waiting`. - Declare the `esbuild` the runtime smoke tool imports. * fix(pi): hold the needs-input marker until the dialog actually closes From review of the previous commit. - Settling under an open dialog no longer retires the marker. stopAnimation painted the plain title unconditionally, so agent_settled, a resolved agent_end, or an idle auto_compaction_end erased it mid-dialog — and because that also cleared the timer, the close then painted the plain title again and the wait was lost for good. - Track the dialog as a boolean, not a depth counter. Pi does its own nesting accounting and emits one pair per stack, which is what the status extension already assumes; two files disagreeing on that would have let an inner close release the outer wait. - Reset the flag on agent_start in both extensions. A turn cannot begin under a dialog holding input focus, so it is the one boundary that can recover a close that never arrived instead of pinning the pane forever. - Leave OMP to its approval events: it reports waits through those already, and painting the marker there too would put title and hook in disagreement. * fix(pi): do not ring the completion bell for a dialog that lost its close From review of the previous commit. - Report working, not done, when ui_prompt_end's isIdle() throws. done is not cosmetic: it reaches dispatchCompletion and fires the pane's finished notification, so a turn that is still running would announce itself. The real done still arrives from agent_end/agent_settled. - Keep the idle-maintenance frame cap accruing while a dialog holds the title, so a dialog left open cannot suspend the guard that stops a compaction spinner whose end event never came. - Guard the dialog handlers against a ctx without ui. The source is generated and untypechecked, and pi does not document the ctx it passes these two events; a TypeError there would surface on every dialog. * fix(pi): let a turn still complete after a dialog loses its runner From review of the previous commit. - Re-arm the completion report when ui_prompt_end's isIdle() throws. The fallback posts working, but the finished turn had already reported its end, so nothing further would ever fire and an idle pane sat spinning. - Count dialog depth in both extensions instead of trusting pi to emit one pair per stack. The guarantee is undocumented, and if it ever does emit a pair per dialog, an inner close would release the wait the outer dialog still holds. A counter costs nothing and drops the dependency. * fix(pi): decide a dialog close from turn state, not from a guess From review of the previous commit. - Fall back to agentEndReported when ctx.isIdle is unavailable or throws. The previous guess of working stranded the common case — a dialog opened at idle — because no later event was coming to correct it, and the agentEndReported re-arm it relied on could not fire either. A turn that already reported its end is not still running, and that is knowledge this process holds without needing ctx at all. - Only suppress spinner frames once the marker is actually painted. Pi may pass a ctx with no ui, and freezing the title on its last working frame is the opposite of what the marker is for. - Gate the titlebar dialog handlers on the OMP runtime too, not just the installed kind: a bare-shell OMP launch runs inside a pi-kind pane, and the status extension already defers there. Extracted that check so both extensions share it rather than carrying two copies. * fix(pi): treat a pane that never ran a turn as idle, not busy From review of the previous commit. - Track turn-in-flight separately from agentEndReported. That flag also dedupes the completion post, so it starts false on a pane that has not run a turn — which read as still-running and left a dialog opened before the first prompt spinning forever. - Retry the marker paint on each dialog open instead of only the outermost, so an outer ctx without ui cannot decide the whole nested stack goes unmarked. - Fall back to the opening ctx when the close carries no ui. Nothing else clears the needs-input marker, so the pane would have kept asking for attention until the next turn. * fix(pi): keep a dying dialog ctx from stranding the needs-input marker The close path paints through the ctx captured at open time, which is the one a session-switching modal is most likely to have invalidated. Guard both paint sites so a throw cannot reject the handler and leave the title on the needs-input marker, and make local turn state the floor for the status extension's idleness verdict instead of a fallback. * fix(pi): hold the dialog wait against pi's own title writes and lost closes Reviewed against real Pi 0.85.1 source rather than inference: - ctx.ui is a getter that calls assertActive() and throws once a session- replacing dialog invalidates the runner, so optional chaining never screened it out and the probe sat outside the try. A throw landed after the depth decrement but before markerPainted cleared, stranding the needs-input marker until the next turn. - Pi writes the same terminal title from its own writers with no event we observe, so the marker is now re-asserted rather than merely not overwritten, on a slow timer that outlives the spinner and its cap. - resetExtensionUI drops an open dialog without resolving its promise, so a replaced or reloaded session never emits the matching ui_prompt_end. Both extensions now release the wait on session_start and shutdown. * fix(pi): build the title inside the guard, not as an argument to it paintTitle caught the setTitle throw but not the two calls one argument to its left: pi.getSessionName() asserts runner liveness the same way ctx.ui does, and process.cwd() throws ENOENT once the worktree is unlinked under a live pane. Four of the six call sites are timer callbacks, where an escape is an uncaught exception and pi exits(1) through its own handler — so the cwd route was reachable today. paintTitle now takes a builder and runs it inside the existing try. * fix(pi): let only the pane-owning process assert the needs-input marker The spinner is harmlessly per-process, but the marker is status the pane reports, and child agents inherit ORCA_PANE_KEY. Gate the two dialog handlers on a PID claim, mirroring ORCA_PI_STATUS_OWNED in the status hook. --- package.json | 1 + pnpm-lock.yaml | 3 + .../pi/agent-status-extension-source.test.ts | 10 +- src/main/pi/agent-status-extension-source.ts | 4 +- src/main/pi/agent-status-handler-source.ts | 7 + .../agent-status-runtime-detection-source.ts | 43 ++- src/main/pi/agent-status-ui-prompt-source.ts | 31 +- src/main/pi/agent-status-ui-prompt.test.ts | 141 ++++++- src/main/pi/titlebar-extension-service.ts | 2 +- src/main/pi/titlebar-extension-source.test.ts | 351 +++++++++++++++++- src/main/pi/titlebar-extension-source.ts | 166 ++++++++- .../providers/pi-family-tool-fields.ts | 10 +- 12 files changed, 719 insertions(+), 50 deletions(-) diff --git a/package.json b/package.json index 321f2ada9ed..0536f4fd606 100644 --- a/package.json +++ b/package.json @@ -243,6 +243,7 @@ "electron-vite": "^5.0.0", "emoji-picker-react": "^4.19.1", "emojibase-data": "17.0.0", + "esbuild": "^0.25.12", "happy-dom": "^20.11.8", "html-to-image": "^1.11.13", "husky": "^9.1.7", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9ee9fff6785..7add63b397b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -366,6 +366,9 @@ importers: emojibase-data: specifier: 17.0.0 version: 17.0.0(emojibase@17.0.0) + esbuild: + specifier: ^0.25.12 + version: 0.25.12 happy-dom: specifier: ^20.11.8 version: 20.11.8 diff --git a/src/main/pi/agent-status-extension-source.test.ts b/src/main/pi/agent-status-extension-source.test.ts index fed179837db..fa9823d76dd 100644 --- a/src/main/pi/agent-status-extension-source.test.ts +++ b/src/main/pi/agent-status-extension-source.test.ts @@ -481,12 +481,14 @@ describe('getPiAgentStatusExtensionSource', () => { await handlerCall }) - it('leaves runtime shutdown to PTY teardown instead of reporting turn completion', () => { + it('leaves runtime shutdown to PTY teardown instead of reporting turn completion', async () => { const harness = createHarness({ kind: 'pi' }) - // Why: Pi emits session_shutdown for reload/new/resume/fork while its PTY - // stays alive. agent_end is the only extension event that proves done. - expect(harness.handlers.session_shutdown).toBeUndefined() + // Why: Pi emits session_shutdown for reload/new/resume/fork while its PTY stays + // alive. agent_end is the only extension event that proves done, so the handler + // exists solely to release a dialog Pi tore down without a close. + await harness.callHook('session_shutdown') + expect(harness.fetchMock).not.toHaveBeenCalled() }) it('bounds stalled delivery to one active request and the latest pending status', async () => { diff --git a/src/main/pi/agent-status-extension-source.ts b/src/main/pi/agent-status-extension-source.ts index 8b046e0db79..38775ca1973 100644 --- a/src/main/pi/agent-status-extension-source.ts +++ b/src/main/pi/agent-status-extension-source.ts @@ -101,7 +101,7 @@ export function getPiAgentStatusExtensionSource(kind: PiAgentKind = 'pi'): strin '// Orca receiver from building an unbounded queue of obsolete snapshots.', 'const HOOK_POST_TIMEOUT_MS = 1000', 'let activePost = false', - ...(kind === 'pi' ? ['let piUiPromptActive = false'] : []), + ...(kind === 'pi' ? ['let piUiPromptDepth = 0', 'let piTurnInFlight = false'] : []), 'let pendingPost: { hookEventName: string; extra: Record; metadata: Record; ompRuntime: boolean } | null = null', ...sessionMetadataSourceLines, '', @@ -167,7 +167,7 @@ export function getPiAgentStatusExtensionSource(kind: PiAgentKind = 'pi'): strin ' hookEventName,', // Why: every coalesced snapshot must retain an open modal, not just its start event. kind === 'pi' - ? ' extra: { ...extra, ...(!ompRuntime && piUiPromptActive ? { ui_prompt_active: true } : {}) },' + ? ' extra: { ...extra, ...(!ompRuntime && piUiPromptDepth > 0 ? { ui_prompt_active: true } : {}) },' : ' extra,', ' metadata: getPostSessionMetadata(ompRuntime),', ' ompRuntime,', diff --git a/src/main/pi/agent-status-handler-source.ts b/src/main/pi/agent-status-handler-source.ts index a769bfc74d2..9d02abbd78d 100644 --- a/src/main/pi/agent-status-handler-source.ts +++ b/src/main/pi/agent-status-handler-source.ts @@ -9,6 +9,7 @@ export function getPiAgentStatusHandlerSourceLines(kind: PiAgentKind): string[] ? [ " pi.on('session_start', (event, ctx) => {", ' updateSessionMetadata(ctx)', + ...(kind === 'pi' ? [' piUiPromptDepth = 0'] : []), ' // Why: /reload re-registers the active session, but it is not a', ' // turn boundary and must not clear the visible status or unread state.', " if (event.reason === 'reload') return", @@ -105,6 +106,9 @@ export function getPiAgentStatusHandlerSourceLines(kind: PiAgentKind): string[] ...captureSessionMetadata, ' clearPendingAgentEndCheck()', ' agentEndReported = false', + // Why: a turn cannot begin under a dialog holding input focus, so this is the one + // boundary that can recover a modal whose close never arrived. + ...(kind === 'pi' ? [' piUiPromptDepth = 0', ' piTurnInFlight = true'] : []), " post('agent_start')", ' })', '', @@ -168,6 +172,9 @@ export function getPiAgentStatusHandlerSourceLines(kind: PiAgentKind): string[] ' function postAgentEndOnce(): void {', ' if (agentEndReported) return', ' agentEndReported = true', + // Why: distinct from agentEndReported, which also dedupes the completion post and so + // starts false on a pane that has not run a turn yet — that pane is idle, not busy. + ...(kind === 'pi' ? [' piTurnInFlight = false'] : []), " post('agent_end')", ' }', '', diff --git a/src/main/pi/agent-status-runtime-detection-source.ts b/src/main/pi/agent-status-runtime-detection-source.ts index 5d9cdbf6de8..6ba5edb69b9 100644 --- a/src/main/pi/agent-status-runtime-detection-source.ts +++ b/src/main/pi/agent-status-runtime-detection-source.ts @@ -1,26 +1,15 @@ import type { PiAgentKind } from '../../shared/pi-agent-kind' -export function getPiAgentStatusRuntimeDetectionSourceLines(kind: PiAgentKind): string[] { - if (kind === 'prime-agent') { - return [ - `const CONFIGURED_HOOK_PATH = '/hook/${kind}'`, - '', - 'function isOmpRuntime(): boolean {', - ' return false', - '}', - '', - 'function resolveHookPath(_ompRuntime: boolean): string {', - ' return CONFIGURED_HOOK_PATH', - '}' - ] - } - +/** Why: a bare-shell OMP launch runs inside a pi-kind pane, so every extension that has to + * defer to OMP's own approval events needs this check — not just the status extension it + * was first written for. */ +export function getPiOmpRuntimeDetectionSourceLines(configuredHookPath: string): string[] { return [ 'function processName(value: unknown): string {', " return String(value || '').split(/[\\\\/]/).pop()?.toLowerCase() || ''", '}', '', - `const CONFIGURED_HOOK_PATH = '/hook/${kind}'`, + `const CONFIGURED_HOOK_PATH = '${configuredHookPath}'`, 'let cachedOmpRuntime: boolean | null = null', '', 'function isOmpRuntime(): boolean {', @@ -39,7 +28,27 @@ export function getPiAgentStatusRuntimeDetectionSourceLines(kind: PiAgentKind): " ['omp', 'omp.js', 'omp.sh', 'omp.cmd', 'omp.exe', 'omp.bat'].includes(name)", ' )', ' return cachedOmpRuntime', - '}', + '}' + ] +} + +export function getPiAgentStatusRuntimeDetectionSourceLines(kind: PiAgentKind): string[] { + if (kind === 'prime-agent') { + return [ + `const CONFIGURED_HOOK_PATH = '/hook/${kind}'`, + '', + 'function isOmpRuntime(): boolean {', + ' return false', + '}', + '', + 'function resolveHookPath(_ompRuntime: boolean): string {', + ' return CONFIGURED_HOOK_PATH', + '}' + ] + } + + return [ + ...getPiOmpRuntimeDetectionSourceLines(`/hook/${kind}`), '', 'function resolveHookPath(ompRuntime: boolean): string {', ' // Why: runtime detection keeps a bare-shell OMP launch from reporting as Pi.', diff --git a/src/main/pi/agent-status-ui-prompt-source.ts b/src/main/pi/agent-status-ui-prompt-source.ts index 2f1ed92c9ae..5790c5c30a7 100644 --- a/src/main/pi/agent-status-ui-prompt-source.ts +++ b/src/main/pi/agent-status-ui-prompt-source.ts @@ -1,6 +1,6 @@ import type { PiAgentKind } from '../../shared/pi-agent-kind' -/** Pi owns nested prompt depth and emits one pair around select/confirm/input/editor/custom. */ +/** Mirrors the titlebar extension's dialog tracking so both agree on when the wait ends. */ export function getPiAgentStatusUiPromptHandlerSourceLines(kind: PiAgentKind): string[] { if (kind !== 'pi') { return [] @@ -9,14 +9,35 @@ export function getPiAgentStatusUiPromptHandlerSourceLines(kind: PiAgentKind): s return [ " pi.on('ui_prompt_start', () => {", ' if (isOmpRuntime()) return', - ' piUiPromptActive = true', + ' piUiPromptDepth++', + ' if (piUiPromptDepth > 1) return', " post('ui_prompt_start')", ' })', '', " pi.on('ui_prompt_end', (_event, ctx) => {", - ' if (isOmpRuntime() || !piUiPromptActive) return', - ' piUiPromptActive = false', - " post('ui_prompt_end', { is_idle: ctx?.isIdle?.() === true })", + ' if (isOmpRuntime() || piUiPromptDepth === 0) return', + ' piUiPromptDepth--', + ' if (piUiPromptDepth > 0) return', + ' // Why: ctx.isIdle throws outright once a session-switching modal invalidates the', + ' // runner (it calls assertActive), so local turn state is the floor, not a fallback:', + ' // with no turn in flight, no later event is coming to correct a working verdict, so', + ' // only consult ctx when this process believes work is running.', + ' let isIdle = !piTurnInFlight', + ' try {', + " if (!isIdle && typeof ctx?.isIdle === 'function') isIdle = ctx.isIdle() === true", + ' } catch {', + ' // Why: a runner this very modal invalidated cannot answer; keep the local verdict.', + ' }', + " post('ui_prompt_end', { is_idle: isIdle })", + ' })', + '', + " pi.on('session_shutdown', () => {", + ' if (isOmpRuntime()) return', + ' // Why: pi tears an open dialog down through resetExtensionUI without resolving its', + ' // promise, so a replaced session never emits the matching ui_prompt_end and the wait', + ' // would stick forever. Reset without posting: shutdown is not a turn boundary, and', + ' // the session_start that follows republishes the corrected state.', + ' piUiPromptDepth = 0', ' })', '' ] diff --git a/src/main/pi/agent-status-ui-prompt.test.ts b/src/main/pi/agent-status-ui-prompt.test.ts index 4ab9341589e..d91ccc37b34 100644 --- a/src/main/pi/agent-status-ui-prompt.test.ts +++ b/src/main/pi/agent-status-ui-prompt.test.ts @@ -92,11 +92,21 @@ describe('Pi UI prompt status', () => { expect(harness.statuses.map((status) => status?.payload.state)).toEqual(['waiting', 'done']) }) - it('does not infer done when the context cannot establish idleness', async () => { + it('returns a pane that never ran a turn to done when idleness is unreadable', async () => { const harness = createHarness() await post(harness, 'ui_prompt_start') await post(harness, 'ui_prompt_end') - expect(harness.statuses.at(-1)?.payload.state).toBe('working') + // Why: no turn has started, so the pane is idle — reporting working would spin forever. + expect(harness.statuses.at(-1)?.payload.state).toBe('done') + }) + + it('trusts local turn state over a ctx that claims work on an idle pane', async () => { + const harness = createHarness() + await post(harness, 'ui_prompt_start') + await harness.callHook('ui_prompt_end', {}, { isIdle: () => false }) + await flushPosts() + // Why: no turn ever started, so nothing later would correct a working verdict. + expect(harness.statuses.at(-1)?.payload.state).toBe('done') }) it('lets the normal settlement hook finish work after a modal closes', async () => { @@ -118,20 +128,139 @@ describe('Pi UI prompt status', () => { const harness = createHarness() await post(harness, 'ui_prompt_start') harness.reload() - await post(harness, 'session_start', { reason: 'reload' }) await post(harness, 'tool_execution_end', { toolName: 'bash' }) + // Why: re-registering handlers is not a session boundary and must not lose the wait. expect(harness.statuses.at(-1)?.payload.state).toBe('waiting') }) - it('keeps a session-switching modal blocked until it actually closes', async () => { + it('releases a modal that a session replacement tore down without a close', async () => { const harness = createHarness() await post(harness, 'before_agent_start', { prompt: 'Old session prompt' }) await post(harness, 'ui_prompt_start') - await post(harness, 'session_start', { reason: 'switch' }) expect(harness.statuses.at(-1)?.payload.state).toBe('waiting') - expect(harness.statuses.at(-1)?.payload.prompt).toBe('') + // Why: pi hides the dialog through resetExtensionUI without resolving its promise, + // so no ui_prompt_end is ever emitted — these two boundaries are the only release. + await post(harness, 'session_shutdown') + await post(harness, 'session_start', { reason: 'switch' }) + await post(harness, 'tool_execution_end', { toolName: 'bash' }) + expect(harness.statuses.at(-1)?.payload.state).not.toBe('waiting') + }) + + it('releases a modal dropped by a reload that emits no shutdown', async () => { + const harness = createHarness() + await post(harness, 'ui_prompt_start') + await post(harness, 'session_start', { reason: 'reload' }) + await post(harness, 'tool_execution_end', { toolName: 'bash' }) + expect(harness.statuses.at(-1)?.payload.state).not.toBe('waiting') + }) + + it('still captures the assistant reply that lands while a modal is open', async () => { + const harness = createHarness() + await post(harness, 'agent_start') + await post(harness, 'message_end', { + message: { role: 'assistant', content: [{ type: 'text', text: 'Before modal' }] } + }) + await post(harness, 'ui_prompt_start') + await post(harness, 'message_end', { + message: { role: 'assistant', content: [{ type: 'text', text: 'Final reply' }] } + }) await harness.callHook('ui_prompt_end', {}, { isIdle: () => true }) await flushPosts() + expect(harness.statuses.at(-1)?.payload).toMatchObject({ + state: 'done', + lastAssistantMessage: 'Final reply' + }) + expect(harness.statuses.at(-1)?.payload.toolName).toBeUndefined() + expect(harness.statuses.at(-1)?.payload.interactivePrompt).toBeUndefined() + }) + + it('still reports the close when the modal invalidated its own runner', async () => { + const harness = createHarness() + await post(harness, 'ui_prompt_start') + await harness.callHook( + 'ui_prompt_end', + {}, + { + isIdle: () => { + throw new Error('extension runner is no longer active') + } + } + ) + await flushPosts() + // Why: a lost close would strand the pane on waiting; no turn is running, so done. + expect(harness.statuses.at(-1)?.payload.state).toBe('done') + }) + + it('keeps a mid-turn modal working when its runner throws on close', async () => { + const harness = createHarness() + await post(harness, 'agent_start') + await post(harness, 'ui_prompt_start') + await harness.callHook( + 'ui_prompt_end', + {}, + { + isIdle: () => { + throw new Error('extension runner is no longer active') + } + } + ) + await flushPosts() + // Why: the turn is still in flight, so done would ring the completion bell early. + expect(harness.statuses.at(-1)?.payload.state).toBe('working') + await post(harness, 'agent_settled') + expect(harness.statuses.at(-1)?.payload.state).toBe('done') + }) + + it('recovers on a new turn when a modal close was lost', async () => { + const harness = createHarness() + await post(harness, 'ui_prompt_start') + expect(harness.statuses.at(-1)?.payload.state).toBe('waiting') + // Why: a turn cannot begin under a dialog holding input focus, so this is recovery. + await post(harness, 'agent_start') + await post(harness, 'tool_execution_end', { toolName: 'bash' }) + expect(harness.statuses.at(-1)?.payload.state).toBe('working') + }) + + it('keeps the wait until the outermost of nested modals closes', async () => { + const harness = createHarness() + await post(harness, 'ui_prompt_start') + await post(harness, 'ui_prompt_start') + await harness.callHook('ui_prompt_end', {}, { isIdle: () => true }) + await flushPosts() + expect(harness.statuses.at(-1)?.payload.state).toBe('waiting') + await harness.callHook('ui_prompt_end', {}, { isIdle: () => true }) + await flushPosts() + expect(harness.statuses.at(-1)?.payload.state).toBe('done') + }) + + it('returns an idle pane to done when its modal lost the runner', async () => { + const harness = createHarness() + await post(harness, 'agent_start') + await post(harness, 'agent_settled') + expect(harness.statuses.at(-1)?.payload.state).toBe('done') + await post(harness, 'ui_prompt_start') + await harness.callHook( + 'ui_prompt_end', + {}, + { + isIdle: () => { + throw new Error('extension runner is no longer active') + } + } + ) + await flushPosts() + // Why: the turn already reported its end, so no later event is coming to correct a + // guess of working — fall back to what this process knows rather than strand it. + expect(harness.statuses.at(-1)?.payload.state).toBe('done') + }) + + it('keeps a mid-turn modal working when its close cannot read idleness', async () => { + const harness = createHarness() + await post(harness, 'agent_start') + await post(harness, 'ui_prompt_start') + await post(harness, 'ui_prompt_end') + expect(harness.statuses.at(-1)?.payload.state).toBe('working') + await post(harness, 'agent_settled') expect(harness.statuses.at(-1)?.payload.state).toBe('done') }) diff --git a/src/main/pi/titlebar-extension-service.ts b/src/main/pi/titlebar-extension-service.ts index 3a43ce4ac38..8a093096f4e 100644 --- a/src/main/pi/titlebar-extension-service.ts +++ b/src/main/pi/titlebar-extension-service.ts @@ -150,7 +150,7 @@ export class PiTitlebarExtensionService { if (kind !== 'prime-agent') { this.writeManagedExtension( join(extensionsDir, ORCA_PI_EXTENSION_FILE), - withOrcaManagedExtensionMarker(getPiTitlebarExtensionSource()) + withOrcaManagedExtensionMarker(getPiTitlebarExtensionSource(kind)) ) this.writeManagedExtension( join(extensionsDir, ORCA_PI_PREFILL_EXTENSION_FILE), diff --git a/src/main/pi/titlebar-extension-source.test.ts b/src/main/pi/titlebar-extension-source.test.ts index bee2e007c57..be21f8c6a16 100644 --- a/src/main/pi/titlebar-extension-source.test.ts +++ b/src/main/pi/titlebar-extension-source.test.ts @@ -3,6 +3,8 @@ import { runInNewContext } from 'node:vm' import ts from 'typescript-api' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { detectAgentStatusFromTitle } from '../../shared/agent-detection' +import type { PiAgentKind } from '../../shared/pi-agent-kind' import { getPiTitlebarExtensionSource } from './titlebar-extension-source' const BRAILLE_RE = /[⠀-⣿]/ @@ -23,8 +25,19 @@ type Harness = { const CWD = '/repo/orca-app' const SESSION = 'omp-session' const IDLE_TITLE = `π - ${SESSION} - orca-app` +const PROMPT_TITLE = `π ! ${SESSION} - orca-app` -function createHarness(options: { paneKey?: string; isIdle?: () => boolean } = {}): Harness { +function createHarness( + options: { + paneKey?: string + isIdle?: () => boolean + kind?: PiAgentKind + processTitle?: string + cwdImpl?: () => string + sessionNameImpl?: () => string + env?: Record + } = {} +): Harness { const titles: string[] = [] const ctx: TitlebarContext = { ui: { @@ -48,8 +61,11 @@ function createHarness(options: { paneKey?: string; isIdle?: () => boolean } = { module, exports: module.exports, process: { - env: { ORCA_PANE_KEY: options.paneKey ?? 'pane-1' }, - cwd: () => CWD + env: { ORCA_PANE_KEY: options.paneKey ?? 'pane-1', ...options.env }, + pid: options.env?.ORCA_PI_TITLE_MARKER_OWNED === undefined ? 111 : 222, + title: options.processTitle ?? 'pi', + argv: ['node', 'pi'], + cwd: options.cwdImpl ?? (() => CWD) }, console: { warn: vi.fn(), error: vi.fn(), log: vi.fn() }, Promise, @@ -61,7 +77,7 @@ function createHarness(options: { paneKey?: string; isIdle?: () => boolean } = { } as Record context.globalThis = context - const output = ts.transpileModule(getPiTitlebarExtensionSource(), { + const output = ts.transpileModule(getPiTitlebarExtensionSource(options.kind ?? 'pi'), { compilerOptions: { module: ts.ModuleKind.CommonJS, target: ts.ScriptTarget.ES2020 } }).outputText runInNewContext(output, context) @@ -76,7 +92,7 @@ function createHarness(options: { paneKey?: string; isIdle?: () => boolean } = { on(name: string, handler: HookHandler) { handlers[name] = handler }, - getSessionName: () => SESSION + getSessionName: options.sessionNameImpl ?? (() => SESSION) }) return { @@ -247,4 +263,329 @@ describe('getPiTitlebarExtensionSource', () => { expect(vi.getTimerCount()).toBe(0) expect(harness.lastTitle()).toBe(IDLE_TITLE) }) + + it('marks a mid-turn dialog as needing input and holds it against the spinner', async () => { + const harness = createHarness() + + await harness.callHook('agent_start') + await harness.callHook('ui_prompt_start') + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + expect(detectAgentStatusFromTitle(PROMPT_TITLE)).toBe('permission') + + // Why: the spinner interval keeps running, but must not repaint over the marker. + await vi.advanceTimersByTimeAsync(800) + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + + await harness.callHook('ui_prompt_end') + expect(harness.lastTitle()).toMatch(BRAILLE_RE) + expect(vi.getTimerCount()).toBe(1) + }) + + it('returns an idle pane to its plain title when the dialog closes', async () => { + const harness = createHarness() + + await harness.callHook('ui_prompt_start') + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + + await harness.callHook('ui_prompt_end') + expect(harness.lastTitle()).toBe(IDLE_TITLE) + expect(vi.getTimerCount()).toBe(0) + }) + + it('only the outermost of nested dialogs moves the title', async () => { + const harness = createHarness() + + await harness.callHook('agent_start') + await harness.callHook('ui_prompt_start') + await harness.callHook('ui_prompt_start') + await harness.callHook('ui_prompt_end') + // Why: the outer dialog still holds input focus. + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + + await harness.callHook('ui_prompt_end') + expect(harness.lastTitle()).toMatch(BRAILLE_RE) + }) + + it('ignores an unmatched dialog close', async () => { + const harness = createHarness() + + await harness.callHook('agent_start') + const titleCount = harness.titles.length + await harness.callHook('ui_prompt_end') + expect(harness.titles.length).toBe(titleCount) + }) + + it.each(['agent_settled', 'session_shutdown'])( + 'keeps the marker when %s lands under an open dialog', + async (name) => { + const harness = createHarness() + + await harness.callHook('agent_start') + await harness.callHook('ui_prompt_start') + await harness.callHook(name) + // Why: settling does not answer the dialog, so the pane still needs the user. + const expected = name === 'session_shutdown' ? IDLE_TITLE : PROMPT_TITLE + expect(harness.lastTitle()).toBe(expected) + // Why: settling stops the spinner but must leave the marker re-assert running, or + // pi's own next title write would silently retire a dialog that is still open. + expect(vi.getTimerCount()).toBe(name === 'session_shutdown' ? 0 : 1) + } + ) + + it('keeps the marker across an idle compaction that finishes under a dialog', async () => { + const harness = createHarness() + + await harness.callHook('ui_prompt_start') + await harness.callHook('auto_compaction_start', { reason: 'idle' }) + await harness.callHook('auto_compaction_end') + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + }) + + it('recovers the spinner on a new turn when a dialog close was lost', async () => { + const harness = createHarness() + + await harness.callHook('ui_prompt_start') + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + + // Why: a turn cannot start under a dialog holding input focus, so this is recovery. + await harness.callHook('agent_start') + await vi.advanceTimersByTimeAsync(80) + expect(harness.lastTitle()).toMatch(BRAILLE_RE) + }) + + it('leaves the marker to OMP approval events instead of painting it', () => { + expect(createHarness({ kind: 'omp' }).handlers.ui_prompt_start).toBeUndefined() + }) + + it('still caps idle maintenance while a dialog holds the title', async () => { + const harness = createHarness() + + await harness.callHook('auto_compaction_start', { reason: 'idle' }) + await harness.callHook('ui_prompt_start') + // Why: an open dialog must not suspend the cap that stops a stranded spinner. + vi.advanceTimersByTime(301_000) + + // Why: the spinner is capped, but the marker re-assert survives it — the dialog is + // still open, so the pane must keep reporting that it needs input. + expect(vi.getTimerCount()).toBe(1) + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + }) + + it('survives a dialog event that carries no ui context', async () => { + const harness = createHarness() + + await harness.callHook('agent_start') + await expect(harness.handlers.ui_prompt_start?.({}, undefined)).resolves.toBeUndefined() + await expect(harness.handlers.ui_prompt_end?.({}, undefined)).resolves.toBeUndefined() + }) + + it('keeps spinning when the dialog event could not paint the marker', async () => { + const harness = createHarness() + + await harness.callHook('agent_start') + await harness.handlers.ui_prompt_start?.({}, undefined) + // Why: suppressing frames without a marker would freeze the title mid-spinner, which + // still reads as working — the opposite of what the marker is for. + await vi.advanceTimersByTimeAsync(160) + expect(harness.lastTitle()).toMatch(BRAILLE_RE) + }) + + it('marks a nested dialog when the outer one could not paint', async () => { + const harness = createHarness() + + await harness.callHook('agent_start') + await harness.handlers.ui_prompt_start?.({}, undefined) + await harness.callHook('ui_prompt_start') + // Why: the outer ctx cannot decide that the whole stack stays unmarked. + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + }) + + it('clears the marker through the opening ctx when the close carries none', async () => { + const harness = createHarness() + + await harness.callHook('ui_prompt_start') + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + await harness.handlers.ui_prompt_end?.({}, undefined) + // Why: otherwise the pane asks for attention until the next turn. + expect(harness.lastTitle()).toBe(IDLE_TITLE) + }) + + it('does not reject when the dialog ctx can no longer paint', async () => { + const harness = createHarness() + const throwing = { + ui: { + setTitle: () => { + throw new Error('extension runner is no longer active') + } + } + } + + await expect(harness.handlers.ui_prompt_start?.({}, throwing)).resolves.toBeUndefined() + // Why: the marker never went up, so the spinner must not stay suppressed. + await harness.callHook('agent_start') + await vi.advanceTimersByTimeAsync(80) + expect(harness.lastTitle()).toMatch(BRAILLE_RE) + }) + + it('does not reject when the captured ctx dies before the dialog closes', async () => { + const harness = createHarness() + let live = true + const dying = { + ui: { + setTitle: (title: string) => { + if (!live) { + throw new Error('extension runner is no longer active') + } + harness.titles.push(title) + } + } + } + + await harness.handlers.ui_prompt_start?.({}, dying) + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + live = false + // Why: the close carries no ui, so it falls back to the ctx the modal invalidated. + await expect(harness.handlers.ui_prompt_end?.({}, undefined)).resolves.toBeUndefined() + // Why: a later turn still recovers a clean title through a live ctx. + await harness.callHook('agent_start') + await vi.advanceTimersByTimeAsync(80) + expect(harness.lastTitle()).toMatch(BRAILLE_RE) + }) + + it('does not strand the marker when the closing ctx throws on ui access', async () => { + const harness = createHarness() + // Why: pi's ctx.ui is a getter that calls assertActive(); a session-replacing dialog + // invalidates the runner, so reading ctx.ui throws rather than yielding undefined. + const stale = { + get ui(): never { + throw new Error('This extension ctx is stale') + } + } + + await harness.callHook('ui_prompt_start') + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + + await expect(harness.handlers.ui_prompt_end?.({}, stale as never)).resolves.toBeUndefined() + // Why: the opening ctx still paints, so the pane stops asking for input. + expect(harness.lastTitle()).toBe(IDLE_TITLE) + + // Why: a stranded markerPainted would suppress every later working frame. + await harness.callHook('agent_start') + await vi.advanceTimersByTimeAsync(80) + expect(harness.lastTitle()).toMatch(BRAILLE_RE) + }) + + it('does not reject when the opening ctx throws on ui access', async () => { + const harness = createHarness() + const stale = { + get ui(): never { + throw new Error('This extension ctx is stale') + } + } + + await harness.callHook('agent_start') + await expect(harness.handlers.ui_prompt_start?.({}, stale as never)).resolves.toBeUndefined() + // Why: no marker went up, so the spinner must keep running. + await vi.advanceTimersByTimeAsync(80) + expect(harness.lastTitle()).toMatch(BRAILLE_RE) + }) + + it('re-asserts the marker when pi repaints the title under a dialog', async () => { + const harness = createHarness() + + await harness.callHook('agent_start') + await harness.callHook('ui_prompt_start') + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + + // Why: pi repaints on session_info_changed/rebindCurrentSession with no event we see, + // so a marker that is merely "not overwritten by us" would be silently lost. + harness.titles.push('π - other - orca-app') + await vi.advanceTimersByTimeAsync(80) + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + }) + + it('re-asserts the marker on an idle pane with no spinner running', async () => { + const harness = createHarness() + + await harness.callHook('ui_prompt_start') + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + + // Why: no turn is running, so renderFrame never fires — only the slow re-assert can + // undo a title pi writes from session_info_changed or its update-check restore. + harness.titles.push('\u03c0 - other - orca-app') + await vi.advanceTimersByTimeAsync(1000) + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + + await harness.callHook('ui_prompt_end') + expect(harness.lastTitle()).toBe(IDLE_TITLE) + expect(vi.getTimerCount()).toBe(0) + }) + + it('releases the marker when a session replacement drops the dialog', async () => { + const harness = createHarness() + + await harness.callHook('ui_prompt_start') + expect(harness.lastTitle()).toBe(PROMPT_TITLE) + + // Why: pi hides the dialog without resolving it, so no close is coming. + await harness.callHook('session_start', { reason: 'switch' }) + expect(vi.getTimerCount()).toBe(0) + await harness.callHook('agent_start') + await vi.advanceTimersByTimeAsync(80) + expect(harness.lastTitle()).toMatch(BRAILLE_RE) + }) + + it('survives a deleted cwd instead of crashing the pi process', async () => { + const harness = createHarness({ + cwdImpl: () => { + throw new Error('ENOENT: uv_cwd') + } + }) + + // Why: these run inside setInterval callbacks, where an escape is an uncaught + // exception and pi exits(1) through its own uncaughtException handler. + await expect(harness.callHook('agent_start')).resolves.toBeUndefined() + await expect(harness.callHook('ui_prompt_start')).resolves.toBeUndefined() + // Why: an unguarded throw in the interval would surface here as an unhandled error. + await vi.advanceTimersByTimeAsync(2000) + await expect(harness.callHook('ui_prompt_end')).resolves.toBeUndefined() + await expect(harness.callHook('agent_settled')).resolves.toBeUndefined() + }) + + it('survives a session name that throws on a stale runtime', async () => { + let live = true + const harness = createHarness({ + sessionNameImpl: () => { + if (!live) { + throw new Error('This extension API is stale') + } + return SESSION + } + }) + + await harness.callHook('agent_start') + await harness.callHook('ui_prompt_start') + live = false + await vi.advanceTimersByTimeAsync(2000) + await expect(harness.callHook('ui_prompt_end')).resolves.toBeUndefined() + }) + + it('leaves the needs-input marker to the process that owns the pane', async () => { + // Why: child agents inherit ORCA_PANE_KEY, and a second process asserting the marker + // would report needs-input for a pane it does not speak for. + const harness = createHarness({ env: { ORCA_PI_TITLE_MARKER_OWNED: '111' } }) + + await harness.callHook('agent_start') + await harness.callHook('ui_prompt_start') + await vi.advanceTimersByTimeAsync(1000) + expect(harness.titles).not.toContain(PROMPT_TITLE) + expect(harness.lastTitle()).toMatch(BRAILLE_RE) + }) + + it('leaves an OMP runtime to its own approval events', () => { + const harness = createHarness({ processTitle: 'omp' }) + + expect(harness.handlers.ui_prompt_start).toBeDefined() + expect(() => harness.handlers.ui_prompt_start?.({}, undefined)).not.toThrow() + }) }) diff --git a/src/main/pi/titlebar-extension-source.ts b/src/main/pi/titlebar-extension-source.ts index a41eafb896f..7fc15c191bc 100644 --- a/src/main/pi/titlebar-extension-source.ts +++ b/src/main/pi/titlebar-extension-source.ts @@ -1,7 +1,54 @@ +import type { PiAgentKind } from '../../shared/pi-agent-kind' +import { getPiOmpRuntimeDetectionSourceLines } from './agent-status-runtime-detection-source' + export const ORCA_PI_EXTENSION_FILE = 'orca-titlebar-spinner.ts' -export function getPiTitlebarExtensionSource(): string { +export function getPiTitlebarExtensionSource(kind: PiAgentKind = 'pi'): string { + // Why: OMP reports input waits through its own approval events, which the status + // extension already maps, and it writes this same marker natively. The runtime check + // matters as well as the kind: a bare-shell OMP launch runs inside a pi-kind pane. + const uiPromptHandlers = + kind === 'pi' + ? [ + " pi.on('ui_prompt_start', async (_event, ctx) => {", + ' if (isOmpRuntime() || !ownsMarker) return', + ' promptDepth++', + ' // Why: retry on every open rather than only the outermost, so an outer ctx', + ' // that could not paint cannot decide the whole stack stays unmarked.', + ' if (markerPainted) return', + ' const painter = resolvePainter(ctx)', + ' // Why: only hold the spinner off once the marker is actually up, or a ctx', + ' // that cannot paint would freeze the title on its last working frame.', + " if (!paintTitle(painter, () => getMarkedTitle(pi, '!'))) return", + ' markerPainted = true', + ' promptCtx = painter', + ' startMarkerReassert(painter)', + ' })', + '', + " pi.on('ui_prompt_end', async (_event, ctx) => {", + ' if (isOmpRuntime() || !ownsMarker || promptDepth === 0) return', + ' promptDepth--', + ' if (promptDepth > 0) return', + ' // Why: the opening ctx already painted once, so a close whose own ctx is stale', + ' // does not leave the needs-input marker up until the next turn.', + ' const painter = resolvePainter(ctx) ?? promptCtx', + ' markerPainted = false', + ' promptCtx = null', + ' stopMarkerReassert()', + ' // Why: a still-live turn resumes its spinner in place; otherwise the pane is idle', + ' // and must drop the needs-input marker rather than keep asking for attention.', + ' if (timer) {', + ' renderFrame(painter)', + ' return', + ' }', + ' paintTitle(painter, () => getBaseTitle(pi))', + ' })', + '' + ] + : [] + return [ + ...(kind === 'pi' ? [...getPiOmpRuntimeDetectionSourceLines(`/hook/${kind}`), ''] : []), 'const BRAILLE_FRAMES = [', " '\\u280b',", " '\\u2819',", @@ -16,36 +63,111 @@ export function getPiTitlebarExtensionSource(): string { ']', '', 'const FRAME_INTERVAL_MS = 80', + '// Why: pi repaints the title from its own writers (session_info_changed, the win32', + '// update-check restore) with no event we observe, so the marker has to be re-asserted', + '// even when no spinner frame is due. Coarse on purpose: it only rewrites one string.', + 'const MARKER_REASSERT_MS = 1000', 'const AGENT_END_IDLE_RECHECK_MS = 25', 'const AGENT_END_IDLE_RECHECK_MAX_MS = 250', '// Why: a failed idle compaction can end without auto_compaction_end, and no agent turn will', '// close a maintenance spinner — cap it so idle maintenance cannot strand a working title.', 'const IDLE_COMPACTION_MAX_FRAMES = Math.ceil(300000 / FRAME_INTERVAL_MS)', '', - 'function getBaseTitle(pi) {', + '// Why: `-` is the plain separator; `!` is the state marker Orca reads as needs-input', + '// (src/shared/pi-state-title-marker.ts), so mobile and the CLI see the wait too.', + 'function getMarkedTitle(pi, marker) {', ' const cwd = process.cwd().split(/[\\\\/]/).filter(Boolean).at(-1) || process.cwd()', ' const session = pi.getSessionName()', - ' return session ? `\\u03c0 - ${session} - ${cwd}` : `\\u03c0 - ${cwd}`', + ' return session', + ' ? `\\u03c0 ${marker} ${session} - ${cwd}`', + ' : `\\u03c0 ${marker} ${cwd}`', + '}', + '', + 'function getBaseTitle(pi) {', + " return getMarkedTitle(pi, '-')", + '}', + '', + '// Why: the ctx.ui pi passes is a getter that calls assertActive() and throws once a', + '// session-replacing dialog invalidates the runner; optional chaining cannot screen', + '// that out. Read it behind a try and never mutate state before a paint has succeeded.', + 'function resolvePainter(ctx) {', + ' try {', + " return typeof ctx?.ui?.setTitle === 'function' ? ctx : null", + ' } catch {', + ' return null', + ' }', + '}', + '', + '// Why: buildTitle runs inside the try because it is not safe either — getSessionName()', + '// calls assertActive() and process.cwd() throws ENOENT once the worktree is deleted.', + '// Most call sites are timer callbacks, where an escape is an uncaught exception and pi', + '// exits(1) through its own uncaughtException handler.', + 'function paintTitle(ctx, buildTitle) {', + ' if (!ctx) return false', + ' try {', + ' ctx.ui.setTitle(buildTitle())', + ' return true', + ' } catch {', + ' return false', + ' }', '}', '', 'export default function (pi) {', ' if (!process.env.ORCA_PANE_KEY) return', + ...(kind === 'pi' + ? [ + ' // Why: child agents inherit the pane env, and the spinner is harmlessly', + ' // per-process — but the needs-input marker is status the pane reports, so only', + ' // one process may assert it. Mirrors ORCA_PI_STATUS_OWNED in the status hook.', + ' const markerOwnerPid = process.env.ORCA_PI_TITLE_MARKER_OWNED', + ' const ownsMarker = !markerOwnerPid || markerOwnerPid === String(process.pid)', + ' if (ownsMarker) process.env.ORCA_PI_TITLE_MARKER_OWNED = String(process.pid)' + ] + : []), + ' let timer = null', ' let frameIndex = 0', ' // Why: only idle maintenance owns a spinner of its own. A threshold compaction runs', ' // inside an agent turn, whose spinner must outlive it, and any newer start clears the', ' // marker so a late idle completion cannot stop current work (#16470).', ' let idleCompactionOwnsSpinner = false', + ' // Why: pi already collapses nested prompts into one start/end pair, so this counter', + ' // guards a close that never arrives, not nesting. A new turn cannot start under a', + ' // dialog holding input focus, so agent_start doubles as recovery.', + ' let promptDepth = 0', + ' let markerPainted = false', + ' let promptCtx = null', + ' // Why: a separate handle from `timer`, which clearAnimation() nulls — the marker must', + ' // survive a turn settling, a shutdown of the spinner, and the idle-maintenance cap.', + ' let markerTimer = null', ' let pendingAgentEndCheck = null', ' let pendingAgentEndContext = null', ' let agentEndIdleRecheckMs = AGENT_END_IDLE_RECHECK_MS', '', + ' function resetPromptState() {', + ' stopMarkerReassert()', + ' promptDepth = 0', + ' markerPainted = false', + ' promptCtx = null', + ' }', + '', ' function clearPendingAgentEndCheck() {', ' if (pendingAgentEndCheck !== null) clearTimeout(pendingAgentEndCheck)', ' pendingAgentEndCheck = null', ' pendingAgentEndContext = null', ' }', '', + ' function stopMarkerReassert() {', + ' if (markerTimer) clearInterval(markerTimer)', + ' markerTimer = null', + ' }', + '', + ' function startMarkerReassert(ctx) {', + ' stopMarkerReassert()', + " markerTimer = setInterval(() => paintTitle(ctx, () => getMarkedTitle(pi, '!')), MARKER_REASSERT_MS)", + " if (typeof markerTimer.unref === 'function') markerTimer.unref()", + ' }', + '', ' function clearAnimation() {', ' if (timer) {', ' clearInterval(timer)', @@ -58,19 +180,35 @@ export function getPiTitlebarExtensionSource(): string { ' function stopAnimation(ctx) {', ' clearPendingAgentEndCheck()', ' clearAnimation()', - ' ctx.ui.setTitle(getBaseTitle(pi))', + ' // Why: settling under an open dialog still leaves the pane waiting on the user, so', + ' // the idle title must not retire the marker the dialog is holding.', + " paintTitle(ctx, () => (markerPainted ? getMarkedTitle(pi, '!') : getBaseTitle(pi)))", ' }', '', ' function renderFrame(ctx) {', + ' // Why: the maintenance cap runs before the dialog guard so a dialog left open', + ' // cannot suspend it; stopAnimation keeps the marker while a dialog is open.', ' if (idleCompactionOwnsSpinner && frameIndex >= IDLE_COMPACTION_MAX_FRAMES) {', ' stopAnimation(ctx)', ' return', ' }', - ' const frame = BRAILLE_FRAMES[frameIndex % BRAILLE_FRAMES.length]', - ' const cwd = process.cwd().split(/[\\\\/]/).filter(Boolean).at(-1) || process.cwd()', - ' const session = pi.getSessionName()', - ' const title = session ? `${frame} \\u03c0 - ${session} - ${cwd}` : `${frame} \\u03c0 - ${cwd}`', - ' ctx.ui.setTitle(title)', + ' // Why: an 80ms working frame would repaint over the needs-input marker within one', + ' // tick, so a mid-turn dialog would still look busy everywhere the title is the', + ' // only evidence. Re-assert rather than skip: pi repaints the title on its own', + ' // (session_info_changed, resetExtensionUI, rebindCurrentSession) and would', + ' // otherwise wipe the marker with nothing to restore it. The frame still counts,', + ' // so the cap above keeps accruing in wall-clock.', + ' if (markerPainted) {', + " paintTitle(ctx, () => getMarkedTitle(pi, '!'))", + ' frameIndex++', + ' return', + ' }', + ' paintTitle(ctx, () => {', + ' const frame = BRAILLE_FRAMES[frameIndex % BRAILLE_FRAMES.length]', + ' const cwd = process.cwd().split(/[\\\\/]/).filter(Boolean).at(-1) || process.cwd()', + ' const session = pi.getSessionName()', + ' return session ? `${frame} \\u03c0 - ${session} - ${cwd}` : `${frame} \\u03c0 - ${cwd}`', + ' })', ' frameIndex++', ' }', '', @@ -101,9 +239,17 @@ export function getPiTitlebarExtensionSource(): string { ' }', '', " pi.on('agent_start', async (_event, ctx) => {", + ' resetPromptState()', ' startAnimation(ctx)', ' })', '', + ' // Why: pi drops an open dialog through resetExtensionUI without resolving its promise,', + ' // so a replaced or reloaded session never sends the matching close. Both boundaries', + ' // prove no dialog from the old session is still on screen.', + " pi.on('session_start', async () => {", + ' resetPromptState()', + ' })', + '', ' // Why: modern Pi/OMP emit agent_end mid-run and only settle later, so settlement is the', ' // authoritative completion boundary. Legacy runtimes never emit it, so agent_end stays.', " pi.on('agent_settled', async (_event, ctx) => {", @@ -126,6 +272,7 @@ export function getPiTitlebarExtensionSource(): string { " if (typeof pendingAgentEndCheck.unref === 'function') pendingAgentEndCheck.unref()", ' })', '', + ...uiPromptHandlers, " pi.on('auto_compaction_start', async (event, ctx) => {", " if (event?.reason !== 'idle') return", ' // Why: the idle worker can fire against a turn that just started, and reason alone does', @@ -142,6 +289,7 @@ export function getPiTitlebarExtensionSource(): string { ' })', '', " pi.on('session_shutdown', async (_event, ctx) => {", + ' resetPromptState()', ' stopAnimation(ctx)', ' })', '}', diff --git a/src/shared/agent-hook-listener/providers/pi-family-tool-fields.ts b/src/shared/agent-hook-listener/providers/pi-family-tool-fields.ts index d20b4aedbf7..65c61981868 100644 --- a/src/shared/agent-hook-listener/providers/pi-family-tool-fields.ts +++ b/src/shared/agent-hook-listener/providers/pi-family-tool-fields.ts @@ -36,7 +36,15 @@ export function extractPiToolFields( eventName === 'ui_prompt_start' || eventName === 'ui_prompt_end') ) { - return clearActiveToolFieldsUpdate() + // Why: the reply is the agent's own text, not modal content, so a turn that finishes + // while a dialog is open must not leave the preview stuck on the previous message. + const assistantText = + eventName === 'message_end' && hookPayload.role === 'assistant' + ? readString(hookPayload, 'text') + : undefined + return assistantText + ? { ...clearActiveToolFieldsUpdate(), lastAssistantMessage: assistantText } + : clearActiveToolFieldsUpdate() } if ( eventName === 'tool_call' || From 6108ce617c8696c3d52a97d29911aed630a22e78 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:53:26 -0700 Subject: [PATCH 2/4] Organize activity menu into filter and view sections (#19547) * refactor: organize activity menu into sections and change toggle callbac Restructure the activity thread options menu to use explicit boolean callbacks instead of toggle functions (rename onToggleUnread to onUnreadOnlyChange) and organize options into logical "Filters" and "View" sections. Remove descriptive tooltips for compact mode and unread filter. Rename ActivityScopeFilterMenuSections to ActivityScopeFilterMenuItems and shift layout responsibility to parent component. * i18n * fix issues * i18n * Hide empty Filters section in activity options menu - Extract visibility logic into reusable hook `useActivityScopeFilterMenuItemsVisible` to avoid duplication - Only render Filters label and items when filters are available, preventing empty section in dropdown - Improves UX by not showing unused menu sections --- .../ActivityThreadOptionsMenu.test.tsx | 54 +++-- .../activity-scope-filter-controls.tsx | 59 ++++-- .../activity/activity-thread-options-menu.tsx | 185 +++++++----------- .../components/sidebar/SidebarAgentsList.tsx | 2 +- src/renderer/src/i18n/locales/en.json | 10 +- src/renderer/src/i18n/locales/es.json | 6 +- src/renderer/src/i18n/locales/fr.json | 4 + src/renderer/src/i18n/locales/ja.json | 6 +- src/renderer/src/i18n/locales/ko.json | 6 +- src/renderer/src/i18n/locales/zh.json | 7 +- 10 files changed, 166 insertions(+), 173 deletions(-) diff --git a/src/renderer/src/components/activity/ActivityThreadOptionsMenu.test.tsx b/src/renderer/src/components/activity/ActivityThreadOptionsMenu.test.tsx index 3f753f3d69d..6ba2b7906a4 100644 --- a/src/renderer/src/components/activity/ActivityThreadOptionsMenu.test.tsx +++ b/src/renderer/src/components/activity/ActivityThreadOptionsMenu.test.tsx @@ -167,9 +167,18 @@ describe('ActivityThreadOptionsMenu', () => { expect(document.body.textContent).toContain('Agent') }) - it('explains compact mode on hover', async () => { + it('updates compact mode without closing the menu', async () => { + const onCompactModeChange = vi.fn() await act(async () => { - root.render() + root.render( + + + + ) }) const trigger = container.querySelector( @@ -179,19 +188,20 @@ describe('ActivityThreadOptionsMenu', () => { trigger?.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Enter' })) }) - const compactMode = document.querySelector('[role="menuitemcheckbox"]') + const compactMode = Array.from( + document.querySelectorAll('[role="menuitemcheckbox"]') + ).find((item) => item.textContent === 'Compact mode') await act(async () => { - compactMode?.dispatchEvent(new Event('pointermove', { bubbles: true })) + compactMode?.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Enter' })) }) - expect(document.body.textContent).toContain( - 'Shows shorter thread rows with one-line titles and two-line status messages.' - ) + expect(onCompactModeChange).toHaveBeenCalledWith(true) + expect(document.body.textContent).toContain('Compact mode') }) it('puts persisted search visibility and unread actions in the menu', async () => { const onShowSearchChange = vi.fn() - const onToggleUnread = vi.fn() + const onUnreadOnlyChange = vi.fn() await act(async () => { root.render( @@ -203,7 +213,7 @@ describe('ActivityThreadOptionsMenu', () => { showSearch onShowSearchChange={onShowSearchChange} unreadOnly={false} - onToggleUnread={onToggleUnread} + onUnreadOnlyChange={onUnreadOnlyChange} /> ) @@ -230,8 +240,8 @@ describe('ActivityThreadOptionsMenu', () => { expect(onShowSearchChange).toHaveBeenCalledWith(false) }) - it('explains show unread threads only on hover without a second unread state marker', async () => { - const onToggleUnread = vi.fn() + it('updates the unread filter without closing the menu', async () => { + const onUnreadOnlyChange = vi.fn() await act(async () => { root.render( @@ -241,7 +251,7 @@ describe('ActivityThreadOptionsMenu', () => { onCompactModeChange={vi.fn()} onMarkAllThreadsRead={vi.fn()} unreadOnly={false} - onToggleUnread={onToggleUnread} + onUnreadOnlyChange={onUnreadOnlyChange} /> ) @@ -254,15 +264,15 @@ describe('ActivityThreadOptionsMenu', () => { trigger?.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Enter' })) }) - const unreadItem = document.querySelector('[role="menuitemcheckbox"]') + const unreadItem = Array.from( + document.querySelectorAll('[role="menuitemcheckbox"]') + ).find((item) => item.textContent === 'Show unread only') await act(async () => { - unreadItem?.dispatchEvent(new Event('pointermove', { bubbles: true })) + unreadItem?.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Enter' })) }) - expect(document.body.textContent).toContain( - 'Filters the activity list to show only threads with unread updates.' - ) - expect(document.querySelector('[data-unread-dot]')).toBeNull() + expect(onUnreadOnlyChange).toHaveBeenCalledWith(true) + expect(document.body.textContent).toContain('Show unread only') }) it('renders show child agents checkbox when onShowChildAgentsChange is provided', async () => { @@ -281,6 +291,14 @@ describe('ActivityThreadOptionsMenu', () => { trigger?.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Enter' })) }) + const childAgentsItem = Array.from( + document.querySelectorAll('[role="menuitemcheckbox"]') + ).find((item) => item.textContent === 'Show child agents') + await act(async () => { + childAgentsItem?.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Enter' })) + }) + + expect(onShowChildAgentsChange).toHaveBeenCalledWith(true) expect(document.body.textContent).toContain('Show child agents') }) }) diff --git a/src/renderer/src/components/activity/activity-scope-filter-controls.tsx b/src/renderer/src/components/activity/activity-scope-filter-controls.tsx index a9e75e10483..e90ec1f3322 100644 --- a/src/renderer/src/components/activity/activity-scope-filter-controls.tsx +++ b/src/renderer/src/components/activity/activity-scope-filter-controls.tsx @@ -1,6 +1,6 @@ import React from 'react' import { useAppStore } from '@/store' -import { DropdownMenuItem, DropdownMenuSeparator } from '@/components/ui/dropdown-menu' +import { DropdownMenuItem } from '@/components/ui/dropdown-menu' import { translate } from '@/i18n/i18n' import SidebarRepositoryFilterSection from '@/components/sidebar/SidebarRepositoryFilterSection' import { SidebarHostScopeMenuSection } from '@/components/sidebar/SidebarHostScopeMenuSection' @@ -11,12 +11,30 @@ import { import { useSidebarHostScopeOptions } from '@/components/sidebar/use-sidebar-host-scope-options' /** - * Host/project scope controls for the Agents activity surfaces. State is the - * persisted agents-view scope (agentsVisibleHostIds / agentsFilterRepoIds), - * deliberately separate from the workspace-nav filters. + * Whether {@link ActivityScopeFilterMenuItems} renders anything. + * Why exported: the parent owns the Filters label and separator, so it has to + * know whether the section would be empty. */ -export function ActivityScopeFilterMenuSections(): React.JSX.Element | null { +export function useActivityScopeFilterMenuItemsVisible(): boolean { const repos = useAppStore((s) => s.repos) + const agentsVisibleHostIds = useAppStore((s) => s.agentsVisibleHostIds) + const agentsFilterRepoIds = useAppStore((s) => s.agentsFilterRepoIds) + const { hostOptions } = useSidebarHostScopeOptions() + return ( + agentsVisibleHostIds !== null || + agentsFilterRepoIds.length > 0 || + shouldShowHostScopeControls(hostOptions) || + repos.length > 1 + ) +} + +/** + * Host/project scope items for the Agents activity surfaces. State is the + * persisted agents-view scope (agentsVisibleHostIds / agentsFilterRepoIds), + * deliberately separate from the workspace-nav filters. The parent owns the + * Filters label and separator. + */ +export function ActivityScopeFilterMenuItems(): React.JSX.Element | null { const agentsVisibleHostIds = useAppStore((s) => s.agentsVisibleHostIds) const setAgentsVisibleHostIds = useAppStore((s) => s.setAgentsVisibleHostIds) const agentsFilterRepoIds = useAppStore((s) => s.agentsFilterRepoIds) @@ -24,25 +42,14 @@ export function ActivityScopeFilterMenuSections(): React.JSX.Element | null { const { hostOptions } = useSidebarHostScopeOptions() const showHostScopeControls = shouldShowHostScopeControls(hostOptions) const hasScopeFilter = agentsVisibleHostIds !== null || agentsFilterRepoIds.length > 0 + const visible = useActivityScopeFilterMenuItemsVisible() - if (!hasScopeFilter && !showHostScopeControls && repos.length <= 1) { + if (!visible) { return null } + return ( <> - {hasScopeFilter ? ( - { - setAgentsVisibleHostIds(null) - setAgentsFilterRepoIds([]) - }} - > - {translate( - 'auto.components.activity.ActivityScopeFilterControls.resetScope', - 'Show all hosts and projects' - )} - - ) : null} {showHostScopeControls ? ( - + {hasScopeFilter ? ( + { + setAgentsVisibleHostIds(null) + setAgentsFilterRepoIds([]) + }} + > + {translate( + 'auto.components.activity.ActivityScopeFilterControls.resetScope', + 'Show all hosts and projects' + )} + + ) : null} ) } diff --git a/src/renderer/src/components/activity/activity-thread-options-menu.tsx b/src/renderer/src/components/activity/activity-thread-options-menu.tsx index bd398986bd3..5a9bd3bc59c 100644 --- a/src/renderer/src/components/activity/activity-thread-options-menu.tsx +++ b/src/renderer/src/components/activity/activity-thread-options-menu.tsx @@ -1,21 +1,12 @@ import React from 'react' -import { - Check, - CheckCheck, - GitFork, - Layers, - ListChecks, - ListFilter, - Rows3, - Search, - Trash2 -} from 'lucide-react' +import { CheckCheck, ListFilter, Trash2 } from 'lucide-react' import { Button } from '@/components/ui/button' import { DropdownMenu, DropdownMenuCheckboxItem, DropdownMenuContent, DropdownMenuItem, + DropdownMenuLabel, DropdownMenuRadioGroup, DropdownMenuRadioItem, DropdownMenuSeparator, @@ -27,12 +18,19 @@ import { import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { translate } from '@/i18n/i18n' import { - ActivityScopeFilterMenuSections, - useActivityScopeFilterActive + ActivityScopeFilterMenuItems, + useActivityScopeFilterActive, + useActivityScopeFilterMenuItemsVisible } from './activity-scope-filter-controls' import type { ActivityGroupBy } from './activity-thread-types' -const ALIGNED_CHECKBOX_ITEM_CLASS = 'pl-2 [&>span.absolute]:hidden' +const GROUP_BY_OPTIONS = [ + 'none', + 'status', + 'project', + 'worktree', + 'agent' +] as const satisfies readonly ActivityGroupBy[] function getActivityGroupByLabel(groupBy: ActivityGroupBy): string { switch (groupBy) { @@ -63,7 +61,7 @@ export function ActivityThreadOptionsMenu({ showSearch = false, onShowSearchChange, unreadOnly = false, - onToggleUnread + onUnreadOnlyChange }: { groupBy?: ActivityGroupBy onGroupByChange?: (groupBy: ActivityGroupBy) => void @@ -78,10 +76,14 @@ export function ActivityThreadOptionsMenu({ showSearch?: boolean onShowSearchChange?: (showSearch: boolean) => void unreadOnly?: boolean - onToggleUnread?: () => void + onUnreadOnlyChange?: (unreadOnly: boolean) => void }): React.JSX.Element { const skipCloseAutoFocusRef = React.useRef(false) const scopeFilterActive = useActivityScopeFilterActive() + const scopeFilterItemsVisible = useActivityScopeFilterMenuItemsVisible() + const hasFilters = Boolean( + onUnreadOnlyChange || onShowChildAgentsChange || scopeFilterItemsVisible + ) const optionsLabel = scopeFilterActive ? translate( 'auto.components.activity.ActivityPrototypePage.threadListOptionsFiltered', @@ -126,7 +128,7 @@ export function ActivityThreadOptionsMenu({ side="right" align="start" sideOffset={8} - className="w-56" + className="w-60" onCloseAutoFocus={(event) => { if (skipCloseAutoFocusRef.current) { event.preventDefault() @@ -134,70 +136,56 @@ export function ActivityThreadOptionsMenu({ } }} > - {onShowSearchChange || onToggleUnread ? ( + {hasFilters ? ( <> - {onShowSearchChange ? ( + + {translate( + 'auto.components.activity.ActivityPrototypePage.filtersSection', + 'Filters' + )} + + {onUnreadOnlyChange ? ( { - skipCloseAutoFocusRef.current = checked === true - onShowSearchChange(checked === true) - }} + checked={unreadOnly} + onCheckedChange={(checked) => onUnreadOnlyChange(checked === true)} + onSelect={(event) => event.preventDefault()} > - - - {translate( - 'auto.components.activity.ActivityPrototypePage.showSearch', - 'Show search' - )} - - {showSearch ? : null} + {translate( + 'auto.components.activity.ActivityPrototypePage.showUnreadOnly', + 'Show unread only' + )} ) : null} - {onToggleUnread ? ( - - - onToggleUnread()} - onSelect={(event) => event.preventDefault()} - > - - - {translate( - 'auto.components.activity.ActivityPrototypePage.showUnreadOnly', - 'Show unread only' - )} - - {unreadOnly ? : null} - - - - {translate( - 'auto.components.activity.ActivityPrototypePage.unreadOnlyDescription', - 'Filters the activity list to show only threads with unread updates.' - )} - - + {onShowChildAgentsChange ? ( + onShowChildAgentsChange(checked === true)} + onSelect={(event) => event.preventDefault()} + > + {translate( + 'auto.components.activity.ActivityPrototypePage.showChildAgents', + 'Show child agents' + )} + ) : null} + ) : null} - + + {translate('auto.components.activity.ActivityPrototypePage.viewSection', 'View')} + {groupBy && onGroupByChange ? ( - - + {translate( 'auto.components.activity.ActivityPrototypePage.770d458144', 'Group by' )} - + {getActivityGroupByLabel(groupBy)} @@ -207,73 +195,36 @@ export function ActivityThreadOptionsMenu({ value={groupBy} onValueChange={(value) => onGroupByChange(value as ActivityGroupBy)} > - {[ - ['none', 'None', 'auto.components.activity.ActivityPrototypePage.none'], - ['status', 'Status', 'auto.components.activity.ActivityPrototypePage.4a3986b200'], - [ - 'project', - 'Project', - 'auto.components.activity.ActivityPrototypePage.8c3b621ddf' - ], - [ - 'worktree', - 'Worktree', - 'auto.components.activity.ActivityPrototypePage.b29191b3e0' - ], - ['agent', 'Agent', 'auto.components.activity.ActivityPrototypePage.f6396e1f85'] - ].map(([value, label, key]) => ( + {GROUP_BY_OPTIONS.map((value) => ( event.preventDefault()} > - {translate(key, label)} + {getActivityGroupByLabel(value)} ))} ) : null} - - - onCompactModeChange(checked === true)} - onSelect={(event) => event.preventDefault()} - > - - - {translate( - 'auto.components.activity.ActivityPrototypePage.f70e4bec47', - 'Compact mode' - )} - - {compactMode ? : null} - - - - {translate( - 'auto.components.activity.ActivityPrototypePage.compactModeDescription', - 'Shows shorter thread rows with one-line titles and two-line status messages.' - )} - - - {onShowChildAgentsChange ? ( + onCompactModeChange(checked === true)} + onSelect={(event) => event.preventDefault()} + > + {translate('auto.components.activity.ActivityPrototypePage.f70e4bec47', 'Compact mode')} + + {onShowSearchChange ? ( onShowChildAgentsChange(checked === true)} - onSelect={(event) => event.preventDefault()} + checked={showSearch} + onCheckedChange={(checked) => { + const show = checked === true + skipCloseAutoFocusRef.current = show + onShowSearchChange(show) + }} > - - - {translate( - 'auto.components.activity.ActivityPrototypePage.showChildAgents', - 'Show child agents' - )} - - {showChildAgents ? : null} + {translate('auto.components.activity.ActivityPrototypePage.showSearch', 'Show search')} ) : null} {onMarkAllThreadsRead || onClearCompleted ? ( diff --git a/src/renderer/src/components/sidebar/SidebarAgentsList.tsx b/src/renderer/src/components/sidebar/SidebarAgentsList.tsx index 3f22d7fc2da..cd782457d64 100644 --- a/src/renderer/src/components/sidebar/SidebarAgentsList.tsx +++ b/src/renderer/src/components/sidebar/SidebarAgentsList.tsx @@ -182,7 +182,7 @@ export default function SidebarAgentsList({ showSearch={showSearch} onShowSearchChange={handleShowSearchChange} unreadOnly={readFilter === 'unread'} - onToggleUnread={() => setReadFilter(readFilter === 'unread' ? 'all' : 'unread')} + onUnreadOnlyChange={(unreadOnly) => setReadFilter(unreadOnly ? 'unread' : 'all')} />, optionsTarget ) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 8e6d862e3b5..fb8e84b7ef7 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -16244,8 +16244,6 @@ "5651b216c6": "Unknown project", "22b22034bc": "Standalone terminal unavailable in Activity.", "afdc2139a8": "Agent terminal closed. Open a new terminal in this workspace to continue.", - "compactModeDescription": "Shows shorter thread rows with one-line titles and two-line status messages.", - "unreadOnlyDescription": "Filters the activity list to show only threads with unread updates.", "clearCompleted": "Clear completed", "none": "None", "search": "Search", @@ -16265,7 +16263,9 @@ "idle": "Idle", "unverifiable": "No recent update", "permission": "Needs attention" - } + }, + "filtersSection": "Filters", + "viewSection": "View" }, "clearCompleted": { "clearedOne": "Cleared 1 completed agent", @@ -16282,8 +16282,8 @@ "dc708f3eff": "Close agents" }, "ActivityScopeFilterControls": { - "resetScope": "Show all hosts and projects", - "hiddenCount": "{{value0}} hidden" + "hiddenCount": "{{value0}} hidden", + "resetScope": "Show all hosts and projects" }, "ActivityThreadHoverCard": { "pathCopied": "Path copied to clipboard", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index f1539887478..2ee1716ea20 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -14188,8 +14188,6 @@ "5651b216c6": "Proyecto desconocido", "22b22034bc": "Terminal independiente no disponible en Actividad.", "afdc2139a8": "Terminal de Agent cerrada. Abre una nueva terminal en este workspace para continuar.", - "compactModeDescription": "Muestra filas de hilo más cortas con títulos de una línea y mensajes de estado de dos líneas.", - "unreadOnlyDescription": "Filtra la lista de actividad para mostrar solo hilos con actualizaciones sin leer.", "clearCompleted": "Borrar completados", "none": "Ninguno", "search": "Buscar", @@ -14207,7 +14205,9 @@ "idle": "Inactivo", "unverifiable": "Sin actualizaciones recientes", "permission": "Requiere atención" - } + }, + "filtersSection": "Filtros", + "viewSection": "Vista" }, "ActivityScopeFilterControls": { "resetScope": "Mostrar todos los hosts y proyectos" diff --git a/src/renderer/src/i18n/locales/fr.json b/src/renderer/src/i18n/locales/fr.json index 0c5f01f067e..6113d606c90 100644 --- a/src/renderer/src/i18n/locales/fr.json +++ b/src/renderer/src/i18n/locales/fr.json @@ -15464,6 +15464,10 @@ "4616ea39fd": "Aller à l'espace de travail", "threadListOptionsFiltered": "Options de la liste des fils, filtres actifs", "showSearch": "Afficher la recherche", + "showUnreadOnly": "Afficher uniquement les fils non lus", + "showChildAgents": "Afficher les agents enfants", + "filtersSection": "Filtres", + "viewSection": "Affichage", "59b131fbd9": "Marquer le fil comme non lu", "beb2c19173": "Non lus", "5651b216c6": "Projet inconnu", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index cd9c24ea7d8..f0c1666d911 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -14188,8 +14188,6 @@ "5651b216c6": "不明なプロジェクト", "22b22034bc": "スタンドアロンターミナルはアクティビティでは使用できません。", "afdc2139a8": "Agent ターミナルが閉じられました。続行するには、このワークスペースで新規ターミナルを開いてください。", - "compactModeDescription": "1 行のタイトルと 2 行のステータスメッセージで短いスレッド行を表示します。", - "unreadOnlyDescription": "未読の更新があるスレッドのみをアクティビティ一覧に表示します。", "clearCompleted": "完了済みをクリア", "none": "なし", "search": "検索", @@ -14207,7 +14205,9 @@ "idle": "アイドル", "unverifiable": "最近の更新なし", "permission": "要対応" - } + }, + "filtersSection": "フィルター", + "viewSection": "表示" }, "ActivityScopeFilterControls": { "resetScope": "すべてのホストとプロジェクトを表示" diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 76d778c1550..42983088062 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -14266,8 +14266,6 @@ "5651b216c6": "알 수 없는 프로젝트", "22b22034bc": "활동에서는 독립형 terminal을 사용할 수 없습니다.", "afdc2139a8": "Agent terminal이 닫혔습니다. 계속하려면 이 워크스페이스에서 새 terminal을 여세요.", - "compactModeDescription": "한 줄 제목과 두 줄 상태 메시지로 더 짧은 스레드 행을 표시합니다.", - "unreadOnlyDescription": "읽지 않은 업데이트가 있는 스레드만 활동 목록에 표시합니다.", "clearCompleted": "완료된 항목 지우기", "none": "없음", "search": "검색", @@ -14285,7 +14283,9 @@ "idle": "유휴", "unverifiable": "최근 업데이트 없음", "permission": "주의 필요" - } + }, + "filtersSection": "필터", + "viewSection": "보기" }, "ActivityScopeFilterControls": { "resetScope": "모든 호스트 및 프로젝트 표시" diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index a267853a78c..09ca1689ea3 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -14266,8 +14266,6 @@ "5651b216c6": "未知项目", "22b22034bc": "独立终端在活动中不可用。", "afdc2139a8": "智能体终端关闭。在此工作区中打开一个新终端以继续。", - "compactModeDescription": "以单行标题和两行状态消息显示更短的线程行。", - "unreadOnlyDescription": "将活动列表筛选为仅显示有未读更新的线程。", "clearCompleted": "清除已完成", "none": "无", "search": "搜索", @@ -14285,8 +14283,11 @@ "idle": "空闲", "unverifiable": "暂无近期更新", "permission": "需注意" - } + }, + "filtersSection": "筛选", + "viewSection": "视图" }, + "ActivityScopeFilterControls": { "resetScope": "显示所有主机和项目" }, From e829bb523a77bbc2f357c8d1237e5a8750fc3d49 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 18:31:49 +0000 Subject: [PATCH 3/4] Update README downloads badge --- docs/assets/readme-downloads.svg | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/assets/readme-downloads.svg b/docs/assets/readme-downloads.svg index 75762752848..724be685ea7 100644 --- a/docs/assets/readme-downloads.svg +++ b/docs/assets/readme-downloads.svg @@ -1,5 +1,5 @@ - - downloads: 43m + + downloads: 44m @@ -15,7 +15,7 @@ downloads downloads - 43m - 43m + 44m + 44m From 2ba2c90cb602d54240b307c02986e5ae53cfc4ae Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:32:01 -0400 Subject: [PATCH 4/4] fix(orchestration): own a worker terminal from creation, not after the boot wait (#19608) * fix(orchestration): own a worker terminal from creation, not after the boot wait A worker pane is visible on desktop and phone the moment it is created, but the worker_terminal_resources row saying orchestration owns it was written only after the agent TUI went idle (up to 60s). A keystroke into the booting pane found no owned row, markWorkerTerminalUserOwned returned 0, and the takeover was dropped - so a later worker-release closed the pane under the user. Record custody on the branches that create a terminal, right after creation and before the tui-idle wait. The Dispatch capability still waits for the agent to come up. An explicit --terminal reuse is untouched: it transfers at authority. With the row present from creation, the failed-start adoption is dead. What a failed start still needs is the Dispatch-context pane identity release re-proves through, which is now copied from the custody row. * chore(i18n): drop the orphan minimumContrast entries #19544 re-added to the runtime catalog --- .../worker-dispatch-authority.ts | 56 ++++- .../worker-dispatch-outcome.ts | 17 +- .../failed-start-dispatch-identity.ts | 34 +++ .../failed-start-terminal-adoption.ts | 68 ------ .../failed-start-terminal-adoption.test.ts | 157 ------------- .../worker/created-worker-terminal-custody.ts | 32 +++ .../failed-start-residual-terminal.test.ts | 191 ---------------- .../worker/failed-start-residual-terminal.ts | 53 ----- .../worker/failed-worker-start-teardown.ts | 23 +- .../worker/local-worker-start.ts | 12 +- .../worker/worker-start-receipt.ts | 20 +- ...orker-terminal-custody-at-creation.test.ts | 216 ++++++++++++++++++ .../src/i18n/en-runtime-required.json | 11 +- 13 files changed, 360 insertions(+), 530 deletions(-) create mode 100644 src/main/runtime/orchestration/db/worker-terminal/failed-start-dispatch-identity.ts delete mode 100644 src/main/runtime/orchestration/db/worker-terminal/failed-start-terminal-adoption.ts delete mode 100644 src/main/runtime/orchestration/failed-start-terminal-adoption.test.ts create mode 100644 src/main/runtime/rpc/methods/orchestration/worker/created-worker-terminal-custody.ts delete mode 100644 src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.test.ts delete mode 100644 src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.ts create mode 100644 src/main/runtime/rpc/methods/orchestration/worker/worker-terminal-custody-at-creation.test.ts diff --git a/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts b/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts index b89468c77a0..5e0f5f5b142 100644 --- a/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts +++ b/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts @@ -160,12 +160,66 @@ export function prepareStartingWorkerAuthority( } } +/** + * Custody for an agent terminal this worker-start just created, recorded at creation instead of + * after the agent boot wait. Until the row exists a keystroke into the booting pane finds no + * ownership to flip, so the takeover is silently dropped and a later `worker-release` closes the + * pane under the user. + * + * Ownership of a pane only; the Dispatch capability stays behind the boot wait, because authority + * must not be handed to a process that has not come up. + */ +export function recordCreatedWorkerTerminalCustody( + this: OrchestrationDb, + params: { + dispatchId: string + handle: string + paneKey: string + processIncarnation: string + worktreeId: string + hostScope?: string | null + } +): void { + this.db.exec('BEGIN IMMEDIATE') + try { + // Same guard as prepareStartingWorkerAuthority, read inside the transaction: a dispatch stopped + // while the terminal was being created must not acquire an owner. + const dispatch = this.getDispatchContextById(params.dispatchId) + const worker = this.getWorkerDispatch(params.dispatchId) + if (!dispatch || dispatch.status !== 'pending' || worker?.state !== 'starting') { + throw new OrchestrationError( + 'dispatch_inactive', + `Dispatch ${params.dispatchId} is not starting.` + ) + } + if (!this.getWorkerTerminalResourceByOwner(params.dispatchId)) { + this.createWorkerTerminalResourceStatement({ + dispatchId: params.dispatchId, + worktreeId: params.worktreeId, + terminalHandle: params.handle, + paneKey: params.paneKey, + processIncarnation: params.processIncarnation, + endpointId: worker.runtime_epoch, + endpointIncarnation: params.processIncarnation, + hostScope: params.hostScope, + ownership: 'owned' + }) + } + this.db.exec('COMMIT') + } catch (error) { + this.db.exec('ROLLBACK') + throw error + } +} + export type WorkerDispatchAuthorityMethods = { prepareStartingWorkerAuthority: typeof prepareStartingWorkerAuthority + recordCreatedWorkerTerminalCustody: typeof recordCreatedWorkerTerminalCustody } export function attachWorkerDispatchAuthority(ctor: { prototype: object }): void { Object.assign(ctor.prototype, { - prepareStartingWorkerAuthority + prepareStartingWorkerAuthority, + recordCreatedWorkerTerminalCustody }) } diff --git a/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.ts b/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.ts index 5ff97f83fd9..6544f519497 100644 --- a/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.ts +++ b/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.ts @@ -2,10 +2,7 @@ import type { WorkerDispatchRow } from '../../types' import { OrchestrationError } from '../../orchestration-error' import type { OrchestrationDb } from '../orchestration-db' import { transitionLifecycleWithDb } from '../lifecycle-transition' -import { - adoptFailedStartTerminal, - type FailedStartTerminalAdoption -} from '../worker-terminal/failed-start-terminal-adoption' +import { recordFailedStartDispatchIdentity } from '../worker-terminal/failed-start-dispatch-identity' export function markWorkerDispatchReady( this: OrchestrationDb, @@ -51,11 +48,7 @@ export function failWorkerStart( // Why (#16095): revocation exists to stop a worker acting on a dispatch that never landed. A // prompt whose turn start went unobserved provably landed, so its worker keeps the authority its // own report needs. - options: { - retainCapability?: boolean - /** A start that died before authority attached still owns the terminal it created. */ - adoptResidualTerminal?: FailedStartTerminalAdoption - } = {} + options: { retainCapability?: boolean } = {} ): WorkerDispatchRow { this.db.exec('BEGIN IMMEDIATE') try { @@ -104,11 +97,7 @@ export function failWorkerStart( }) } this.closeQuestionsForDispatch(dispatchId) - adoptFailedStartTerminal( - this, - this.getWorkerDispatch(dispatchId) as WorkerDispatchRow, - options.adoptResidualTerminal - ) + recordFailedStartDispatchIdentity(this, this.getWorkerDispatch(dispatchId) as WorkerDispatchRow) this.db.exec('COMMIT') return this.getWorkerDispatch(dispatchId) as WorkerDispatchRow } catch (error) { diff --git a/src/main/runtime/orchestration/db/worker-terminal/failed-start-dispatch-identity.ts b/src/main/runtime/orchestration/db/worker-terminal/failed-start-dispatch-identity.ts new file mode 100644 index 00000000000..671b12c39a7 --- /dev/null +++ b/src/main/runtime/orchestration/db/worker-terminal/failed-start-dispatch-identity.ts @@ -0,0 +1,34 @@ +import type { WorkerDispatchRow } from '../../types' +import type { OrchestrationDb } from '../orchestration-db' + +/** + * A start that dies before `prepareStartingWorkerAuthority` never filled the Dispatch context in, + * and release re-proves identity through it — so the custody row written at terminal creation would + * name a pane no release path could match. Copy that identity across. + * + * `capability_hash` stays null, so this grants nothing: it records which pane the Dispatch owns. + * + * No transaction: composes inside `failWorkerStart`'s. + */ +export function recordFailedStartDispatchIdentity( + db: OrchestrationDb, + worker: WorkerDispatchRow +): void { + const resource = db.getWorkerTerminalResourceByOwner(worker.dispatch_id) + if (!resource || resource.terminal_handle !== worker.agent_terminal_handle) { + return + } + db.db + .prepare( + `UPDATE dispatch_contexts + SET assignee_handle = ?, assignee_pane_key = ?, process_incarnation = ?, host_scope = ? + WHERE id = ? AND status = 'failed' AND capability_hash IS NULL` + ) + .run( + resource.terminal_handle, + resource.pane_key, + resource.process_incarnation, + resource.host_scope, + worker.dispatch_id + ) +} diff --git a/src/main/runtime/orchestration/db/worker-terminal/failed-start-terminal-adoption.ts b/src/main/runtime/orchestration/db/worker-terminal/failed-start-terminal-adoption.ts deleted file mode 100644 index 607ae9f45a7..00000000000 --- a/src/main/runtime/orchestration/db/worker-terminal/failed-start-terminal-adoption.ts +++ /dev/null @@ -1,68 +0,0 @@ -import type { WorkerDispatchRow } from '../../types' -import type { OrchestrationDb } from '../orchestration-db' - -/** Identity of a terminal this worker-start created and never handed to an owner. */ -export type FailedStartTerminalAdoption = { - terminalHandle: string - worktreeId: string | null - paneKey: string - processIncarnation: string - hostScope?: string | null -} - -/** - * A start that dies before `prepareStartingWorkerAuthority` leaves the terminal it created with no - * owner, so no release path can ever close it and the fleet can only say `inspect`. Record the - * ownership the successful path would have recorded, so ordinary `worker-release` owns the cleanup. - * - * No transaction: composes inside `failWorkerStart`'s. - */ -export function adoptFailedStartTerminal( - db: OrchestrationDb, - worker: WorkerDispatchRow, - adoption: FailedStartTerminalAdoption | undefined -): void { - if (!adoption || worker.agent_terminal_handle !== adoption.terminalHandle) { - return - } - if (db.getWorkerTerminalResourceByOwner(worker.dispatch_id)) { - return - } - // A second owner for one process could close it twice, or close a terminal already handed on. - const conflict = db.db - .prepare( - `SELECT 1 FROM worker_terminal_resources - WHERE ownership_state <> 'released' - AND (terminal_handle = ? OR process_incarnation = ?) LIMIT 1` - ) - .get(adoption.terminalHandle, adoption.processIncarnation) - if (conflict) { - return - } - db.createWorkerTerminalResourceStatement({ - dispatchId: worker.dispatch_id, - worktreeId: adoption.worktreeId ?? worker.worktree_id, - terminalHandle: adoption.terminalHandle, - paneKey: adoption.paneKey, - processIncarnation: adoption.processIncarnation, - endpointId: worker.runtime_epoch ?? null, - endpointIncarnation: adoption.processIncarnation, - hostScope: adoption.hostScope ?? null, - ownership: 'owned' - }) - // Release re-proves identity through the Dispatch context, which a failed start never filled in. - // This records which pane the Dispatch owns; `capability_hash` stays null, so it grants nothing. - db.db - .prepare( - `UPDATE dispatch_contexts - SET assignee_handle = ?, assignee_pane_key = ?, process_incarnation = ?, host_scope = ? - WHERE id = ? AND status = 'failed' AND capability_hash IS NULL` - ) - .run( - adoption.terminalHandle, - adoption.paneKey, - adoption.processIncarnation, - adoption.hostScope ?? null, - worker.dispatch_id - ) -} diff --git a/src/main/runtime/orchestration/failed-start-terminal-adoption.test.ts b/src/main/runtime/orchestration/failed-start-terminal-adoption.test.ts deleted file mode 100644 index b980a7f2a25..00000000000 --- a/src/main/runtime/orchestration/failed-start-terminal-adoption.test.ts +++ /dev/null @@ -1,157 +0,0 @@ -import { afterEach, describe, expect, it } from 'vitest' -import { OrchestrationDb } from './db' - -const HANDLE = 'term_residual' -const PANE_KEY = 'tab_residual:leaf_residual' -const INCARNATION = 'runtime:pty-residual:1' - -describe('a start that fails before authority still owns the terminal it created', () => { - let db: OrchestrationDb | undefined - - afterEach(() => { - db?.close() - }) - - /** Replays the shipping order: readiness stage records the handle, then the wait fails. */ - function failStartAfterCreatingTerminal( - adoption?: Parameters[3] - ): { db: OrchestrationDb; dispatchId: string } { - const d = (db = new OrchestrationDb(':memory:')) - const task = d.createTask({ runId: 'run_legacy_local', spec: 'residual terminal' }) - const started = d.createStartingWorkerDispatch({ - creator: { kind: 'system' }, - maxDepth: Number.MAX_SAFE_INTEGER, - taskId: task.id, - startOptions: {} - }) - const effects = [ - { kind: 'terminal', role: 'agent', action: 'created', id: HANDLE, surface: 'visible' } - ] - d.recordWorkerStage({ - dispatchId: started.dispatch.id, - stage: 'terminal_readying', - worktreeId: 'repo::worktree', - terminalHandle: HANDLE, - effects, - residualResources: effects - }) - d.failWorkerStart( - started.dispatch.id, - 'agent_readiness', - 'Agent startup blocked: codex-interactive-prompt', - adoption - ) - return { db: d, dispatchId: started.dispatch.id } - } - - const adoption = { - adoptResidualTerminal: { - terminalHandle: HANDLE, - worktreeId: 'repo::worktree', - paneKey: PANE_KEY, - processIncarnation: INCARNATION, - hostScope: null - } - } - - it('leaves nothing that can close the terminal when the start is not adopted', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal() - - expect(d.getWorkerTerminalResourceByOwner(dispatchId)).toBeUndefined() - expect(d.requestWorkerTerminalRelease(dispatchId)).toMatchObject({ - disposition: 'retained', - reason: 'no_owned_resource' - }) - }) - - it('records the ownership the successful path would have recorded', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal(adoption) - - expect(d.getWorkerTerminalResourceByOwner(dispatchId)).toMatchObject({ - owner_dispatch_id: dispatchId, - terminal_handle: HANDLE, - pane_key: PANE_KEY, - process_incarnation: INCARNATION, - ownership_state: 'owned', - release_state: 'not_requested' - }) - }) - - it('lets worker-release proceed on the failed dispatch', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal(adoption) - - expect(d.requestWorkerTerminalRelease(dispatchId)).toMatchObject({ - disposition: 'requested', - resource: { release_state: 'requested' } - }) - }) - - it('re-proves identity through the dispatch context release reads', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal(adoption) - - expect( - d.isDispatchProcessCurrent({ dispatchId, paneKey: PANE_KEY, processIncarnation: INCARNATION }) - ).toBe(true) - // Adoption records which pane the dispatch owns; it never restores authority over it. - expect(d.getDispatchContextById(dispatchId)).toMatchObject({ - status: 'failed', - capability_hash: null - }) - expect(d.getDispatchContextById(dispatchId)?.capability_revoked_at).not.toBeNull() - }) - - it('publishes the terminal as reclaimable so the fleet names release', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal(adoption) - - expect(d.listWorkerTerminalResources({ dispatchIds: [dispatchId] })[0]).toMatchObject({ - agentTerminalHandle: HANDLE, - terminalState: 'reclaimable' - }) - }) - - it('never claims a terminal the durable row does not name', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal({ - adoptResidualTerminal: { ...adoption.adoptResidualTerminal, terminalHandle: 'term_other' } - }) - - expect(d.getWorkerTerminalResourceByOwner(dispatchId)).toBeUndefined() - }) - - it('never claims a terminal another live resource already accounts for', () => { - const d = (db = new OrchestrationDb(':memory:')) - const first = d.createStartingWorkerDispatch({ - creator: { kind: 'system' }, - maxDepth: Number.MAX_SAFE_INTEGER, - taskId: d.createTask({ runId: 'run_legacy_local', spec: 'owner' }).id, - startOptions: {} - }) - d.prepareStartingWorkerAuthority({ - dispatchId: first.dispatch.id, - handle: HANDLE, - paneKey: PANE_KEY, - processIncarnation: INCARNATION, - worktreeId: 'repo::worktree', - setupState: 'not_applicable', - effects: [], - terminalOwnership: 'created' - }) - const second = d.createStartingWorkerDispatch({ - creator: { kind: 'system' }, - maxDepth: Number.MAX_SAFE_INTEGER, - taskId: d.createTask({ runId: 'run_legacy_local', spec: 'claimant' }).id, - startOptions: {} - }) - d.recordWorkerStage({ - dispatchId: second.dispatch.id, - stage: 'terminal_readying', - terminalHandle: HANDLE - }) - - d.failWorkerStart(second.dispatch.id, 'agent_readiness', 'blocked', adoption) - - expect(d.getWorkerTerminalResourceByOwner(second.dispatch.id)).toBeUndefined() - expect(d.getWorkerTerminalResourceByOwner(first.dispatch.id)).toMatchObject({ - ownership_state: 'owned' - }) - }) -}) diff --git a/src/main/runtime/rpc/methods/orchestration/worker/created-worker-terminal-custody.ts b/src/main/runtime/rpc/methods/orchestration/worker/created-worker-terminal-custody.ts new file mode 100644 index 00000000000..c73884b1d96 --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration/worker/created-worker-terminal-custody.ts @@ -0,0 +1,32 @@ +import type { OrcaRuntimeService } from '../../../../orca-runtime' +import type { OrchestrationDb } from '../../../../orchestration/db' +import { requireWorkerAuthority } from './worker-topology' + +/** + * Custody for an agent terminal this start created, recorded when the terminal exists rather than + * after the agent boot wait: a keystroke into the booting pane has to find an `owned` row to flip, + * or the takeover is dropped and a later `worker-release` closes the pane under the user. + * + * Ownership of a pane only. The Dispatch capability still waits for the agent to come up. + * + * `created` is false for an explicit `--terminal` reuse, which is the caller's own pane, and for a + * structured session, which reaches its authority in this same turn and so has no gap to close. + */ +export function recordCreatedWorkerTerminalCustody( + runtime: OrcaRuntimeService, + stage: { db: OrchestrationDb; dispatchId: string; worktreeId: string; terminalHandle: string }, + created: boolean +): void { + if (!created) { + return + } + const authority = requireWorkerAuthority(runtime, stage.terminalHandle) + stage.db.recordCreatedWorkerTerminalCustody({ + dispatchId: stage.dispatchId, + handle: stage.terminalHandle, + paneKey: authority.paneKey, + processIncarnation: authority.processIncarnation, + worktreeId: stage.worktreeId, + hostScope: authority.hostScope ?? null + }) +} diff --git a/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.test.ts b/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.test.ts deleted file mode 100644 index 413a397462b..00000000000 --- a/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.test.ts +++ /dev/null @@ -1,191 +0,0 @@ -import { afterEach, describe, expect, it } from 'vitest' -import type { OrcaRuntimeService } from '../../../../orca-runtime' -import { OrchestrationDb } from '../../../../orchestration/db' -import { resolveResidualAgentTerminal } from './failed-start-residual-terminal' -import { failWorkerStartWithReceipt } from './worker-start-receipt' -import type { WorkerEffect } from './worker-topology' - -const HANDLE = 'term_residual' -const PANE_KEY = 'tab_residual:leaf_residual' -const INCARNATION = 'pty-residual:1' - -const createdAgentTerminal: WorkerEffect = { - kind: 'terminal', - role: 'agent', - action: 'created', - id: HANDLE, - surface: 'visible' -} - -function createRuntime(overrides: Partial> = {}): OrcaRuntimeService { - return { - getOrchestrationDispatchAuthority: () => ({ - paneKey: PANE_KEY, - processIncarnation: INCARNATION, - hostScope: { kind: 'local', hostId: 'local' } - }), - getTerminalPaneKey: () => PANE_KEY, - getTerminalProcessIncarnation: () => INCARNATION, - ...overrides - } as unknown as OrcaRuntimeService -} - -describe('residual agent terminal left by a failed start', () => { - it('resolves identity for a terminal this start created', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime(), - effects: [createdAgentTerminal], - terminalHandle: HANDLE, - worktreeId: 'repo::worktree' - }) - ).toEqual({ - terminalHandle: HANDLE, - worktreeId: 'repo::worktree', - paneKey: PANE_KEY, - processIncarnation: INCARNATION, - hostScope: JSON.stringify({ kind: 'local', hostId: 'local' }) - }) - }) - - it('resolves the agent-first worktree terminal the same way', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime(), - effects: [{ ...createdAgentTerminal, action: 'reused_agent_terminal' }], - terminalHandle: HANDLE, - worktreeId: null - }) - ).toMatchObject({ terminalHandle: HANDLE }) - }) - - it('never claims a caller-supplied terminal', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime(), - effects: [{ ...createdAgentTerminal, action: 'reused' }], - terminalHandle: HANDLE, - worktreeId: null - }) - ).toBeUndefined() - }) - - it('never claims a setup terminal', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime(), - effects: [{ ...createdAgentTerminal, role: 'setup' }], - terminalHandle: HANDLE, - worktreeId: null - }) - ).toBeUndefined() - }) - - it('refuses a pane whose process cannot be identified', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime({ - getOrchestrationDispatchAuthority: () => null, - getTerminalProcessIncarnation: () => null - }), - effects: [createdAgentTerminal], - terminalHandle: HANDLE, - worktreeId: null - }) - ).toBeUndefined() - }) - - it('refuses when the start never resolved a terminal', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime(), - effects: [], - terminalHandle: undefined, - worktreeId: null - }) - ).toBeUndefined() - }) - - it('stays silent when identity resolution throws', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime({ - getOrchestrationDispatchAuthority: () => { - throw new Error('handle retired') - } - }), - effects: [createdAgentTerminal], - terminalHandle: HANDLE, - worktreeId: null - }) - ).toBeUndefined() - }) -}) - -describe('failed worker-start receipt for a residual terminal', () => { - let db: OrchestrationDb | undefined - - afterEach(() => { - db?.close() - }) - - function failStart(residual: boolean): { recovery?: string } { - const d = (db = new OrchestrationDb(':memory:')) - const task = d.createTask({ runId: 'run_legacy_local', spec: 'residual receipt' }) - const started = d.createStartingWorkerDispatch({ - creator: { kind: 'system' }, - maxDepth: Number.MAX_SAFE_INTEGER, - taskId: task.id, - startOptions: {} - }) - d.recordWorkerStage({ - dispatchId: started.dispatch.id, - stage: 'terminal_readying', - terminalHandle: HANDLE, - effects: [createdAgentTerminal], - residualResources: [createdAgentTerminal] - }) - return failWorkerStartWithReceipt({ - db: d, - mode: { - mode: 'terminal', - preferred: 'terminal', - reason: 'user_default', - detail: 'terminal by default' - } as const, - runId: 'run_residual', - taskId: task.id, - dispatchId: started.dispatch.id, - failedStage: 'agent_readiness', - error: new Error('Agent startup blocked: codex-interactive-prompt'), - setup: { - requested: 'not_applicable', - effective: 'not_applicable', - source: 'existing_worktree', - hookFound: false, - startupPolicy: 'start-immediately', - state: 'not_applicable' - }, - launch: { requested: { agent: 'codex' }, effective: { agent: 'codex' } } as never, - ...(residual - ? { - residualAgentTerminal: { - terminalHandle: HANDLE, - worktreeId: 'repo::worktree', - paneKey: PANE_KEY, - processIncarnation: INCARNATION, - hostScope: null - } - } - : {}) - }) as { recovery?: string } - } - - it('names worker-release for the terminal it left behind', () => { - expect(failStart(true).recovery).toContain('worker-release') - }) - - it('promises no cleanup when there is no residual terminal', () => { - expect(failStart(false).recovery).toBeUndefined() - }) -}) diff --git a/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.ts b/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.ts deleted file mode 100644 index e42923e93a9..00000000000 --- a/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.ts +++ /dev/null @@ -1,53 +0,0 @@ -import type { OrcaRuntimeService } from '../../../../orca-runtime' -import type { FailedStartTerminalAdoption } from '../../../../orchestration/db/worker-terminal/failed-start-terminal-adoption' -import type { WorkerEffect } from './worker-topology' - -/** True only for an agent terminal this worker-start brought into existence. An explicit - * `--terminal` reuse records `reused` and is never residual — it is the caller's terminal. */ -function orchestrationCreatedAgentTerminal( - effects: readonly WorkerEffect[], - handle: string -): boolean { - return effects.some( - (effect) => - effect.kind === 'terminal' && - effect.role === 'agent' && - effect.id === handle && - (effect.action?.startsWith('created') === true || effect.action === 'reused_agent_terminal') - ) -} - -/** - * Identity for the terminal a failed start leaves behind, so the failed Dispatch can own it and - * `worker-release` can close it. Returns nothing unless the pane and process are both provable: - * an unprovable identity must never authorize a later close. - */ -export function resolveResidualAgentTerminal(args: { - runtime: OrcaRuntimeService - effects: readonly WorkerEffect[] - terminalHandle: string | undefined - worktreeId: string | null -}): FailedStartTerminalAdoption | undefined { - const handle = args.terminalHandle - if (!handle || !orchestrationCreatedAgentTerminal(args.effects, handle)) { - return undefined - } - try { - const authority = args.runtime.getOrchestrationDispatchAuthority(handle) - const paneKey = authority?.paneKey ?? args.runtime.getTerminalPaneKey(handle) - const processIncarnation = - authority?.processIncarnation ?? args.runtime.getTerminalProcessIncarnation(handle) - if (!paneKey || !processIncarnation) { - return undefined - } - return { - terminalHandle: handle, - worktreeId: args.worktreeId, - paneKey, - processIncarnation, - hostScope: authority?.hostScope ? JSON.stringify(authority.hostScope) : null - } - } catch { - return undefined - } -} diff --git a/src/main/runtime/rpc/methods/orchestration/worker/failed-worker-start-teardown.ts b/src/main/runtime/rpc/methods/orchestration/worker/failed-worker-start-teardown.ts index 32827377b54..250b639fc60 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/failed-worker-start-teardown.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/failed-worker-start-teardown.ts @@ -3,40 +3,27 @@ import { discardStructuredWorkerSession, releaseStructuredWorkerSession } from '../../orchestration-structured-worker-session' -import { resolveResidualAgentTerminal } from './failed-start-residual-terminal' import type { createStructuredWorkerSessionForWorktree } from './worker-topology' -import type { FailedStartTerminalAdoption } from '../../../../orchestration/db/worker-terminal/failed-start-terminal-adoption' /** - * Undoes what a start created before it failed, and reports what `worker-release` still owns. + * Undoes what a start created before it failed. * * A start that never reached ready leaves no settlement to release the hold later, and its session * was already published as a chat tab — without the discard, a failed start strands a dead chat tab * that the durable restore index republishes on every app launch. Both halves are best-effort by * construction, so neither can replace the real error. + * + * A created PTY terminal is deliberately NOT torn down: its custody row was written at creation, so + * `worker-release` on the failed Dispatch owns that cleanup and the coordinator decides when. */ export async function tearDownFailedWorkerStart(args: { runtime: OrcaRuntimeService structuredSession: Awaited> | null dispatchId: string - effects: unknown[] - terminalHandle: string | undefined - worktreeId: string | null -}): Promise { +}): Promise { const { runtime, structuredSession } = args - // A structured session is torn down outright here, so it must never also be adopted as a residual - // terminal for `worker-release` to close a second time. - const residualAgentTerminal = structuredSession - ? undefined - : resolveResidualAgentTerminal({ - runtime, - effects: args.effects as never, - terminalHandle: args.terminalHandle, - worktreeId: args.worktreeId - }) releaseStructuredWorkerSession(args.dispatchId, runtime) if (structuredSession) { await discardStructuredWorkerSession(structuredSession.identity.sessionId, runtime) } - return residualAgentTerminal } diff --git a/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts b/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts index 48b14f9a84e..d67d09b766a 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts @@ -19,6 +19,7 @@ import { failWorkerStartWithReceipt } from './worker-start-receipt' import { parseTaskDeps } from './task-deps-argument' import { assertExplicitWorkerTerminalUsable } from './explicit-worker-terminal-validation' import { deliverWorkerDispatchPreamble } from './deliver-worker-dispatch-preamble' +import { recordCreatedWorkerTerminalCustody } from './created-worker-terminal-custody' import { tearDownFailedWorkerStart } from './failed-worker-start-teardown' import { createExistingWorktreeWorkerTerminal, @@ -203,6 +204,7 @@ export async function startLocalWorker(args: { setup: setupReceipt, effects } + recordCreatedWorkerTerminalCustody(runtime, setupStage, !params.terminal && !structuredSession) if (persistGatedSetupSpawnFailure(setupStage)) { failedStage = 'setup_start' throw new Error('Setup terminal failed to start before the gated agent launch.') @@ -285,13 +287,10 @@ export async function startLocalWorker(args: { ...(terminalRevealWarning ? { warning: terminalRevealWarning } : {}) } } catch (error) { - const residualAgentTerminal = await tearDownFailedWorkerStart({ + await tearDownFailedWorkerStart({ runtime, structuredSession, - dispatchId: started.dispatch.id, - effects, - terminalHandle, - worktreeId: resolvedWorktree?.id ?? null + dispatchId: started.dispatch.id }) return failWorkerStartWithReceipt({ db, @@ -302,8 +301,7 @@ export async function startLocalWorker(args: { error, setup: setupReceipt, launch: launch.receipt, - mode, - ...(residualAgentTerminal ? { residualAgentTerminal } : {}) + mode }) } } diff --git a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-receipt.ts b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-receipt.ts index 9fd98dd9db3..f2dee00b44c 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-receipt.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-receipt.ts @@ -4,8 +4,8 @@ import { isUnknownWorkerStartOutcome, type WorkerSetupReceipt } from './worker-t import type { OrchestrationWorkerLaunchReceipt } from './worker-launch-preferences' import type { WorkerStartModeReceipt } from '../../orchestration-worker-start-mode' import { isAgentSessionPtyWriteRefusedError } from '../../../../../../shared/agent-session-pty-write-admission' -import type { FailedStartTerminalAdoption } from '../../../../orchestration/db/worker-terminal/failed-start-terminal-adoption' import { structuredChatPtyWriteRefusalCopy } from '../../../../../../shared/agent-session-pty-write-refusal-copy' +import { isStructuredWorkerHandle } from '../../../../structured-worker-identity' export function failWorkerStartWithReceipt(args: { db: OrchestrationDb @@ -17,8 +17,6 @@ export function failWorkerStartWithReceipt(args: { setup: WorkerSetupReceipt launch: OrchestrationWorkerLaunchReceipt mode: WorkerStartModeReceipt - /** The terminal this start created and never handed to an owner. */ - residualAgentTerminal?: FailedStartTerminalAdoption }): unknown { const agentSessionRefusal = isAgentSessionPtyWriteRefusedError(args.error) ? args.error.refusal @@ -33,14 +31,14 @@ export function failWorkerStartWithReceipt(args: { : args.db.failWorkerStart(args.dispatchId, args.failedStage, reason, { // Why (#16095): the preamble is written before submission is verified, so a stalled // verdict never means the worker lacks its task — keep the authority its report needs. - retainCapability: isAgentPromptStalledError(args.error), - ...(args.residualAgentTerminal ? { adoptResidualTerminal: args.residualAgentTerminal } : {}) + retainCapability: isAgentPromptStalledError(args.error) }) - // Only claim cleanup the ownership table actually accepted; the adoption declines a terminal - // another resource already accounts for. - const adopted = - Boolean(args.residualAgentTerminal) && - Boolean(args.db.getWorkerTerminalResourceByOwner(args.dispatchId)) + // Only name cleanup this start actually left behind: a terminal it created and still owns. A + // structured session is discarded by the teardown, a pane the user typed into is theirs, and an + // unknown outcome is not settled — none of the three has anything for `worker-release` to close. + const residual = unknown ? undefined : args.db.getWorkerTerminalResourceByOwner(args.dispatchId) + const releasable = + residual?.ownership_state === 'owned' && !isStructuredWorkerHandle(residual.terminal_handle) return { runId: args.runId, taskId: args.taskId, @@ -55,7 +53,7 @@ export function failWorkerStartWithReceipt(args: { effects: JSON.parse(worker.effects) as unknown[], residualResources: JSON.parse(worker.residual_resources) as unknown[], ...(agentSessionRefusal ? { agentSessionRefusal } : {}), - ...(adopted + ...(releasable ? { recovery: `This start created a terminal that never ran the Task. Close it with: orca orchestration worker-release --dispatch ${args.dispatchId}` } diff --git a/src/main/runtime/rpc/methods/orchestration/worker/worker-terminal-custody-at-creation.test.ts b/src/main/runtime/rpc/methods/orchestration/worker/worker-terminal-custody-at-creation.test.ts new file mode 100644 index 00000000000..201201e5877 --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration/worker/worker-terminal-custody-at-creation.test.ts @@ -0,0 +1,216 @@ +/** + * Custody for an agent terminal this start created is written when the terminal is created, not + * after the agent boot wait. + * + * A worker pane is visible on desktop and phone the moment it exists. While the row was written + * only after `tui-idle` (up to 60 s later), a keystroke into the booting pane found no `owned` row, + * `markWorkerTerminalUserOwned` returned 0, and the takeover was lost — so a later `worker-release` + * closed the pane the user had claimed. + */ + +import { afterEach, describe, expect, it, vi } from 'vitest' +import { OrchestrationDb } from '../../../../orchestration/db' +import { createOrchestrationWorkerReleaseHarness } from './worker-release.test-support' + +const READY_WAIT = { + handle: 'term_worker', + condition: 'tui-idle', + satisfied: true, + status: 'running', + exitCode: null +} + +describe('worker terminal custody is recorded at terminal creation', () => { + const h = createOrchestrationWorkerReleaseHarness() + + afterEach(() => h.cleanup()) + + /** Holds the agent boot wait open so the mid-start database state can be read. */ + function holdBootWait(): { finish: (satisfied?: boolean) => void } { + const gate = h.deferred() + vi.spyOn(h.runtime, 'waitForTerminal').mockReturnValue(gate.promise as never) + return { + finish: (satisfied = true) => + gate.resolve({ ...READY_WAIT, satisfied, status: satisfied ? 'running' : 'exited' }) + } + } + + function startingDispatchId(): string { + return ( + h.db.db + .prepare("SELECT dispatch_id FROM worker_dispatches WHERE state = 'starting'") + .get() as { dispatch_id: string } + ).dispatch_id + } + + async function startHeldAtBootWait(options: { terminal?: string } = {}): Promise<{ + dispatchId: string + taskId: string + start: Promise + finish: (satisfied?: boolean) => void + }> { + const task = h.db.createTask({ spec: 'custody at creation', runId: h.activeRunId }) + const { finish } = holdBootWait() + const start = h.call('orchestration.workerStart', { + task: task.id, + from: 'term_coord', + ...(options.terminal ? { terminal: options.terminal } : { agent: 'codex' }) + }) + await vi.waitFor(() => expect(h.runtime.waitForTerminal).toHaveBeenCalled()) + return { dispatchId: startingDispatchId(), taskId: task.id, start, finish } + } + + it('owns the created terminal before the boot wait resolves', async () => { + h.setup() + const held = await startHeldAtBootWait() + + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toMatchObject({ + ownership_state: 'owned', + release_state: 'not_requested', + terminal_handle: 'term_worker', + pane_key: h.workerPaneKey, + process_incarnation: 'runtime_test:term_worker:1', + host_scope: JSON.stringify({ kind: 'local', hostId: 'local' }) + }) + // worker-list reads the same row: a booting worker now says `active`, not `retained`. + expect(h.db.listWorkerTerminalResources({ dispatchIds: [held.dispatchId] })[0]).toMatchObject({ + agentTerminalHandle: 'term_worker', + terminalState: 'active' + }) + + held.finish() + await expect(held.start).resolves.toMatchObject({ state: 'ready' }) + }) + + it('claims nothing for an explicitly reused terminal until authority transfers it', async () => { + h.setup() + const held = await startHeldAtBootWait({ terminal: 'term_worker' }) + + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toBeUndefined() + + held.finish() + await expect(held.start).resolves.toMatchObject({ state: 'ready' }) + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toMatchObject({ + ownership_state: 'external', + retained_reason: 'external_terminal' + }) + }) + + it('lets a keystroke during the boot wait take the pane, and release then retains it', async () => { + h.setup() + const held = await startHeldAtBootWait() + + await expect( + h.call('orchestration.workerTerminalUserInput', { paneKey: h.workerPaneKey }) + ).resolves.toEqual({ changed: 1 }) + + held.finish() + await expect(held.start).resolves.toMatchObject({ state: 'ready' }) + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toMatchObject({ + ownership_state: 'user_owned', + retained_reason: 'user_takeover' + }) + + h.settle(held.taskId, held.dispatchId, 'succeeded') + await expect( + h.call('orchestration.workerRelease', { dispatch: held.dispatchId }) + ).resolves.toMatchObject({ state: 'retained', reason: 'user_takeover', processAction: 'none' }) + expect(h.runtime.closeTerminal).not.toHaveBeenCalled() + }) + + it('still refuses to release a starting worker that already owns its terminal', async () => { + h.setup() + const held = await startHeldAtBootWait() + + await expect( + h.call('orchestration.workerRelease', { dispatch: held.dispatchId }) + ).rejects.toThrow(/only a settled worker can release/) + + held.finish() + await held.start + }) + + it('leaves a start that died on the boot wait a terminal worker-release can close', async () => { + h.setup() + const held = await startHeldAtBootWait() + held.finish(false) + + await expect(held.start).resolves.toMatchObject({ + state: 'failed', + failedStage: 'agent_readiness', + recovery: expect.stringContaining('worker-release') + }) + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toMatchObject({ + ownership_state: 'owned', + terminal_handle: 'term_worker' + }) + + await expect( + h.call('orchestration.workerRelease', { dispatch: held.dispatchId }) + ).resolves.toMatchObject({ state: 'released', processAction: 'closed_agent_terminal' }) + expect(h.runtime.closeTerminal).toHaveBeenCalledWith('term_worker') + }) + + it('promises no cleanup while the start outcome is still unknown', async () => { + h.setup() + const task = h.db.createTask({ spec: 'unknown outcome', runId: h.activeRunId }) + const unknown = Object.assign(new Error('the execution host went away'), { + code: 'operation_unknown' + }) + vi.spyOn(h.runtime, 'waitForTerminal').mockRejectedValue(unknown) + + const receipt = (await h.call('orchestration.workerStart', { + task: task.id, + from: 'term_coord', + agent: 'codex' + })) as { state: string; dispatchId: string; nextCommands?: string[] } + + expect(receipt).toMatchObject({ state: 'outcome_unknown' }) + // worker-release refuses an unsettled worker, so the receipt must not name it. + expect(receipt).not.toHaveProperty('recovery') + expect(receipt.nextCommands?.join(' ')).toContain('worker-abandon') + expect(h.db.getWorkerTerminalResourceByOwner(receipt.dispatchId)).toMatchObject({ + ownership_state: 'owned' + }) + }) + + it('promises no cleanup for a reused terminal whose start died', async () => { + h.setup() + const held = await startHeldAtBootWait({ terminal: 'term_worker' }) + held.finish(false) + + const receipt = await held.start + expect(receipt).toMatchObject({ state: 'failed' }) + expect(receipt).not.toHaveProperty('recovery') + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toBeUndefined() + }) +}) + +describe('custody refuses a dispatch that stopped while its terminal was being created', () => { + let db: OrchestrationDb | undefined + + afterEach(() => db?.close()) + + it('records no owner once the dispatch is no longer starting', () => { + const d = (db = new OrchestrationDb(':memory:')) + const started = d.createStartingWorkerDispatch({ + creator: { kind: 'system' }, + maxDepth: Number.MAX_SAFE_INTEGER, + taskId: d.createTask({ runId: 'run_legacy_local', spec: 'stopped mid-create' }).id, + startOptions: {} + }) + // Startup reconciliation abandons a `starting` worker whose terminal it cannot find. + d.reconcileMissingWorkerTerminal(started.dispatch.id, 'runtime restarted') + + expect(() => + d.recordCreatedWorkerTerminalCustody({ + dispatchId: started.dispatch.id, + handle: 'term_worker', + paneKey: 'tab_w:leaf_w', + processIncarnation: 'pty_w:1', + worktreeId: 'repo::worktree' + }) + ).toThrow(/is not starting/) + expect(d.getWorkerTerminalResourceByOwner(started.dispatch.id)).toBeUndefined() + }) +}) diff --git a/src/renderer/src/i18n/en-runtime-required.json b/src/renderer/src/i18n/en-runtime-required.json index 47024572e8c..a1c33b790bc 100644 --- a/src/renderer/src/i18n/en-runtime-required.json +++ b/src/renderer/src/i18n/en-runtime-required.json @@ -1503,16 +1503,7 @@ "ask_before_closing_running_terminals_description": "Show a confirmation before closing a terminal that has a running command or agent.", "ask_before_closing_running_terminals_title": "Ask Before Closing Running Terminals", "cc8c5ca224": "Windows default", - "d78fc4fdef": "Loading distributions", - "minimumContrast": { - "automatic": "Automatic: {{light}} on light backgrounds, {{dark}} on dark.", - "description": "Lifts terminal foreground colors that sit too close to the background. Leave blank for automatic, or set 1 to render program colors exactly as sent.", - "disabled": "Correction off. Programs that rely on low contrast, like Powerline separators, render as sent.", - "pinned": "Targets {{ratio}}:1 contrast for foreground colors, where possible.", - "placeholder": "Auto", - "suffix": "blank = automatic, 1 = off", - "title": "Minimum Contrast Ratio" - } + "d78fc4fdef": "Loading distributions" }, "TerminalSettingsPreview": { "d06664e889": "dark"