From 2d5fc76d8fbea6ce50bc54dfba6c7af908c16675 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 2 Oct 2026 07:40:54 -0700 Subject: [PATCH] fix(orchestration): take a foreground-proven launch without a dialog settle With agent hooks off, a worker whose brief rode its launch line is judged by the agent holding its terminal. That verdict used to wait a further 2 s for a startup dialog to settle; it now counts as soon as the foreground read proves the agent and a direct read of the screen shows no startup dialog. The foreground read itself is unchanged. --- .../launch-turn-start-observation.test.ts | 63 ++++++++++++++++++- .../runtime/launch-turn-start-observation.ts | 10 ++- ...untime-agent-prompt-request-correlation.ts | 6 ++ .../orca-runtime-launch-turn-start.test.ts | 21 +++++++ .../worker/worker-launch-brief.test.ts | 29 ++++----- .../worker/worker-start-turn-observation.ts | 24 +------ 6 files changed, 113 insertions(+), 40 deletions(-) diff --git a/src/main/runtime/launch-turn-start-observation.test.ts b/src/main/runtime/launch-turn-start-observation.test.ts index 76396f33ac0..75cd3511e92 100644 --- a/src/main/runtime/launch-turn-start-observation.test.ts +++ b/src/main/runtime/launch-turn-start-observation.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { LAUNCH_HOOK_SILENCE_MS, observeLaunchTurnStart, @@ -15,6 +15,7 @@ function probe(overrides: Partial): LaunchTurnStartProbe { observeHookTurn: pendingHookTurn, hookReachedPane: () => false, readWorkingSequence: () => 0, + dialogOnScreen: () => false, launchRecorded: () => true, readForeground: async () => 'agent', ...overrides @@ -74,4 +75,64 @@ describe('observeLaunchTurnStart', () => { ) ).resolves.toBe('unobserved') }) + + // Why pinned: a worker start with hooks off must be no slower than main, which settled on its + // first evidence; a fixed wait here is the receipt arriving seconds after the brief. + describe('with hooks off, on fake time', () => { + afterEach(() => vi.useRealTimers()) + + /** The verdict once `ms` of fake time has passed, or `pending`. */ + async function verdictAfter(observing: Promise, ms: number): Promise { + let verdict = 'pending' + void observing.then((value) => { + verdict = value + }) + await vi.advanceTimersByTimeAsync(ms) + return verdict + } + + // The foreground read is a process scan that can take seconds under load; it is the only wait. + it('takes the agent proven in front as soon as the read answers, with no dialog on screen', async () => { + vi.useFakeTimers() + const readForeground = () => + new Promise<'agent'>((resolve) => setTimeout(() => resolve('agent'), 4_000)) + const observing = observeLaunchTurnStart( + probe({ observeHookTurn: undefined, readForeground }), + { launchStartedAt: Date.now(), timeoutMs: 30_000 } + ) + await expect(verdictAfter(observing, 3_999)).resolves.toBe('pending') + await expect(verdictAfter(observing, 1)).resolves.toBe('unsupported') + }) + + it('does not take the agent in front while a startup dialog is on screen', async () => { + vi.useFakeTimers() + let dialog = true + const observing = observeLaunchTurnStart( + probe({ observeHookTurn: undefined, dialogOnScreen: () => dialog }), + { launchStartedAt: Date.now(), timeoutMs: 30_000 } + ) + await expect(verdictAfter(observing, 5_000)).resolves.toBe('pending') + dialog = false + await expect(verdictAfter(observing, 1_000)).resolves.toBe('unsupported') + }) + }) + + it('waits the hook silence out, and no longer, when hooks are on but never report', async () => { + vi.useFakeTimers() + try { + let verdict = 'pending' + void observeLaunchTurnStart(probe({}), { + launchStartedAt: Date.now(), + timeoutMs: 30_000 + }).then((value) => { + verdict = value + }) + await vi.advanceTimersByTimeAsync(LAUNCH_HOOK_SILENCE_MS - 250) + expect(verdict).toBe('pending') + await vi.advanceTimersByTimeAsync(500) + expect(verdict).toBe('unsupported') + } finally { + vi.useRealTimers() + } + }) }) diff --git a/src/main/runtime/launch-turn-start-observation.ts b/src/main/runtime/launch-turn-start-observation.ts index 29a83c15ce7..75319177e21 100644 --- a/src/main/runtime/launch-turn-start-observation.ts +++ b/src/main/runtime/launch-turn-start-observation.ts @@ -25,6 +25,8 @@ export type LaunchTurnStartProbe = { hookReachedPane: () => boolean /** The pane's title/lifecycle turn-start count: the evidence main accepted. */ readWorkingSequence: () => number + /** Whether a startup dialog is on the pane's screen now; read directly, so it needs no quiet. */ + dialogOnScreen: () => boolean /** Whether the launch command is still recorded on its PTY; the shell's command-finished retires it. */ launchRecorded: () => boolean readForeground: () => Promise @@ -35,8 +37,9 @@ export type LaunchTurnStartProbe = { * * The agent's hook is the proof wherever it can give one. A launch whose hooks never reach the pane * (turned off, or not installed on that host) is judged as main judged it: a title turn-start edge, - * or else the agent holding its terminal, which carries the prompt on its command line. A launch - * the shell reports finished, with no agent in front, exited at startup. + * or else the agent holding its terminal with no startup dialog on screen, since it carries the + * prompt on its command line. A launch the shell reports finished, with no agent in front, exited at + * startup. */ export async function observeLaunchTurnStart( probe: LaunchTurnStartProbe, @@ -87,7 +90,8 @@ async function watchLaunchEvidence( } if (Date.now() >= nextForegroundReadAt) { nextForegroundReadAt = Date.now() + LAUNCH_FOREGROUND_READ_MS - if ((await probe.readForeground()) === 'agent') { + // A dialog on screen holds the agent; the caller's dialog watch reports it. + if ((await probe.readForeground()) === 'agent' && !probe.dialogOnScreen()) { return 'unsupported' } } diff --git a/src/main/runtime/orca-runtime-agent-prompt-request-correlation.ts b/src/main/runtime/orca-runtime-agent-prompt-request-correlation.ts index f4912c06d2a..d163d301052 100644 --- a/src/main/runtime/orca-runtime-agent-prompt-request-correlation.ts +++ b/src/main/runtime/orca-runtime-agent-prompt-request-correlation.ts @@ -17,6 +17,7 @@ import { observeLaunchTurnStart, type LaunchTurnStartVerdict } from './launch-turn-start-observation' +import { readFreshComposerHold } from './launched-agent-composer-readiness' export class OrcaRuntimeWithAgentPromptRequestCorrelation extends OrcaRuntimeWithSerializeAgentPromptSubmission { private readonly agentPromptCorrelation = new AgentPromptRequestCorrelation() @@ -136,6 +137,11 @@ export class OrcaRuntimeWithAgentPromptRequestCorrelation extends OrcaRuntimeWit : {}), hookReachedPane: () => this.getFreshExplicitAgentStatusForPty(handle, ptyId) !== null, readWorkingSequence: () => this.getAgentPromptActivity(handle, ptyId).workingSequence, + dialogOnScreen: () => + readFreshComposerHold( + this.getTerminalAgentStatusSnapshot(handle, ptyId).waitText, + this.readLiveTerminalScreenLines(ptyId) + ) === 'dialog', launchRecorded: () => Boolean(this.ptysById.get(ptyId)?.launchAgent), readForeground: async () => agent ? await this.readLaunchedAgentForeground(ptyId, agent) : 'unknown' diff --git a/src/main/runtime/orca-runtime-launch-turn-start.test.ts b/src/main/runtime/orca-runtime-launch-turn-start.test.ts index 80d517ce0b5..1bc22b36671 100644 --- a/src/main/runtime/orca-runtime-launch-turn-start.test.ts +++ b/src/main/runtime/orca-runtime-launch-turn-start.test.ts @@ -122,6 +122,27 @@ describe('observeTerminalLaunchTurnStart', () => { } ) + // Why: Claude's hook is its usual proof; with hooks turned off it must not wait out the silence. + it.skipIf(process.platform === 'win32')( + 'judges a Claude launch at once when hooks are turned off', + async () => { + const { runtime, handle } = await launchedCodex(() => [], { + hooksEnabled: false, + foreground: () => 'claude' + }) + const startedAt = Date.now() + + await expect( + runtime.observeTerminalLaunchTurnStart( + handle, + { launchStartedAt: startedAt, agent: 'claude' }, + 5_000 + ) + ).resolves.toBe('unsupported') + expect(Date.now() - startedAt).toBeLessThan(1_000) + } + ) + it.skipIf(process.platform === 'win32')( 'reports a launch the shell finished, with the shell back in front, as exited', async () => { diff --git a/src/main/runtime/rpc/methods/orchestration/worker/worker-launch-brief.test.ts b/src/main/runtime/rpc/methods/orchestration/worker/worker-launch-brief.test.ts index bb674ab6066..b61b030d1e6 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/worker-launch-brief.test.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/worker-launch-brief.test.ts @@ -151,21 +151,22 @@ describe('worker-start with the brief on the launch command line', () => { }) }) - it('is not ready while a trust dialog holds it', async () => { + // Why pinned: the observer reads no dialog on screen before it says so; settling here again + // put the receipt seconds behind main with hooks off. + it('is ready at once, without waiting on the startup dialog watch', async () => { h.setup() - vi.mocked(h.runtime.waitForTerminal).mockResolvedValue({ - handle: 'term_worker', - condition: 'tui-idle', - satisfied: false, - status: 'running', - exitCode: null, - blockedReason: 'agent-trust-workspace' - }) - await expect(startProvenByForeground('trust-blocked worker')).resolves.toMatchObject({ - state: 'outcome_unknown', - stage: 'turn_start_blocked', - lastError: expect.stringContaining('Agent startup blocked: agent-trust-workspace') - }) + vi.mocked(h.runtime.waitForTerminal).mockReturnValue(new Promise(() => {})) + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] }) + try { + let receipt: unknown = 'pending' + void startProvenByForeground('fast worker').then((value) => { + receipt = value + }) + await vi.advanceTimersByTimeAsync(100) + expect(receipt).toMatchObject({ state: 'ready', turnStart: 'unsupported' }) + } finally { + vi.useRealTimers() + } }) }) diff --git a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-turn-observation.ts b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-turn-observation.ts index 89c358b894b..0d788b2d385 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-turn-observation.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-turn-observation.ts @@ -97,8 +97,6 @@ export function describeUnobservedWorkerTurnStart(agent: string | null): string /** Pause before re-arming a dialog watch that found the agent idle; an idle screen re-reads instantly. */ const STARTUP_DIALOG_REWATCH_MS = 500 -/** Past the 1.5 s quiet a dialog needs to read as one, so a dialog already painting is caught. */ -const STARTUP_DIALOG_SETTLE_MS = 2_000 /** * Turn-start verdict for a worker whose brief rode its launch command line. The agent's hook turn @@ -139,14 +137,10 @@ export async function observeWorkerLaunchTurnStart(args: { if (verdict === 'exited') { throw new Error('Agent exited before its first turn started; the shell is back in front.') } - if (verdict === 'observed') { + // `unsupported` already read no startup dialog on screen, so neither waits for one to settle. + if (verdict === 'observed' || verdict === 'unsupported') { return { verdict } } - if (verdict === 'unsupported') { - // Why: an agent proven in front can still be held by a dialog that has not settled on screen. - const blockedReason = await settledWithin(dialog, STARTUP_DIALOG_SETTLE_MS) - return blockedReason ? blockedLaunchObservation(blockedReason) : { verdict } - } // Why: a dialog already on screen outranks any verdict short of an observed turn. const blockedReason = await settledOrNull(dialog) if (blockedReason) { @@ -229,17 +223,3 @@ async function abortableDelay(ms: number, signal: AbortSignal): Promise { async function settledOrNull(promise: Promise): Promise { return await Promise.race([promise, Promise.resolve(null)]) } - -async function settledWithin(promise: Promise, ms: number): Promise { - let timer: ReturnType | undefined - try { - return await Promise.race([ - promise, - new Promise((resolve) => { - timer = setTimeout(() => resolve(null), ms) - }) - ]) - } finally { - clearTimeout(timer) - } -}