From 2fcbd468db5a1534a6218d9bdf227a0a129d0e75 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 20 Aug 2026 04:20:56 -0400 Subject: [PATCH] fix(terminal): preserve settled prompt authority --- config/reliability-gates.jsonc | 25 +++++-- src/main/runtime/orca-runtime.test.ts | 59 +++++++++++++++ src/main/runtime/orca-runtime.ts | 36 +++++---- src/main/runtime/rpc/methods/terminal.ts | 11 ++- .../rpc/terminal-agent-prompt-send.test.ts | 73 ++++++++++++++++++- 5 files changed, 183 insertions(+), 21 deletions(-) diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index a49258bf157..6320991a144 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -9413,13 +9413,16 @@ "providers": ["local", "daemon", "ssh", "remote-runtime"], "coveredPlatforms": ["macos", "windows"], "coveredProviders": ["local"], - "coverageNotes": "Source-built macOS and physical Windows Electron journeys cover the public CLI through RPC, runtime identity, real PTY/ConPTY writes, and a delayed fake Codex composer. Focused units cover CLI intent, fresh Claude/Codex foreground identity, framing bytes, stale-agent shell fallback, and exact legacy behavior for every other configured agent. SSH, daemon, and remote-runtime remain explicit gaps.", + "coverageNotes": "Source-built macOS and physical Windows Electron journeys cover the public CLI through RPC, runtime identity, real PTY/ConPTY writes, and a delayed fake Codex composer. Focused units cover CLI intent, transient wrapper resolution, fresh foreground identity over stale launch metadata, paired-mobile floor takeover before delayed Enter, framing bytes, stale-agent shell fallback, and exact legacy behavior for every other configured agent. SSH, daemon, and remote-runtime live evidence remain explicit gaps.", "motivatingLinks": [ "https://linear.app/stably/issue/STA-4328", + "https://linear.app/stably/issue/STA-4486", + "https://linear.app/stably/issue/STA-4487", + "https://linear.app/stably/issue/STA-4488", "https://github.com/stablyai/orca/issues/14525" ], - "invariant": "A public CLI send containing non-empty text plus Enter and no interrupt identifies agent-prompt intent. The owning runtime uses bracketed-paste and settled submit only when a fresh foreground-process read positively identifies the exact target as Claude or Codex on a desktop call; every other agent, stale agent metadata over a shell, text-only input, bare Enter, interrupts, mobile input, query replies, and older clients keep the direct terminal contract.", - "oracle": "Launch a bare fake Codex through the real Electron PTY and delay its composer render for 1,200 ms, beyond the legacy generic 500 ms gap. Send a 557-byte prompt with the public CLI and require the marker, zero premature Enter, no rescue Enter, and one submission. POSIX child stdin must retain one bracketed-paste frame; Windows ConPTY consumes that control frame, so runtime tests require the frame at the PTY write boundary while the Windows child requires the exact prompt. The same harness against the installed pre-fix runtime must record one premature Enter and require one bare rescue Enter. Focused units require CLI intent only for text plus Enter, settlement only for freshly identified Claude/Codex, the legacy platform delay for every other configured agent, stale Codex launch identity over a shell to fall back, no wrapper polling, and byte-for-byte direct delivery for non-targets.", + "invariant": "A public CLI send containing non-empty text plus Enter and no interrupt identifies agent-prompt intent. The owning runtime uses bracketed-paste and settled submit only when a bounded foreground read positively identifies the exact target as Claude or Codex on a desktop call; known transient wrappers may resolve within the existing bound, and proven current foreground identity outranks historical launch identity for render settlement. The exact PTY binding and desktop input-floor authority are rechecked before every paste chunk and delayed Enter. Every other agent, stale agent metadata over a shell, text-only input, bare Enter, interrupts, mobile input, query replies, and older clients keep the direct terminal contract.", + "oracle": "Launch a bare fake Codex through the real Electron PTY and delay its composer render for 1,200 ms, beyond the legacy generic 500 ms gap. Send a 557-byte prompt with the public CLI and require the marker, zero premature Enter, no rescue Enter, and one submission. POSIX child stdin must retain one bracketed-paste frame; Windows ConPTY consumes that control frame, so runtime tests require the frame at the PTY write boundary while the Windows child requires the exact prompt. The same harness against the installed pre-fix runtime must record one premature Enter and require one bare rescue Enter. Focused units drive a node-to-Codex wrapper transition with fake time, prove fresh foreground Codex outranks stale non-target launch identity at the render gate, and transfer the paired-mobile floor between paste and suffix while recording request, PTY generation, owner, exact writes, and ordering. The baseline must fall back or write Enter in all three cases; the candidate must settle once or reject before cross-owner Enter, with no direct fallback or duplicate input.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts src/cli/handlers/terminal.test.ts src/main/runtime/rpc/terminal-agent-prompt-send.test.ts src/main/runtime/rpc/terminal-send.test.ts src/main/runtime/orca-runtime.test.ts src/shared/agent-prompt-injection.test.ts", "pnpm run build:cli && pnpm exec playwright test tests/e2e/terminal-send-agent-prompt-submit.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1", @@ -9447,7 +9450,8 @@ "file": "src/main/runtime/rpc/terminal-agent-prompt-send.test.ts", "assertions": [ "a freshly proven Claude or Codex desktop target uses settled prompt delivery", - "a non-target agent or shell preserves the existing direct send" + "a non-target agent or shell preserves the existing direct send", + "a paired mobile floor takeover after paste rejects the delayed Enter without direct fallback" ] }, { @@ -9456,6 +9460,8 @@ "Claude and Codex wait for marker-gated composer quiescence before submit", "every other configured TUI agent retains the legacy platform delay", "a speculative CLI prompt check does not poll wrapper foregrounds", + "a known transient wrapper resolves to Codex within the bounded foreground retry", + "proven current Codex foreground identity controls settlement ahead of stale launch identity", "the PTY write boundary retains bracketed framing and one delayed submit" ] }, @@ -9504,6 +9510,15 @@ "result": "passed", "durationSeconds": 23.2, "summary": "After rebasing onto current main, the final narrowed runtime/CLI/RPC suite passed 1,201 tests with one skip, including fresh Claude/Codex identity, stale-agent shell fallback, and legacy timing for every other configured agent." + }, + { + "date": "2026-08-20", + "runner": "local", + "platform": "macos", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/cli/handlers/terminal.test.ts src/main/runtime/rpc/terminal-agent-prompt-send.test.ts src/main/runtime/rpc/terminal-send.test.ts src/main/runtime/orca-runtime.test.ts src/shared/agent-prompt-injection.test.ts", + "result": "passed", + "durationSeconds": 12.9, + "summary": "The 1,227-test candidate gate passed with one existing skip. Its three controlled oracles also failed on origin/main and with only the candidate production hunks disabled, then passed together on the candidate: node-to-Codex wrapper settlement, current foreground over stale launch identity, and paired-mobile floor takeover before delayed Enter." } ], "runtimeBudget": { @@ -9520,7 +9535,7 @@ }, "performanceBudget": { "required": true, - "evidence": "Only a CLI request with non-empty text plus Enter and no interrupt adds a bounded fresh foreground read and one non-retrying liveness check. Positively identified Claude and Codex reuse the O(prompt bytes) chunked prompt sender plus one PTY-local listener until post-marker quiescence or the bounded fallback. Every other agent and shell retains the existing direct-send path. Text-only, bare Enter, interrupt, mobile, renderer stream input, and query replies add no work." + "evidence": "Only a CLI request with non-empty text plus Enter and no interrupt adds foreground classification. Directly recognized targets keep the existing two-read bound; only known node/python wrappers reuse the existing 150 ms, 6.5 second bounded resolver, and the deterministic transition test records three provider reads for node-to-Codex settlement. Positively identified Claude and Codex reuse the O(prompt bytes) chunked prompt sender plus one PTY-local listener until post-marker quiescence or the bounded fallback. Floor checks are PTY-local map reads before existing writes; no session listing, subprocess, listener, timer, or provider fanout is added elsewhere." }, "promotionCriteria": [ "Collect live Codex and Claude CLI sends on representative slow systems.", diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index dab6531c425..b2196eb48c2 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -17049,6 +17049,40 @@ describe('OrcaRuntimeService', () => { } }) + it('uses proven current Codex foreground identity ahead of stale launch identity', async () => { + vi.useFakeTimers() + try { + const writes: string[] = [] + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn: vi.fn().mockResolvedValue({ id: 'pty-bg' }), + write: (_ptyId, data) => { + writes.push(data) + acknowledgeAgentPromptSubmit(runtime, 'pty-bg', data) + return true + }, + kill: () => true, + getForegroundProcess: async () => 'codex' + }) + const { handle } = await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { + launchAgent: 'aider' + }) + + await expect(runtime.isTerminalRunningSettledPromptAgent(handle)).resolves.toBe(true) + const sendPromise = runtime.sendTerminalAgentPrompt(handle, 'review this change') + await vi.advanceTimersByTimeAsync(AGENT_PROMPT_SUBMIT_DELAY_MS) + expect(writes).not.toContain('\r') + + runtime.onPtyData('pty-bg', '\x1b[?25hcomposer rendered', Date.now()) + await vi.advanceTimersByTimeAsync(1_500) + await sendPromise + + expect(writes.filter((data) => data === '\r')).toHaveLength(1) + } finally { + vi.useRealTimers() + } + }) + it('submits a silent Claude composer once after the bounded render fallback', async () => { vi.useFakeTimers() try { @@ -24149,6 +24183,31 @@ describe('OrcaRuntimeService', () => { expect(getForegroundProcess).toHaveBeenCalledTimes(1) }) + it('settles a CLI prompt after a transient foreground wrapper resolves to Codex', async () => { + vi.useFakeTimers() + const getForegroundProcess = vi.fn().mockResolvedValueOnce('node').mockResolvedValue('codex') + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn: vi.fn().mockResolvedValue({ id: 'pty-bg' }), + write: () => true, + kill: () => true, + getForegroundProcess + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + const { handle } = await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { + command: 'bash', + title: 'bash' + }) + syncSinglePty(runtime, 'pty-bg', { paneTitle: 'bash' }) + + const settled = runtime.isTerminalRunningSettledPromptAgent(handle) + await vi.advanceTimersByTimeAsync(150) + + await expect(settled).resolves.toBe(true) + expect(getForegroundProcess).toHaveBeenCalledTimes(3) + }) + it.each(['claude', 'codex'] as const)( 'authorizes settled CLI prompts only after positive %s foreground identity', async (agent) => { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 5e48785e74e..ed8545b68e0 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -550,7 +550,8 @@ import { repoIsRemote } from '../../shared/agent-launch-remote' import { isAgentForegroundWrapperProcess, isExpectedAgentProcess, - recognizeAgentProcess + recognizeAgentProcess, + type RecognizedAgentProcess } from '../../shared/agent-process-recognition' import { haveSameDisabledTuiAgents, @@ -19505,7 +19506,7 @@ export class OrcaRuntimeService { dispose: () => void } | null { const pty = this.ptysById.get(ptyId) - const agent = pty?.launchAgent ?? pty?.foregroundAgent + const agent = pty?.foregroundAgent ?? pty?.launchAgent if (!isTerminalSendSettlementAgent(agent)) { return null } @@ -34642,7 +34643,8 @@ export class OrcaRuntimeService { if (!ptyId || !trackedPty || !this.ptyController) { return false } - const recognized = recognizeAgentProcess(await this.ptyController.getForegroundProcess(ptyId)) + const foregroundProcess = await this.ptyController.getForegroundProcess(ptyId) + const recognized = await this.recognizeForegroundAgentProcess(ptyId, foregroundProcess) const recognizedAgent = recognized?.agent if (!isTerminalSendSettlementAgent(recognizedAgent)) { return false @@ -34729,19 +34731,27 @@ export class OrcaRuntimeService { foregroundProcess: string, options: { suppressClaude?: boolean; retryWrappers?: boolean } = {} ): Promise { + return (await this.recognizeForegroundAgentProcess(ptyId, foregroundProcess, options)) !== null + } + + private async recognizeForegroundAgentProcess( + ptyId: string, + foregroundProcess: string | null, + options: { suppressClaude?: boolean; retryWrappers?: boolean } = {} + ): Promise { const initialRecognition = recognizeAgentProcess(foregroundProcess) if (initialRecognition !== null) { - return !( - options.suppressClaude === true && + return options.suppressClaude === true && isExpectedAgentProcess(initialRecognition.processName, 'claude') - ) + ? null + : initialRecognition } if ( options.retryWrappers === false || !this.isAgentWrapperForegroundProcess(foregroundProcess) || !this.ptyController ) { - return false + return null } const startedAt = Date.now() while (Date.now() - startedAt < FOREGROUND_AGENT_WRAPPER_RETRY_TIMEOUT_MS) { @@ -34751,19 +34761,19 @@ export class OrcaRuntimeService { const refreshedProcess = await this.ptyController.getForegroundProcess(ptyId) const refreshedRecognition = recognizeAgentProcess(refreshedProcess) if (refreshedRecognition !== null) { - return !( - options.suppressClaude === true && + return options.suppressClaude === true && isExpectedAgentProcess(refreshedRecognition.processName, 'claude') - ) + ? null + : refreshedRecognition } if (!refreshedProcess || !this.isAgentWrapperForegroundProcess(refreshedProcess)) { - return false + return null } } - return false + return null } - private isAgentWrapperForegroundProcess(processName: string): boolean { + private isAgentWrapperForegroundProcess(processName: string | null): boolean { // Why: daemon/SSH PTYs can report the interpreter before the async cmdline cache resolves; retry only known wrappers. return isAgentForegroundWrapperProcess(processName) } diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index 5dee92a7676..0ac99785e10 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -1402,7 +1402,16 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ } const mobileFloorClientId = resolveMobileFloorClientId(driver, params.client) const mobileFloorClaim: MobileInputFloorClaimHolder = { current: null } - const beforeWrite = assertSendPreconditions + const assertPromptInputAuthority = + params.agentPrompt === true && params.client?.type === 'desktop' + ? (ptyId?: string): void => { + assertTerminalSendExactPtyBinding(runtime, params.terminal, ptyId) + if (ptyId && isTerminalInputLockedForClient(runtime, ptyId, params.client)) { + throw new Error('terminal_guard_not_writable') + } + } + : undefined + const beforeWrite = assertSendPreconditions ?? assertPromptInputAuthority const useSettledAgentPrompt = params.agentPrompt === true && hasText && diff --git a/src/main/runtime/rpc/terminal-agent-prompt-send.test.ts b/src/main/runtime/rpc/terminal-agent-prompt-send.test.ts index eb7624c95e4..7ddf9e495eb 100644 --- a/src/main/runtime/rpc/terminal-agent-prompt-send.test.ts +++ b/src/main/runtime/rpc/terminal-agent-prompt-send.test.ts @@ -45,7 +45,7 @@ describe('terminal agent prompt send RPC', () => { expect(response.ok).toBe(true) expect(runtime.isTerminalRunningSettledPromptAgent).toHaveBeenCalledWith('terminal-1') expect(sendTerminalAgentPrompt).toHaveBeenCalledWith('terminal-1', 'review this change', { - beforeWrite: undefined, + beforeWrite: expect.any(Function), signal: undefined }) expect(sendTerminal).not.toHaveBeenCalled() @@ -81,8 +81,77 @@ describe('terminal agent prompt send RPC', () => { expect(sendTerminal).toHaveBeenCalledWith( 'terminal-1', { text: 'echo x', enter: true, interrupt: false }, - { beforeWrite: undefined } + { beforeWrite: expect.any(Function) } ) expect(sendTerminalAgentPrompt).not.toHaveBeenCalled() }) + + it('does not send delayed Enter after a paired mobile client takes the floor', async () => { + const writes: string[] = [] + const ordering: string[] = [] + let floorOwner: 'desktop' | 'mobile' = 'desktop' + let releaseSettlement!: () => void + const settlement = new Promise((resolve) => { + releaseSettlement = resolve + }) + let pasteWritten!: () => void + const paste = new Promise((resolve) => { + pasteWritten = resolve + }) + const sendTerminal = vi.fn() + const sendTerminalAgentPrompt = vi.fn( + async ( + _handle: string, + _prompt: string, + options: { beforeWrite?: (ptyId: string) => void } + ) => { + await options.beforeWrite?.('pty-1') + writes.push('prompt') + ordering.push('request-4488:generation-1:desktop:paste') + pasteWritten() + await settlement + await options.beforeWrite?.('pty-1') + writes.push('\r') + ordering.push('request-4488:generation-1:mobile:enter') + return { handle: 'terminal-1', accepted: true, bytesWritten: 7 } + } + ) + const runtime = makeRuntime({ + resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + getDriver: vi.fn(() => + floorOwner === 'mobile' + ? { kind: 'mobile' as const, clientId: 'mobile-1' } + : { kind: 'desktop' as const } + ), + isTerminalRunningSettledPromptAgent: vi.fn().mockResolvedValue(true), + sendTerminal, + sendTerminalAgentPrompt + }) + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + + const response = dispatcher.dispatch( + makeRequest({ + terminal: 'terminal-1', + text: 'prompt', + enter: true, + agentPrompt: true, + client: { id: 'orca-cli', type: 'desktop' } + }) + ) + await paste + floorOwner = 'mobile' + ordering.push('request-4488:generation-1:mobile:floor') + releaseSettlement() + + await expect(response).resolves.toMatchObject({ + ok: true, + result: { send: { accepted: false, bytesWritten: 0 } } + }) + expect(writes).toEqual(['prompt']) + expect(ordering).toEqual([ + 'request-4488:generation-1:desktop:paste', + 'request-4488:generation-1:mobile:floor' + ]) + expect(sendTerminal).not.toHaveBeenCalled() + }) })