diff --git a/src/main/pi/agent-status-extension-omp-lifecycle.test.ts b/src/main/pi/agent-status-extension-omp-lifecycle.test.ts new file mode 100644 index 00000000000..9ff37cb0859 --- /dev/null +++ b/src/main/pi/agent-status-extension-omp-lifecycle.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it, vi } from 'vitest' + +import { createAgentStatusExtensionHarness } from './agent-status-extension-test-harness' + +function postedHookNames(fetchMock: ReturnType): string[] { + return fetchMock.mock.calls.map( + (call) => JSON.parse(String(call[1]?.body)).payload.hook_event_name as string + ) +} + +const OMP_RUNTIME_CASES = [ + ['configured OMP', { kind: 'omp' as const }], + ['title-routed OMP', { kind: 'pi' as const, title: 'omp' }], + ['argv-routed OMP', { kind: 'pi' as const, argv: ['node', '/usr/local/bin/omp'] }] +] as const + +describe('OMP agent_end contract', () => { + it.each(OMP_RUNTIME_CASES)( + 'keeps %s working when agent_end will continue', + async (_name, args) => { + vi.useFakeTimers() + try { + const harness = createAgentStatusExtensionHarness(args) + const context = { isIdle: vi.fn(() => true) } + + await harness.callHook('agent_start') + await harness.callHook('agent_end', { willContinue: true }, context) + await vi.advanceTimersByTimeAsync(1_000) + + expect(postedHookNames(harness.fetchMock)).toEqual(['agent_start']) + expect(context.isIdle).not.toHaveBeenCalled() + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } + } + ) + + it.each(OMP_RUNTIME_CASES)( + 'settles a completed %s turn without waiting for ctx.isIdle', + async (_name, args) => { + for (const event of [{ willContinue: false }, {}]) { + const harness = createAgentStatusExtensionHarness(args) + const context = { isIdle: vi.fn(() => false) } + + await harness.callHook('agent_start') + await harness.callHook('agent_end', event, context) + + await vi.waitFor(() => + expect(postedHookNames(harness.fetchMock)).toEqual(['agent_start', 'agent_end']) + ) + expect(context.isIdle).not.toHaveBeenCalled() + } + } + ) + + it('settles a later terminal OMP agent_end after a continuation', async () => { + const harness = createAgentStatusExtensionHarness({ kind: 'omp' }) + const context = { isIdle: vi.fn(() => false) } + + await harness.callHook('agent_start') + await harness.callHook('agent_end', { willContinue: true }, context) + expect(postedHookNames(harness.fetchMock)).toEqual(['agent_start']) + + await harness.callHook('agent_end', { willContinue: false }, context) + await vi.waitFor(() => + expect(postedHookNames(harness.fetchMock)).toEqual(['agent_start', 'agent_end']) + ) + }) + + it('does not apply the OMP contract to Pi or Prime', async () => { + vi.useFakeTimers() + try { + for (const kind of ['pi', 'prime-agent'] as const) { + const harness = createAgentStatusExtensionHarness({ kind }) + const context = { isIdle: vi.fn(() => false) } + + await harness.callHook('agent_end', { willContinue: false }, context) + await vi.advanceTimersByTimeAsync(1_000) + + expect(postedHookNames(harness.fetchMock)).toEqual([]) + expect(context.isIdle).toHaveBeenCalled() + } + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/main/pi/agent-status-extension-source.test.ts b/src/main/pi/agent-status-extension-source.test.ts index b735cd60198..9f9e5b598d9 100644 --- a/src/main/pi/agent-status-extension-source.test.ts +++ b/src/main/pi/agent-status-extension-source.test.ts @@ -1,193 +1,9 @@ -import { runInNewContext } from 'node:vm' -// TypeScript 7 is a native CLI; transpile tests still need the legacy JavaScript API. -import ts from 'typescript-api' import { describe, expect, it, vi } from 'vitest' -import { getPiAgentStatusExtensionSource } from './agent-status-extension-source' - -type HookContext = { - isIdle?: () => boolean - sessionManager?: { - getSessionId?: () => unknown - getSessionFile?: () => unknown - } -} - -type HookHandler = (event?: unknown, context?: HookContext) => Promise | void - -type FakeCurlChild = { - on: ReturnType - stdin: { - on: ReturnType - end: ReturnType - } -} - -type Harness = { - fetchMock: ReturnType - spawnMock: ReturnType - spawnedChildren: FakeCurlChild[] - fsMock: { - existsSync: ReturnType - readFileSync: ReturnType - statSync: ReturnType - } - handlers: Record - processEnv: Record - callHook: (name: string, event?: unknown, context?: HookContext) => Promise - // Re-invoke the extension factory in the same process (as Pi does on an - // in-process extension reload), swapping in the freshly registered handlers. - reload: () => void -} - -const BASE_ENV = { - ORCA_PANE_KEY: 'pane-1', - ORCA_AGENT_LAUNCH_TOKEN: 'launch-1', - ORCA_TAB_ID: 'tab-1', - ORCA_WORKTREE_ID: 'tree-1', - ORCA_AGENT_HOOK_PORT: '4321', - ORCA_AGENT_HOOK_TOKEN: 'token-1', - ORCA_AGENT_HOOK_ENV: 'env-1', - ORCA_AGENT_HOOK_VERSION: '1.2.3' -} satisfies Record - -// Why: ownership keys on process.pid, so reload and child-process tests need -// stable, distinct identities. -const SELF_PID = 4242 - -function createHarness(args: { - kind: 'pi' | 'omp' | 'prime-agent' - env?: Record - pid?: number - title?: string - argv?: string[] - existsSync?: (path: string) => boolean - readFileSync?: (path: string, encoding: string) => string - statSync?: (path: string) => { mtimeMs: number; size: number; ino: number } - fetchImpl?: (...params: Parameters) => Promise -}): Harness { - const fetchMock = vi.fn( - args.fetchImpl ?? - (async () => ({ - ok: true - })) - ) - - const spawnedChildren: FakeCurlChild[] = [] - const spawnMock = vi.fn(() => { - const child: FakeCurlChild = { - on: vi.fn(), - stdin: { - on: vi.fn(), - end: vi.fn() - } - } - spawnedChildren.push(child) - return child - }) - - const fsMock = { - existsSync: vi.fn(args.existsSync ?? (() => false)), - statSync: vi.fn( - args.statSync ?? - ((path: string) => { - throw Object.assign(new Error(`ENOENT: ${path}`), { code: 'ENOENT' }) - }) - ), - readFileSync: vi.fn( - args.readFileSync ?? - ((path: string) => { - throw Object.assign(new Error(`ENOENT: ${path}`), { code: 'ENOENT' }) - }) - ) - } - - const module = { - exports: {} as { default?: (pi: { on: (name: string, handler: HookHandler) => void }) => void } - } - const requireMock = vi.fn((specifier: string) => { - if (specifier === 'fs') { - return fsMock - } - if (specifier === 'child_process') { - return { spawn: spawnMock } - } - throw new Error(`unexpected require(${specifier})`) - }) - - const processMock = { - env: { - ...BASE_ENV, - ...(args.kind === 'prime-agent' ? { PRIME_AGENT_INTERNAL_DAEMON_WORKER: '1' } : {}), - ...args.env - }, - pid: args.pid ?? SELF_PID, - title: args.title ?? 'node', - argv: args.argv ?? ['node', '/usr/bin/orca'] - } - - const context = { - module, - exports: module.exports, - require: requireMock, - process: processMock, - fetch: fetchMock, - console: { - warn: vi.fn(), - error: vi.fn(), - log: vi.fn() - }, - Promise, - Buffer, - URL, - AbortController, - setTimeout, - clearTimeout - } as Record - context.globalThis = context - - const source = getPiAgentStatusExtensionSource(args.kind) - const output = ts.transpileModule(source, { - compilerOptions: { - module: ts.ModuleKind.CommonJS, - target: ts.ScriptTarget.ES2020 - } - }).outputText - runInNewContext(output, context) - - const register = module.exports.default - if (!register) { - throw new Error('expected default export from generated source') - } - - const handlers: Record = {} - const registerInto = (target: Record): void => { - register({ - on(name: string, handler: HookHandler) { - target[name] = handler - } - }) - } - registerInto(handlers) - - return { - fetchMock, - spawnMock, - spawnedChildren, - fsMock, - handlers, - processEnv: processMock.env, - callHook: async (name, event, hookContext) => { - await handlers[name]?.(event, hookContext) - }, - reload: () => { - for (const key of Object.keys(handlers)) { - delete handlers[key] - } - registerInto(handlers) - } - } -} +import { + AGENT_STATUS_EXTENSION_SELF_PID as SELF_PID, + createAgentStatusExtensionHarness as createHarness +} from './agent-status-extension-test-harness' describe('getPiAgentStatusExtensionSource', () => { it('registers Prime hooks only in the event-emitting daemon worker', () => { @@ -826,10 +642,10 @@ describe('getPiAgentStatusExtensionSource', () => { } }) - it('keeps reporting Pi-compatible agents once their agent_end handlers settle', async () => { + it('keeps polling Pi and Prime until their agent_end handlers settle', async () => { vi.useFakeTimers() try { - for (const kind of ['pi', 'omp', 'prime-agent'] as const) { + for (const kind of ['pi', 'prime-agent'] as const) { const harness = createHarness({ kind }) let idle = false const context = { isIdle: vi.fn(() => idle) } diff --git a/src/main/pi/agent-status-extension-test-harness.ts b/src/main/pi/agent-status-extension-test-harness.ts new file mode 100644 index 00000000000..eec2b615017 --- /dev/null +++ b/src/main/pi/agent-status-extension-test-harness.ts @@ -0,0 +1,190 @@ +import { runInNewContext } from 'node:vm' +// TypeScript 7 is a native CLI; transpile tests still need the legacy JavaScript API. +import ts from 'typescript-api' +import { vi } from 'vitest' + +import { getPiAgentStatusExtensionSource } from './agent-status-extension-source' + +export type HookContext = { + isIdle?: () => boolean + sessionManager?: { + getSessionId?: () => unknown + getSessionFile?: () => unknown + } +} + +export type HookHandler = (event?: unknown, context?: HookContext) => Promise | void + +type FakeCurlChild = { + on: ReturnType + stdin: { + on: ReturnType + end: ReturnType + } +} + +export type AgentStatusExtensionHarness = { + fetchMock: ReturnType + spawnMock: ReturnType + spawnedChildren: FakeCurlChild[] + fsMock: { + existsSync: ReturnType + readFileSync: ReturnType + statSync: ReturnType + } + handlers: Record + processEnv: Record + callHook: (name: string, event?: unknown, context?: HookContext) => Promise + // Re-invoke the extension factory in the same process (as Pi does on an + // in-process extension reload), swapping in the freshly registered handlers. + reload: () => void +} + +const BASE_ENV = { + ORCA_PANE_KEY: 'pane-1', + ORCA_AGENT_LAUNCH_TOKEN: 'launch-1', + ORCA_TAB_ID: 'tab-1', + ORCA_WORKTREE_ID: 'tree-1', + ORCA_AGENT_HOOK_PORT: '4321', + ORCA_AGENT_HOOK_TOKEN: 'token-1', + ORCA_AGENT_HOOK_ENV: 'env-1', + ORCA_AGENT_HOOK_VERSION: '1.2.3' +} satisfies Record + +// Why: ownership keys on process.pid, so reload and child-process tests need +// stable, distinct identities. +export const AGENT_STATUS_EXTENSION_SELF_PID = 4242 + +export function createAgentStatusExtensionHarness(args: { + kind: 'pi' | 'omp' | 'prime-agent' + env?: Record + pid?: number + title?: string + argv?: readonly string[] + existsSync?: (path: string) => boolean + readFileSync?: (path: string, encoding: string) => string + statSync?: (path: string) => { mtimeMs: number; size: number; ino: number } + fetchImpl?: (...params: Parameters) => Promise +}): AgentStatusExtensionHarness { + const fetchMock = vi.fn( + args.fetchImpl ?? + (async () => ({ + ok: true + })) + ) + + const spawnedChildren: FakeCurlChild[] = [] + const spawnMock = vi.fn(() => { + const child: FakeCurlChild = { + on: vi.fn(), + stdin: { + on: vi.fn(), + end: vi.fn() + } + } + spawnedChildren.push(child) + return child + }) + + const fsMock = { + existsSync: vi.fn(args.existsSync ?? (() => false)), + statSync: vi.fn( + args.statSync ?? + ((path: string) => { + throw Object.assign(new Error(`ENOENT: ${path}`), { code: 'ENOENT' }) + }) + ), + readFileSync: vi.fn( + args.readFileSync ?? + ((path: string) => { + throw Object.assign(new Error(`ENOENT: ${path}`), { code: 'ENOENT' }) + }) + ) + } + + const module = { + exports: {} as { default?: (pi: { on: (name: string, handler: HookHandler) => void }) => void } + } + const requireMock = vi.fn((specifier: string) => { + if (specifier === 'fs') { + return fsMock + } + if (specifier === 'child_process') { + return { spawn: spawnMock } + } + throw new Error(`unexpected require(${specifier})`) + }) + + const processMock = { + env: { + ...BASE_ENV, + ...(args.kind === 'prime-agent' ? { PRIME_AGENT_INTERNAL_DAEMON_WORKER: '1' } : {}), + ...args.env + }, + pid: args.pid ?? AGENT_STATUS_EXTENSION_SELF_PID, + title: args.title ?? 'node', + argv: args.argv ?? ['node', '/usr/bin/orca'] + } + + const context = { + module, + exports: module.exports, + require: requireMock, + process: processMock, + fetch: fetchMock, + console: { + warn: vi.fn(), + error: vi.fn(), + log: vi.fn() + }, + Promise, + Buffer, + URL, + AbortController, + setTimeout, + clearTimeout + } as Record + context.globalThis = context + + const source = getPiAgentStatusExtensionSource(args.kind) + const output = ts.transpileModule(source, { + compilerOptions: { + module: ts.ModuleKind.CommonJS, + target: ts.ScriptTarget.ES2020 + } + }).outputText + runInNewContext(output, context) + + const register = module.exports.default + if (!register) { + throw new Error('expected default export from generated source') + } + + const handlers: Record = {} + const registerInto = (target: Record): void => { + register({ + on(name: string, handler: HookHandler) { + target[name] = handler + } + }) + } + registerInto(handlers) + + return { + fetchMock, + spawnMock, + spawnedChildren, + fsMock, + handlers, + processEnv: processMock.env, + callHook: async (name, event, hookContext) => { + await handlers[name]?.(event, hookContext) + }, + reload: () => { + for (const key of Object.keys(handlers)) { + delete handlers[key] + } + registerInto(handlers) + } + } +} diff --git a/src/main/pi/agent-status-handler-source.ts b/src/main/pi/agent-status-handler-source.ts index fa9de9d9bd4..1bb431285e2 100644 --- a/src/main/pi/agent-status-handler-source.ts +++ b/src/main/pi/agent-status-handler-source.ts @@ -115,7 +115,8 @@ export function getPiAgentStatusHandlerSourceLines(kind: PiAgentKind): string[] ' })', '', ' // Why: modern Pi stays non-idle across retry/compaction/follow-up work,', - ' // while legacy Pi/OMP becomes idle after its final agent_end handlers.', + ' // while legacy Pi becomes idle after its final agent_end handlers.', + ' // OMP instead marks non-terminal agent_end events with willContinue.', ' const AGENT_END_IDLE_RECHECK_MS = 25', ' const AGENT_END_IDLE_RECHECK_MAX_MS = 250', ' let agentSettledSupported = false', @@ -167,8 +168,14 @@ export function getPiAgentStatusHandlerSourceLines(kind: PiAgentKind): string[] ' postAgentEndOnce()', ' })', '', - " pi.on('agent_end', (_event, ctx) => {", + " pi.on('agent_end', (event, ctx) => {", ...captureSessionMetadata, + ' if (isOmpRuntime()) {', + ' clearPendingAgentEndCheck()', + ' if (event?.willContinue === true) return', + ' postAgentEndOnce()', + ' return', + ' }', ' if (agentSettledSupported) return', " if (!ctx || typeof ctx.isIdle !== 'function') {", ' postAgentEndOnce()', diff --git a/src/main/runtime/command-finished-launch-authority.test.ts b/src/main/runtime/command-finished-launch-authority.test.ts new file mode 100644 index 00000000000..bfda7ad05a5 --- /dev/null +++ b/src/main/runtime/command-finished-launch-authority.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest' + +import { shouldRetireLaunchAuthorityOnCommandFinished } from './command-finished-launch-authority' + +describe('shouldRetireLaunchAuthorityOnCommandFinished', () => { + it('does not retire while a recognized agent is still foreground', () => { + expect(shouldRetireLaunchAuthorityOnCommandFinished('omp')).toBe(false) + expect(shouldRetireLaunchAuthorityOnCommandFinished('omp.exe')).toBe(false) + expect(shouldRetireLaunchAuthorityOnCommandFinished('/usr/local/bin/omp')).toBe(false) + expect(shouldRetireLaunchAuthorityOnCommandFinished('pi')).toBe(false) + expect(shouldRetireLaunchAuthorityOnCommandFinished('codex')).toBe(false) + }) + + it('retires only when the execution host shows a shell in the foreground', () => { + expect(shouldRetireLaunchAuthorityOnCommandFinished('zsh')).toBe(true) + expect(shouldRetireLaunchAuthorityOnCommandFinished('bash')).toBe(true) + expect(shouldRetireLaunchAuthorityOnCommandFinished('powershell.exe')).toBe(true) + expect(shouldRetireLaunchAuthorityOnCommandFinished('pwsh')).toBe(true) + }) + + it('treats a missing foreground read as unverifiable, not exited', () => { + expect(shouldRetireLaunchAuthorityOnCommandFinished(null)).toBe(false) + expect(shouldRetireLaunchAuthorityOnCommandFinished('')).toBe(false) + }) + + it('does not treat an unknown non-shell process as agent exit', () => { + expect(shouldRetireLaunchAuthorityOnCommandFinished('node')).toBe(false) + expect(shouldRetireLaunchAuthorityOnCommandFinished('python3')).toBe(false) + }) +}) diff --git a/src/main/runtime/command-finished-launch-authority.ts b/src/main/runtime/command-finished-launch-authority.ts new file mode 100644 index 00000000000..dafb6e2451a --- /dev/null +++ b/src/main/runtime/command-finished-launch-authority.ts @@ -0,0 +1,25 @@ +import { recognizeAgentProcess } from '../../shared/agent-process-recognition' +import { isShellProcess } from '../../shared/shell-process-detection' + +// Why: match the renderer nested-shell settle window so a leaked OSC 133;D +// from an agent's bash/tool child is not treated as the launched agent exiting. +export const COMMAND_FINISHED_LAUNCH_AUTHORITY_SETTLE_MS = 350 + +/** + * True when a command-finished OSC should retire launched-agent hook authority. + * + * Nested tool shells leak OSC 133;D onto the main PTY while the agent is still + * foreground. Loss of contact (null) is unverifiable, not exited, so it must + * not retire a live pane. + */ +export function shouldRetireLaunchAuthorityOnCommandFinished( + foregroundProcess: string | null +): boolean { + if (!foregroundProcess) { + return false + } + if (recognizeAgentProcess(foregroundProcess)) { + return false + } + return isShellProcess(foregroundProcess) +} diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 3688fec9c39..5b05091ead0 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -64,6 +64,7 @@ import { resolveWorktreeScanCacheTtlMs, type RuntimeTerminalAgentStatusEvent } from './orca-runtime' +import { COMMAND_FINISHED_LAUNCH_AUTHORITY_SETTLE_MS } from './command-finished-launch-authority' import { RUNTIME_GRAPH_RELOAD_TIMEOUT_MS } from './runtime-graph-reload-lifecycle' import { appendRecentPtyPathCandidates, @@ -13182,7 +13183,8 @@ describe('OrcaRuntimeService', () => { spawn, write: () => true, kill: () => true, - getForegroundProcess: async () => null + getForegroundProcess: async () => 'zsh', + confirmForegroundProcess: async () => 'zsh' }) runtime.setNotifier({ worktreesChanged: vi.fn(), @@ -13220,7 +13222,14 @@ describe('OrcaRuntimeService', () => { }) ).toBeDefined() - runtime.onPtyData('pty-authority', '\x1b]133;D;0\x07', 100) + vi.useFakeTimers() + try { + runtime.onPtyData('pty-authority', '\x1b]133;D;0\x07', 100) + expect(retireAuthority).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(COMMAND_FINISHED_LAUNCH_AUTHORITY_SETTLE_MS) + } finally { + vi.useRealTimers() + } expect(retireAuthority).toHaveBeenCalledWith(spawnEnv.ORCA_PANE_KEY) expect(runtime.verifyOrchestrationCompatibilityCaller(evidence)).toBeNull() @@ -13231,11 +13240,118 @@ describe('OrcaRuntimeService', () => { ).toBeUndefined() }) - it('retires only receipted restored PTY authority on command completion and exit', () => { + it('keeps launched-agent authority when command-finished still shows OMP foreground', async () => { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-omp-nested', incarnationId: 'process-omp' }) + const retireAuthority = vi.fn() + const runtime = new OrcaRuntimeService(store, undefined, { + attestAgentHookCompatibilityAuthority: (candidate) => ({ + paneKey: candidate.paneKey, + source: 'current_hook' + }), + retireAgentHookCompatibilityAuthority: retireAuthority + }) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => 'omp', + confirmForegroundProcess: async () => 'omp' + }) + runtime.setNotifier({ + worktreesChanged: vi.fn(), + reposChanged: vi.fn(), + activateWorktree: vi.fn(), + createTerminal: vi.fn(), + revealTerminalSession: vi.fn().mockResolvedValue({ tabId: 'tab-omp-nested' }), + splitTerminal: vi.fn(), + renameTerminal: vi.fn(), + focusTerminal: vi.fn(), + closeTerminal: vi.fn(), + sleepWorktree: vi.fn(), + terminalFitOverrideChanged: vi.fn(), + terminalDriverChanged: vi.fn() + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + + await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { + command: 'omp', + launchConfig: { agentCommand: 'omp', agentArgs: '', agentEnv: {} } + }) + + vi.useFakeTimers() + try { + runtime.onPtyData('pty-omp-nested', '\x1b]133;D;0\x07', 100) + await vi.advanceTimersByTimeAsync(COMMAND_FINISHED_LAUNCH_AUTHORITY_SETTLE_MS) + } finally { + vi.useRealTimers() + } + + expect(retireAuthority).not.toHaveBeenCalled() + }) + + it('does not treat an unverifiable command-finished foreground read as agent exit', async () => { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-unverified', incarnationId: 'process-u' }) + const retireAuthority = vi.fn() + const runtime = new OrcaRuntimeService(store, undefined, { + attestAgentHookCompatibilityAuthority: (candidate) => ({ + paneKey: candidate.paneKey, + source: 'current_hook' + }), + retireAgentHookCompatibilityAuthority: retireAuthority + }) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + confirmForegroundProcess: async () => null + }) + runtime.setNotifier({ + worktreesChanged: vi.fn(), + reposChanged: vi.fn(), + activateWorktree: vi.fn(), + createTerminal: vi.fn(), + revealTerminalSession: vi.fn().mockResolvedValue({ tabId: 'tab-unverified' }), + splitTerminal: vi.fn(), + renameTerminal: vi.fn(), + focusTerminal: vi.fn(), + closeTerminal: vi.fn(), + sleepWorktree: vi.fn(), + terminalFitOverrideChanged: vi.fn(), + terminalDriverChanged: vi.fn() + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + + await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { + command: 'omp', + launchConfig: { agentCommand: 'omp', agentArgs: '', agentEnv: {} } + }) + + vi.useFakeTimers() + try { + runtime.onPtyData('pty-unverified', '\x1b]133;D;0\x07', 100) + await vi.advanceTimersByTimeAsync(COMMAND_FINISHED_LAUNCH_AUTHORITY_SETTLE_MS) + } finally { + vi.useRealTimers() + } + + expect(retireAuthority).not.toHaveBeenCalled() + }) + + it('retires only receipted restored PTY authority on command completion and exit', async () => { const retireAuthority = vi.fn() const runtime = new OrcaRuntimeService(store, undefined, { retireAgentHookCompatibilityAuthority: retireAuthority }) + runtime.setPtyController({ + spawn: vi.fn(), + write: () => true, + kill: () => true, + getForegroundProcess: async () => 'zsh', + confirmForegroundProcess: async () => 'zsh' + }) const internals = runtime as unknown as { recordPtyWorktree: (ptyId: string, worktreeId: string, state: Record) => void restoredOrchestrationAuthorityByPtyId: Map> @@ -13277,10 +13393,17 @@ describe('OrcaRuntimeService', () => { hostScope: { kind: 'local', hostId: 'local' } }) - runtime.emitDaemonPtyTransientFact('pty-restored-command', { - kind: 'command-finished', - exitCode: 0 - }) + vi.useFakeTimers() + try { + runtime.emitDaemonPtyTransientFact('pty-restored-command', { + kind: 'command-finished', + exitCode: 0 + }) + expect(retireAuthority).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(COMMAND_FINISHED_LAUNCH_AUTHORITY_SETTLE_MS) + } finally { + vi.useRealTimers() + } runtime.onPtyExit('pty-restored-exit', 0, 'restored-exit') runtime.onPtyExit('pty-ordinary-shell', 0, 'ordinary-shell') diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index aac73f5e7f4..de28bc2eebb 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -43,6 +43,10 @@ import type { TerminalSideEffectBatch, TerminalSideEffectFact } from '../../shared/terminal-side-effect-facts' +import { + COMMAND_FINISHED_LAUNCH_AUTHORITY_SETTLE_MS, + shouldRetireLaunchAuthorityOnCommandFinished +} from './command-finished-launch-authority' import type { TerminalGitHubPRLink } from '../../shared/terminal-github-pr-link-detector' import { TerminalKittyKeyboardModeTracker } from '../../shared/terminal-kitty-keyboard-mode-tracker' import { parseTerminalKittyKeyboardFlags } from '../../shared/terminal-kitty-keyboard-flags' @@ -3661,6 +3665,7 @@ export class OrcaRuntimeService { string, RestoredOrchestrationAuthorityReceipt >() + private launchAuthorityCommandFinishedGenerationByPtyId = new Map() private ptyControllerInventorySequence = 0 private ptyControllerAggregateInventoryGeneration = 0 private ptyControllerInventoryGenerationByProvider = new Map() @@ -11459,7 +11464,7 @@ export class OrcaRuntimeService { this.recordTerminalSideEffectFact(ptyId, { kind: 'bell' }) return case 'command-finished': - this.retirePtyAgentLaunchAuthority(ptyId) + void this.maybeRetireLaunchAuthorityAfterCommandFinished(ptyId) this.recordTerminalSideEffectFact(ptyId, { kind: 'command-finished', exitCode: fact.exitCode @@ -11748,7 +11753,7 @@ export class OrcaRuntimeService { this.confirmPtyAgentExit(ptyId) }, onCommandFinished: (exitCode: number | null) => { - this.retirePtyAgentLaunchAuthority(ptyId) + void this.maybeRetireLaunchAuthorityAfterCommandFinished(ptyId) this.recordTerminalSideEffectFact(ptyId, { kind: 'command-finished', exitCode }) }, onBell: () => { @@ -13963,6 +13968,7 @@ export class OrcaRuntimeService { } private retirePtyAgentLaunchAuthority(ptyId: string): void { + this.launchAuthorityCommandFinishedGenerationByPtyId.delete(ptyId) const pty = this.ptysById.get(ptyId) if (!pty) { return @@ -13991,6 +13997,41 @@ export class OrcaRuntimeService { } } + // Why: full-screen agents leak nested-shell OSC 133;D onto the main PTY. + // Retire launched-agent hook authority only after the execution host shows + // a shell in the foreground; a failed/null read is unverifiable, not exited. + private async maybeRetireLaunchAuthorityAfterCommandFinished(ptyId: string): Promise { + const generation = (this.launchAuthorityCommandFinishedGenerationByPtyId.get(ptyId) ?? 0) + 1 + this.launchAuthorityCommandFinishedGenerationByPtyId.set(ptyId, generation) + await new Promise((resolve) => { + const timer = setTimeout(resolve, COMMAND_FINISHED_LAUNCH_AUTHORITY_SETTLE_MS) + if (typeof timer.unref === 'function') { + timer.unref() + } + }) + if (this.launchAuthorityCommandFinishedGenerationByPtyId.get(ptyId) !== generation) { + return + } + if (!this.ptysById.get(ptyId)) { + return + } + let foregroundProcess: string | null = null + try { + foregroundProcess = + (await (this.ptyController?.confirmForegroundProcess?.(ptyId) ?? + this.ptyController?.getForegroundProcess(ptyId))) ?? null + } catch { + return + } + if (this.launchAuthorityCommandFinishedGenerationByPtyId.get(ptyId) !== generation) { + return + } + if (!shouldRetireLaunchAuthorityOnCommandFinished(foregroundProcess)) { + return + } + this.retirePtyAgentLaunchAuthority(ptyId) + } + async resolveTerminalCwd(handle: string): Promise { const ptyId = this.resolveLeafForHandle(handle)?.ptyId if (!ptyId) { diff --git a/src/shared/agent-hook-listener-pi-compatible.test.ts b/src/shared/agent-hook-listener-pi-compatible.test.ts index b0519378b18..4aaa5440ee6 100644 --- a/src/shared/agent-hook-listener-pi-compatible.test.ts +++ b/src/shared/agent-hook-listener-pi-compatible.test.ts @@ -308,6 +308,25 @@ describe('shared agent-hook-listener', () => { toolInput: 'pnpm test' }) expect(tool?.payload.interactivePrompt).toBeUndefined() + + const done = normalizeHookPayload( + state, + 'omp', + { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + env: 'production', + version: '1', + payload: { hook_event_name: 'agent_end' } + }, + 'production' + ) + expect(done?.payload).toMatchObject({ + state: 'done', + prompt: 'wire omp status', + agentType: 'omp' + }) }) it('maps OMP ask to blocked without publishing a native prompt', () => {