From a2082fcda8c781971ead5b07f32379c89cd8b4cb Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 20 Aug 2026 06:12:29 -0400 Subject: [PATCH] fix(terminal): require fresh settled prompt proof --- config/reliability-gates.jsonc | 28 +++++--- src/main/providers/ssh-pty-provider.test.ts | 7 ++ src/main/providers/ssh-pty-provider.ts | 4 ++ src/main/runtime/orca-runtime.test.ts | 66 +++++++++++++++++++ src/main/runtime/orca-runtime.ts | 24 ++++--- .../ssh-docker-agent-prompt-submit.spec.ts | 7 +- 6 files changed, 117 insertions(+), 19 deletions(-) diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index f24d81a9ef7..0da24ed4ea6 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -5586,7 +5586,7 @@ "platform": "macos", "command": "SKIP_BUILD=1 pnpm exec playwright test tests/e2e/terminal-streaming-refocus-viewport.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1 --repeat-each=5", "result": "passed", - "durationSeconds": 35.2, + "durationSeconds": 38.7, "summary": "Five consecutive Electron iterations passed after replacing fixed-time readiness and stale tab capture with deterministic viewport and pane-identity oracles." } ], @@ -9425,6 +9425,7 @@ "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 exec vitest run --config config/vitest.config.ts src/cli/handlers/terminal.test.ts src/main/providers/ssh-pty-provider.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", "pnpm run build:relay && ORCA_E2E_SSH_DOCKER=1 pnpm exec playwright test tests/e2e/ssh-docker-agent-prompt-submit.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1", "pnpm.cmd exec playwright test tests/e2e/terminal-send-agent-prompt-submit.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1", @@ -9432,6 +9433,7 @@ ], "testFiles": [ "src/cli/handlers/terminal.test.ts", + "src/main/providers/ssh-pty-provider.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", @@ -9448,6 +9450,12 @@ "text-only and bare Enter remain direct terminal input" ] }, + { + "file": "src/main/providers/ssh-pty-provider.test.ts", + "assertions": [ + "SSH strong confirmation uses the existing fresh relay foreground request without a wire addition" + ] + }, { "file": "src/main/runtime/rpc/terminal-agent-prompt-send.test.ts", "assertions": [ @@ -9463,9 +9471,11 @@ "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", + "an unresolved wrapper exhausts exactly 45 foreground reads at 6.6 seconds of fake time and writes nothing", "current foreground proof stays request-local inside the PTY generation queue and outranks stale launch identity", "identity transition, unavailable inspection, cancellation, and concurrent classification cannot write, fall back, or fan out", "foreground authority is rechecked after an awaited pre-write guard and before delayed Enter", + "fresh provider confirmation overrides cache-backed target evidence before settled paste and suffix writes", "the PTY write boundary retains bracketed framing and one delayed submit" ] }, @@ -9482,7 +9492,7 @@ "file": "tests/e2e/ssh-docker-agent-prompt-submit.spec.ts", "assertions": [ "two half-open SSH relay wrapper inspections precede a bounded retry that identifies Codex", - "the remote fake agent receives one post-render Enter and no premature Enter", + "the remote fake agent receives exactly one framed prompt plus one post-render Enter and no other bytes", "process inspection stays within the asserted retry budget", "execution stays on the SSH host, teardown is unconditional, and an unrelated remote PTY remains writable" ] @@ -9529,10 +9539,10 @@ "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", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/cli/handlers/terminal.test.ts src/main/providers/ssh-pty-provider.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": 11.6, - "summary": "The final 1,235-test candidate gate passed with one existing skip in 11.6s. Its three ticket oracles fail on origin/main and with only the candidate production hunks disabled, then pass together on the candidate. Follow-up adversarial contracts also prove request-local identity, prompt refusal after foreground loss or unavailable inspection, abort-aware foreground reads, serialized classification, and a non-yielding floor check beside each PTY write." + "durationSeconds": 10.8, + "summary": "The final 1,264-test candidate gate passed with one existing skip in 10.8s. Its three ticket oracles fail on origin/main and with only the candidate production hunks disabled, then pass together on the candidate. Follow-up adversarial contracts also prove request-local identity, fresh provider confirmation, prompt refusal after foreground loss or unavailable inspection, abort-aware foreground reads, serialized classification, bounded unresolved-wrapper reads, and a non-yielding floor check beside each PTY write." }, { "date": "2026-08-20", @@ -9549,8 +9559,8 @@ "platform": "linux", "command": "pnpm run build:relay && ORCA_E2E_SSH_DOCKER=1 pnpm exec playwright test tests/e2e/ssh-docker-agent-prompt-submit.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1", "result": "passed", - "durationSeconds": 37.2, - "summary": "The candidate passed against an ephemeral Debian SSH target and real relay with two controlled half-open process reads, an asserted upper retry bound, one post-render remote Enter, SSH execution-host proof, bounded best-effort terminal close followed by unconditional target cleanup, and an unrelated live remote PTY. The byte-identical oracle fails when only settled-wrapper delivery is disabled because legacy delivery settles before the retry barrier." + "durationSeconds": 34.1, + "summary": "The candidate passed against an ephemeral Debian SSH target and real relay with two controlled half-open process reads, an asserted upper retry bound, the exact complete framed prompt plus one post-render remote Enter, fresh SSH foreground confirmation over the existing relay request, SSH execution-host proof, bounded best-effort terminal close followed by unconditional target cleanup, and an unrelated live remote PTY. The byte-identical oracle fails when only settled-wrapper delivery is disabled because legacy delivery settles before the retry barrier." } ], "runtimeBudget": { @@ -9563,11 +9573,11 @@ }, "redGreenEvidence": { "status": "complete", - "evidence": "The same controlled ticket oracles are red on origin/main and with each production fix disabled, then green together on the candidate. With only the reviewer-driven structural changes disabled, the byte-identical identity-transition, wrapper-cancellation, and synchronous-floor tests all fail; restored candidate passes all three. The paired-mobile Electron oracle is green on the candidate and red without the delayed authority recheck. The Docker SSH oracle is green on the candidate and red with only settled-wrapper delivery disabled, where legacy delivery settles before the retry barrier. Existing macOS and Windows fake-composer evidence remains green with prematureEnters=0 and rescueSent=false." + "evidence": "The committed commands above use the same test sources for every comparison. Overlaying the candidate test-only diffs on affected baseline 4b2ed5ddd470fc69cf836d840748afae0908eeb4 and rebased base c92f394cde2612e0a913bf447e94ab1f0b0f9789 makes the three ticket oracles red; the candidate is green; reverse-applying only production commits 2fcbd468db, de4463b977, and a5116e485e with the tests retained makes them red again. The reviewer authority tests are 4/4 red at test-only commit 5418e31585 and green after a5116e485e. The fresh-confirmation oracle is red on a5116e485e: cache-backed Codex evidence writes the complete prompt and Enter while the provider's fresh confirmation reports zsh. The first Docker rerun with strong confirmation but no SSH provider implementation rejected safely with accepted=false; adding SSH confirmation over the existing fresh relay request makes the byte-identical oracle green. The paired-mobile Electron oracle is green on the candidate and red without the delayed authority recheck. The Docker SSH oracle is green on the candidate and red with only settled-wrapper delivery disabled, where legacy delivery settles before the retry barrier. Existing macOS and Windows fake-composer evidence remains green with prematureEnters=0 and rescueSent=false." }, "performanceBudget": { "required": true, - "evidence": "Only a CLI request with non-empty text plus Enter and no interrupt adds foreground classification. The proof runs inside the existing PTY/generation submission queue, so concurrent sends cannot overlap provider scans. Direct targets use three identity/liveness reads; a node-to-Codex transition uses four, while only known node/python wrappers can enter the existing cancellable 150 ms, 6.5 second resolver. Positively identified Claude and Codex reuse the O(prompt bytes) chunked sender plus one PTY-local listener. Floor checks remain constant-time PTY-local map reads at each write yield; no session listing, global fanout, extra listener, or new timer is added." + "evidence": "Only a CLI request with non-empty text plus Enter and no interrupt adds foreground classification. The proof runs inside the existing PTY/generation submission queue, so concurrent sends cannot overlap provider scans. Classification uses three foreground reads for a direct target and four for a node-to-Codex transition; the two existing pre-write rechecks use strong provider confirmation when available without adding calls. Only known node/python wrappers can enter the cancellable 150 ms, 6.5 second retry window; an in-flight read remains bounded by its provider's existing transport timeout. Positively identified Claude and Codex reuse the O(prompt bytes) chunked sender plus one PTY-local listener. Floor checks remain constant-time PTY-local map reads at each write yield; no session listing, global fanout, extra listener, or new timer is added." }, "promotionCriteria": [ "Collect live Codex and Claude CLI sends on representative slow systems.", diff --git a/src/main/providers/ssh-pty-provider.test.ts b/src/main/providers/ssh-pty-provider.test.ts index 0fc8702750d..4f4e057ec71 100644 --- a/src/main/providers/ssh-pty-provider.test.ts +++ b/src/main/providers/ssh-pty-provider.test.ts @@ -230,6 +230,13 @@ describe('SshPtyProvider', () => { expectRequest(mux.request, 'pty.getForegroundProcess', { id: 'pty-1' }) }) + it('confirms foreground process with a fresh relay request', async () => { + mux.request.mockResolvedValue('codex') + + await expect(provider.confirmForegroundProcess(scopedPty1)).resolves.toBe('codex') + expectRequest(mux.request, 'pty.getForegroundProcess', { id: 'pty-1' }) + }) + it('preserves unavailable process inspection', async () => { const inspection = { foregroundProcess: null, diff --git a/src/main/providers/ssh-pty-provider.ts b/src/main/providers/ssh-pty-provider.ts index 99df854f4d0..6e967a48bc2 100644 --- a/src/main/providers/ssh-pty-provider.ts +++ b/src/main/providers/ssh-pty-provider.ts @@ -260,6 +260,10 @@ export class SshPtyProvider implements IPtyProvider { return result as string | null } + async confirmForegroundProcess(id: string): Promise { + return await this.getForegroundProcess(id) + } + async inspectProcess(id: string): Promise { return (await this.mux.request('pty.inspectProcess', { id: this.toRelayPtyId(id) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 0963e6543a5..05007e00ce2 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -24269,6 +24269,42 @@ describe('OrcaRuntimeService', () => { expect(writes).toEqual([]) }) + it('uses fresh provider confirmation before settled writes', async () => { + vi.useFakeTimers() + const writes: string[] = [] + const getForegroundProcess = vi.fn().mockResolvedValue('codex') + const confirmForegroundProcess = vi.fn().mockResolvedValue('zsh') + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn: vi.fn().mockResolvedValue({ id: 'pty-bg' }), + write: (_ptyId, data) => { + writes.push(data) + if (data.includes(AGENT_PROMPT_BRACKETED_PASTE_END)) { + setTimeout(() => runtime.onPtyData('pty-bg', '\x1b[?25h', Date.now()), 0) + } + acknowledgeAgentPromptSubmit(runtime, 'pty-bg', data) + return true + }, + kill: () => true, + getForegroundProcess, + confirmForegroundProcess + }) + const { handle } = await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { + command: 'bash', + title: 'bash' + }) + const send = runtime.sendTerminalAgentPrompt(handle, 'review this change', { + requireSettledForeground: true + }) + const rejected = expect(send).rejects.toThrow('terminal_guard_not_writable') + + await vi.runAllTimersAsync() + + await rejected + expect(confirmForegroundProcess).toHaveBeenCalled() + expect(writes).toEqual([]) + }) + it('revalidates settled foreground after the pre-write guard yields', async () => { const writes: string[] = [] const getForegroundProcess = vi @@ -24372,6 +24408,36 @@ describe('OrcaRuntimeService', () => { expect(writes).toEqual([]) }) + it('refuses an unresolved wrapper at the retry ceiling without writing', async () => { + vi.useFakeTimers() + const writes: string[] = [] + const getForegroundProcess = vi.fn().mockResolvedValue('node') + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn: vi.fn().mockResolvedValue({ id: 'pty-bg' }), + write: (_ptyId, data) => { + writes.push(data) + return true + }, + kill: () => true, + getForegroundProcess + }) + const { handle } = await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { + command: 'bash', + title: 'bash' + }) + const send = runtime.sendTerminalAgentPrompt(handle, 'review this change', { + requireSettledForeground: true + }) + const rejected = expect(send).rejects.toThrow('terminal_guard_not_writable') + + await vi.advanceTimersByTimeAsync(6_600) + + await rejected + expect(getForegroundProcess).toHaveBeenCalledTimes(45) + expect(writes).toEqual([]) + }) + it('cancels an in-flight settled foreground inspection without writing', async () => { const controller = new AbortController() let inspectionStarted!: () => void diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index d6d52b1e1bd..58ee8199ead 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -19547,19 +19547,27 @@ export class OrcaRuntimeService { expectedAgent: TuiAgent, signal?: AbortSignal ): Promise { - if (!this.ptyController) { - throw new Error('terminal_guard_not_writable') - } - const foregroundProcess = await waitForAgentPromptPromise( - this.ptyController.getForegroundProcess(ptyId), - signal - ) + const foregroundProcess = await this.readSettledPromptForegroundConfirmation(ptyId, signal) assertAgentPromptRequestActive(signal) if (recognizeAgentProcess(foregroundProcess)?.agent !== expectedAgent) { throw new Error('terminal_guard_not_writable') } } + private async readSettledPromptForegroundConfirmation( + ptyId: string, + signal?: AbortSignal + ): Promise { + const controller = this.ptyController + if (!controller) { + throw new Error('terminal_guard_not_writable') + } + const foreground = controller.confirmForegroundProcess + ? controller.confirmForegroundProcess(ptyId) + : controller.getForegroundProcess(ptyId) + return await waitForAgentPromptPromise(foreground, signal) + } + private createAgentPromptRenderGate( ptyId: string, provenAgent?: TuiAgent @@ -34753,7 +34761,7 @@ export class OrcaRuntimeService { } assertAgentPromptRequestActive(signal) const confirmed = recognizeAgentProcess( - await waitForAgentPromptPromise(this.ptyController.getForegroundProcess(ptyId), signal) + await this.readSettledPromptForegroundConfirmation(ptyId, signal) ) assertAgentPromptRequestActive(signal) if (confirmed?.agent !== recognized.agent) { diff --git a/tests/e2e/ssh-docker-agent-prompt-submit.spec.ts b/tests/e2e/ssh-docker-agent-prompt-submit.spec.ts index 0c7ce76cd42..f0d7011e1fe 100644 --- a/tests/e2e/ssh-docker-agent-prompt-submit.spec.ts +++ b/tests/e2e/ssh-docker-agent-prompt-submit.spec.ts @@ -3,6 +3,10 @@ import { readFileSync } from 'node:fs' import path from 'node:path' import { promisify } from 'node:util' import type { Page } from '@stablyai/playwright-test' +import { + AGENT_PROMPT_SUBMIT, + buildAgentPromptPasteBytes +} from '../../src/shared/agent-prompt-injection' import { expect, test } from './helpers/orca-app' import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' import { @@ -248,8 +252,7 @@ test.describe('Docker SSH settled prompt submission', () => { await expect(send).resolves.toMatchObject({ send: { accepted: true } }) await expect.poll(() => remoteFile(target!, REMOTE_REPORT)).not.toBe('') const observedInput = remoteInput(target) - expect(observedInput).toContain(prompt) - expect(observedInput.match(/\r/g)).toHaveLength(1) + expect(observedInput).toBe(`${buildAgentPromptPasteBytes(prompt)}${AGENT_PROMPT_SUBMIT}`) expect(JSON.parse(remoteFile(target, REMOTE_REPORT))).toMatchObject({ contractOk: true, submitted: true,