diff --git a/src/main/runtime/agent-prompt-submission-runtime.test.ts b/src/main/runtime/agent-prompt-submission-runtime.test.ts index e5bf5892e58..44f61708751 100644 --- a/src/main/runtime/agent-prompt-submission-runtime.test.ts +++ b/src/main/runtime/agent-prompt-submission-runtime.test.ts @@ -143,6 +143,69 @@ describe('agent prompt submission runtime', () => { expect(writes).toEqual([]) }) + it('does not paste into an output-only permission prompt', async () => { + vi.useFakeTimers() + vi.setSystemTime(1_000) + const { runtime, handle, writes } = await createPromptRuntime(() => undefined) + runtime.onPtyData('pty-prompt', '\x1b]0;Codex idle\x07', Date.now()) + vi.setSystemTime(2_000) + runtime.onPtyData( + 'pty-prompt', + 'Permission required\nAllow once\nAllow always\nReject\n', + Date.now() + ) + + await expect(runtime.sendTerminalAgentPrompt(handle, 'review this')).rejects.toThrow( + 'agent_prompt_blocked' + ) + expect(writes).toEqual([]) + }) + + it('does not send Enter after output-only permission appears during settlement', async () => { + vi.useFakeTimers() + vi.setSystemTime(1_000) + const { runtime, handle, writes } = await createPromptRuntime((runtime, data) => { + if (data.includes(AGENT_PROMPT_BRACKETED_PASTE_END)) { + vi.setSystemTime(2_000) + runtime.onPtyData( + 'pty-prompt', + 'Permission required\nAllow once\nAllow always\nReject\n', + Date.now() + ) + } + }) + runtime.onPtyData('pty-prompt', '\x1b]0;Codex idle\x07', Date.now()) + const submission = runtime.sendTerminalAgentPrompt(handle, 'review this') + const rejected = expect(submission).rejects.toThrow('agent_prompt_blocked') + + await vi.runAllTimersAsync() + + await rejected + expect(writes).not.toContain('\r') + }) + + it.each([ + '\x1b]0;Codex waiting for permission\x07\x1b]0;Codex idle\x07', + '\x1b]9999;{"state":"working","agentType":"aider"}\x07' + + '\x1b]0;Codex waiting for permission\x07', + '\x1b]0;Codex waiting for permission\x07' + + '\x1b]9999;{"state":"working","agentType":"aider"}\x07' + ])('does not send Enter after coalesced permission activity', async (output) => { + vi.useFakeTimers() + const { runtime, handle, writes } = await createPromptRuntime((runtime, data) => { + if (data.includes(AGENT_PROMPT_BRACKETED_PASTE_END)) { + runtime.onPtyData('pty-prompt', output, Date.now()) + } + }) + const submission = runtime.sendTerminalAgentPrompt(handle, 'review this') + const rejected = expect(submission).rejects.toThrow('agent_prompt_blocked') + + await vi.runAllTimersAsync() + + await rejected + expect(writes).not.toContain('\r') + }) + it('stops a chunked paste when permission appears between chunks', async () => { const { runtime, handle, writes } = await createPromptRuntime(() => undefined) let writeChecks = 0 @@ -197,7 +260,7 @@ describe('agent prompt submission runtime', () => { expect(writes.filter((data) => data === '\r')).toHaveLength(1) }) - it('prefers a newer explicit working status over an older permission title', async () => { + it('does not treat an unchanged newer working status as submission evidence', async () => { vi.useFakeTimers() vi.setSystemTime(1_000) const { runtime, handle, writes } = await createPromptRuntime((runtime, data) => { @@ -218,9 +281,10 @@ describe('agent prompt submission runtime', () => { ) const submission = runtime.sendTerminalAgentPrompt(handle, 'review this') + const rejected = expect(submission).rejects.toThrow('agent_prompt_stalled') await vi.runAllTimersAsync() - await expect(submission).resolves.toMatchObject({ accepted: true }) + await rejected expect(writes.filter((data) => data === '\r')).toHaveLength(1) }) @@ -293,9 +357,12 @@ describe('agent prompt submission runtime', () => { it('serializes concurrent prompt submissions within one PTY generation', async () => { vi.useFakeTimers() + let enterCount = 0 const { runtime, handle, writes } = await createPromptRuntime((runtime, data) => { if (data === '\r') { + enterCount += 1 runtime.onPtyData('pty-prompt', '\x1b]0;Codex working\x07', Date.now()) + runtime.onPtyData('pty-prompt', '\x1b]0;Codex idle\x07', Date.now()) } }) @@ -312,6 +379,7 @@ describe('agent prompt submission runtime', () => { expect(firstEnter).toBeGreaterThan(firstPaste) expect(secondPaste).toBeGreaterThan(firstEnter) expect(secondEnter).toBeGreaterThan(secondPaste) + expect(enterCount).toBe(2) }) it('does not queue a replacement generation behind an obsolete submission', async () => { diff --git a/src/main/runtime/agent-prompt-submission-verification.test.ts b/src/main/runtime/agent-prompt-submission-verification.test.ts index 0b93e047d20..a0f3e1a5213 100644 --- a/src/main/runtime/agent-prompt-submission-verification.test.ts +++ b/src/main/runtime/agent-prompt-submission-verification.test.ts @@ -8,6 +8,7 @@ import { function activity(overrides: Partial = {}): AgentPromptActivity { return { generation: 1, + permissionSequence: 2, workingSequence: 4, status: 'idle', ...overrides @@ -89,6 +90,21 @@ describe('agent prompt submission verification', () => { await rejected }) + it('blocks when permission appears and clears between polls', async () => { + vi.useFakeTimers() + let current = activity() + const verification = verifyAgentPromptSubmission({ + baseline: current, + readActivity: () => current + }) + const rejected = expect(verification).rejects.toThrow('agent_prompt_blocked') + + current = activity({ permissionSequence: 3 }) + await vi.advanceTimersByTimeAsync(50) + + await rejected + }) + it('rejects an existing permission state', async () => { const current = activity({ status: 'permission' }) @@ -97,12 +113,18 @@ describe('agent prompt submission verification', () => { ).rejects.toThrow('agent_prompt_blocked') }) - it('does not wait when the agent is already working', async () => { + it('does not accept an unchanged working baseline', async () => { + vi.useFakeTimers() const current = activity({ status: 'working' }) + const verification = verifyAgentPromptSubmission({ + baseline: current, + readActivity: () => current + }) + const rejected = expect(verification).rejects.toThrow('agent_prompt_stalled') - await expect( - verifyAgentPromptSubmission({ baseline: current, readActivity: () => current }) - ).resolves.toBeUndefined() + await vi.advanceTimersByTimeAsync(AGENT_PROMPT_EFFECT_TIMEOUT_MS) + + await rejected }) it('rejects a replaced terminal generation', async () => { diff --git a/src/main/runtime/agent-prompt-submission-verification.ts b/src/main/runtime/agent-prompt-submission-verification.ts index dc7f4864800..41b5f5b1732 100644 --- a/src/main/runtime/agent-prompt-submission-verification.ts +++ b/src/main/runtime/agent-prompt-submission-verification.ts @@ -3,6 +3,7 @@ const AGENT_PROMPT_EFFECT_POLL_MS = 50 export type AgentPromptActivity = Readonly<{ generation: number + permissionSequence: number workingSequence: number status: 'working' | 'permission' | 'idle' | null }> @@ -17,16 +18,13 @@ export async function verifyAgentPromptSubmission( options: AgentPromptVerificationOptions ): Promise { throwIfAgentPromptAborted(options.signal) - assertPromptNotBlocked(options.baseline) - if (options.baseline.status === 'working') { - return - } + assertPromptNotBlocked(options.baseline, options.baseline) const deadline = Date.now() + AGENT_PROMPT_EFFECT_TIMEOUT_MS while (Date.now() < deadline) { const current = options.readActivity() assertSamePromptGeneration(options.baseline, current) - assertPromptNotBlocked(current) + assertPromptNotBlocked(options.baseline, current) if (agentPromptLifecycleChanged(options.baseline, current)) { return } @@ -35,7 +33,7 @@ export async function verifyAgentPromptSubmission( const current = options.readActivity() assertSamePromptGeneration(options.baseline, current) - assertPromptNotBlocked(current) + assertPromptNotBlocked(options.baseline, current) if (agentPromptLifecycleChanged(options.baseline, current)) { return } @@ -46,10 +44,7 @@ function agentPromptLifecycleChanged( baseline: AgentPromptActivity, current: AgentPromptActivity ): boolean { - if (current.status === 'working') { - return true - } - return current.status === 'idle' && current.workingSequence > baseline.workingSequence + return current.workingSequence > baseline.workingSequence } function assertSamePromptGeneration( @@ -61,8 +56,8 @@ function assertSamePromptGeneration( } } -function assertPromptNotBlocked(activity: AgentPromptActivity): void { - if (activity.status === 'permission') { +function assertPromptNotBlocked(baseline: AgentPromptActivity, current: AgentPromptActivity): void { + if (current.status === 'permission' || current.permissionSequence > baseline.permissionSequence) { throw new Error('agent_prompt_blocked') } } diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 3b42483e4f3..1c3a7e082d8 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -1485,6 +1485,14 @@ type RuntimePtyWorktreeRecord = { tailWaitState?: TerminalTailWaitState } +type TerminalAgentStatusSnapshot = { + waitText: string + waitBlockedAt: number | null + title: string | null + titleStatus: AgentStatus | null + titleStatusIsLive: boolean +} + type TerminalCreateOptions = { command?: string claudeAgentTeamsSourceCommand?: string @@ -3198,6 +3206,7 @@ export class OrcaRuntimeService { string, { status: AgentStatus | null; workingSequence: number; updatedAt: number } >() + private agentPromptPermissionSequenceByPtyId = new Map() private agentPromptExplicitStatusFloorByPtyId = new Map() private agentPromptSubmissionTailByPtyId = new Map>() private providerSequenceInitializedPtys = new Set() @@ -11071,6 +11080,7 @@ export class OrcaRuntimeService { } if (tailGainedNewerBlockedReason(previousWaitState, nextWaitState, state.appended)) { pty.waitBlockedAt = at + this.recordAgentPromptPermissionObservation(ptyId) } state.lastAt = at state.lastWaitState = nextWaitState @@ -11654,6 +11664,7 @@ export class OrcaRuntimeService { this.osc7ScanTailByPtyId.delete(ptyId) this.agentStatusOscProcessorsByPtyId.delete(ptyId) this.agentPromptLifecycleByPtyId.delete(ptyId) + this.agentPromptPermissionSequenceByPtyId.delete(ptyId) const pty = this.ptysById.get(ptyId) if (pty) { pty.lastOscTitle = null @@ -11900,6 +11911,9 @@ export class OrcaRuntimeService { } private recordAgentPromptLifecycleState(ptyId: string, status: AgentStatus | null): void { + if (status === 'permission') { + this.recordAgentPromptPermissionObservation(ptyId) + } const current = this.agentPromptLifecycleByPtyId.get(ptyId) const updatedAt = Date.now() if (!current) { @@ -11918,6 +11932,13 @@ export class OrcaRuntimeService { }) } + private recordAgentPromptPermissionObservation(ptyId: string): void { + this.agentPromptPermissionSequenceByPtyId.set( + ptyId, + (this.agentPromptPermissionSequenceByPtyId.get(ptyId) ?? 0) + 1 + ) + } + private getPtyLifecycleGeneration(ptyId: string): number { const existing = this.ptyLifecycleGenerationById.get(ptyId) if (existing !== undefined) { @@ -11931,6 +11952,7 @@ export class OrcaRuntimeService { private advancePtyLifecycleGeneration(ptyId: string): void { this.ptyLifecycleGenerationById.set(ptyId, this.nextPtyLifecycleGeneration++) this.agentPromptLifecycleByPtyId.delete(ptyId) + this.agentPromptPermissionSequenceByPtyId.delete(ptyId) this.agentPromptExplicitStatusFloorByPtyId.set(ptyId, Date.now()) this.legacyWorkerRecoveredPtys.delete(ptyId) // Why: a respawn under the same session id needs its own subscriber-driven attach. @@ -17997,21 +18019,10 @@ export class OrcaRuntimeService { const ptyId = this.getTerminalAgentStatusPtyId(handle) const terminal = this.getTerminalAgentStatusSnapshot(handle, ptyId) const explicitStatus = this.getFreshExplicitAgentStatusForHandle(handle) - const blockedByWaitText = detectTerminalWaitBlockedReason(terminal.waitText) - const liveTitleClearsBlockedText = - terminal.titleStatusIsLive && - terminal.titleStatus !== null && - terminal.titleStatus !== 'permission' && - !isOpenCodeNativeTitle(terminal.title) - if (terminal.titleStatus === 'permission' && terminal.titleStatusIsLive) { - return { handle, isRunningAgent: true, status: 'permission' } - } + const lifecycle = this.agentPromptLifecycleByPtyId.get(ptyId) if ( - blockedByWaitText && - !liveTitleClearsBlockedText && - (!explicitStatus || - explicitStatus.status === 'permission' || - (terminal.waitBlockedAt !== null && terminal.waitBlockedAt >= explicitStatus.updatedAt)) + (terminal.titleStatus === 'permission' && terminal.titleStatusIsLive) || + this.hasAuthoritativeTerminalWaitPermission(terminal, explicitStatus, lifecycle) ) { return { handle, isRunningAgent: true, status: 'permission' } } @@ -18081,13 +18092,7 @@ export class OrcaRuntimeService { private getTerminalAgentStatusSnapshot( handle: string, expectedPtyId: string - ): { - waitText: string - waitBlockedAt: number | null - title: string | null - titleStatus: AgentStatus | null - titleStatusIsLive: boolean - } { + ): TerminalAgentStatusSnapshot { const pty = this.getLivePtyForHandle(handle) if (pty) { if (!pty.pty.connected || pty.pty.ptyId !== expectedPtyId) { @@ -18146,6 +18151,33 @@ export class OrcaRuntimeService { } } + private hasAuthoritativeTerminalWaitPermission( + terminal: TerminalAgentStatusSnapshot, + explicitStatus: { status: AgentStatus; updatedAt: number } | null, + lifecycle: { status: AgentStatus | null; updatedAt: number } | null | undefined + ): boolean { + const blockedByWaitText = detectTerminalWaitBlockedReason(terminal.waitText) + if (!blockedByWaitText) { + return false + } + if (explicitStatus?.status === 'permission' || lifecycle?.status === 'permission') { + return true + } + const newestClearAt = Math.max( + explicitStatus?.updatedAt ?? -1, + lifecycle?.status ? lifecycle.updatedAt : -1 + ) + if (terminal.waitBlockedAt !== null) { + return terminal.waitBlockedAt >= newestClearAt + } + const liveTitleClearsBlockedText = + terminal.titleStatusIsLive && + terminal.titleStatus !== null && + terminal.titleStatus !== 'permission' && + !isOpenCodeNativeTitle(terminal.title) + return !liveTitleClearsBlockedText && !explicitStatus + } + private async terminalHasShellForegroundProcess(handle: string, ptyId: string): Promise { if (!this.ptyController) { return false @@ -18557,9 +18589,8 @@ export class OrcaRuntimeService { ): Promise { assertAgentPromptRequestActive(options.signal) this.assertAgentPromptGeneration(ptyId, generation) - if (this.getAgentPromptActivity(handle, ptyId).status === 'permission') { - throw new Error('agent_prompt_blocked') - } + const permissionBaseline = this.getAgentPromptActivity(handle, ptyId) + this.assertAgentPromptPermissionSafe(permissionBaseline, permissionBaseline) const renderGate = this.createAgentPromptRenderGate(ptyId) let wrotePasteBytes = false let completedPaste = false @@ -18573,9 +18604,10 @@ export class OrcaRuntimeService { await options.beforeWrite?.(ptyId) assertAgentPromptRequestActive(options.signal) this.assertAgentPromptGeneration(ptyId, generation) - if (this.getAgentPromptActivity(handle, ptyId).status === 'permission') { - throw new Error('agent_prompt_blocked') - } + this.assertAgentPromptPermissionSafe( + permissionBaseline, + this.getAgentPromptActivity(handle, ptyId) + ) if (nextChunk.done) { renderGate?.arm() } @@ -18624,9 +18656,7 @@ export class OrcaRuntimeService { assertAgentPromptRequestActive(options.signal) this.assertAgentPromptGeneration(ptyId, generation) const baseline = this.getAgentPromptActivity(handle, ptyId) - if (baseline.status === 'permission') { - throw new Error('agent_prompt_blocked') - } + this.assertAgentPromptPermissionSafe(permissionBaseline, baseline) const suffixWrote = this.ptyController?.write(ptyId, AGENT_PROMPT_SUBMIT) ?? false if (!suffixWrote) { throw new Error(options.suffixFailureError ?? 'terminal_not_writable') @@ -18680,14 +18710,32 @@ export class OrcaRuntimeService { (!explicit || lifecycle.updatedAt > explicit.updatedAt || (lifecycle.updatedAt === explicit.updatedAt && lifecycle.status === 'permission')) - const status = lifecycleIsNewer ? lifecycle.status : (explicit?.status ?? ptyStatus ?? null) + const terminal = this.getTerminalAgentStatusSnapshot(handle, ptyId) + const status = this.hasAuthoritativeTerminalWaitPermission(terminal, explicit, lifecycle) + ? 'permission' + : lifecycleIsNewer + ? lifecycle.status + : (explicit?.status ?? ptyStatus ?? null) return { generation: this.getPtyLifecycleGeneration(ptyId), + permissionSequence: this.agentPromptPermissionSequenceByPtyId.get(ptyId) ?? 0, workingSequence: lifecycle?.workingSequence ?? 0, status } } + private assertAgentPromptPermissionSafe( + baseline: AgentPromptActivity, + current: AgentPromptActivity + ): void { + if ( + current.status === 'permission' || + current.permissionSequence > baseline.permissionSequence + ) { + throw new Error('agent_prompt_blocked') + } + } + private assertAgentPromptGeneration(ptyId: string, expected: number): void { if (this.getPtyLifecycleGeneration(ptyId) !== expected) { throw new Error('terminal_handle_stale') diff --git a/tests/e2e/terminal-send-agent-prompt-submit.spec.ts b/tests/e2e/terminal-send-agent-prompt-submit.spec.ts index 5a1893da9d1..1ae525d29da 100644 --- a/tests/e2e/terminal-send-agent-prompt-submit.spec.ts +++ b/tests/e2e/terminal-send-agent-prompt-submit.spec.ts @@ -140,3 +140,52 @@ test('CLI reports a swallowed Enter without submitting a second Enter', async ({ markerReceived: true }) }) + +test('CLI does not write prompt bytes into an active permission dialog', async ({ + electronApp, + orcaPage, + testRepoPath +}) => { + test.setTimeout(90_000) + await waitForSessionReady(orcaPage) + const userDataDir = await electronApp.evaluate(({ app }) => app.getPath('userData')) + const repoRoot = process.cwd() + let stdout = '' + try { + const result = await execFileAsync( + process.execPath, + [ + path.join(repoRoot, 'tests', 'tools', 'repro-terminal-send-submit.mjs'), + '--cli', + path.join(repoRoot, 'config', 'scripts', 'orca-dev.mjs'), + '--worktree', + testRepoPath, + '--expect-blocked', + '--report', + fixtureReport, + '--marker', + fixtureMarker, + '--discard-report' + ], + { + cwd: repoRoot, + env: { ...process.env, ORCA_DEV_USER_DATA_PATH: userDataDir }, + timeout: 60_000 + } + ) + stdout = result.stdout + } catch (error) { + const failed = error as Error & { stdout?: string; stderr?: string } + throw new Error([failed.message, failed.stdout, failed.stderr].filter(Boolean).join('\n')) + } + + expect(JSON.parse(stdout)).toMatchObject({ + rescueSent: false, + sendErrorCode: 'agent_prompt_blocked', + contractOk: true, + submitted: false, + receivedBytes: 0, + receivedEnters: 0, + markerReceived: false + }) +}) diff --git a/tests/tools/repro-terminal-send-submit.mjs b/tests/tools/repro-terminal-send-submit.mjs index be4c1fdee37..201e51c9364 100644 --- a/tests/tools/repro-terminal-send-submit.mjs +++ b/tests/tools/repro-terminal-send-submit.mjs @@ -161,7 +161,9 @@ async function parentMain() { const marker = argValue('marker', `ORCA_TERMINAL_SEND_${process.pid}_${Date.now()}`) const prompt = `${marker} ${'slow composer payload '.repeat(24)}` const expectStalled = hasFlag('expect-stalled') + const expectBlocked = hasFlag('expect-blocked') await mkdir(tempDir, { recursive: true }) + await rm(reportPath, { force: true }) const command = argValue('agent-command') ?? @@ -173,6 +175,7 @@ async function parentMain() { shellQuote(marker), '--timeout-ms', String(timeoutMs), + ...(expectBlocked ? ['--permission-before-send'] : []), ...(process.platform === 'win32' ? ['--allow-unframed-paste'] : []) ])) const added = await callOrca(cli, ['repo', 'add', '--path', cwd], cwd) @@ -201,11 +204,18 @@ async function parentMain() { } try { - await callOrca( - cli, - ['terminal', 'wait', '--terminal', handle, '--for', 'tui-idle', '--timeout-ms', '10000'], - cwd - ) + if (expectBlocked) { + const setupReport = await readReport(reportPath, 10_000) + if (!setupReport) { + throw new Error('terminal permission prompt did not materialize') + } + } else { + await callOrca( + cli, + ['terminal', 'wait', '--terminal', handle, '--for', 'tui-idle', '--timeout-ms', '10000'], + cwd + ) + } let sendErrorCode = null try { await callOrca( @@ -214,14 +224,17 @@ async function parentMain() { cwd ) } catch (error) { - if (!expectStalled || error?.code !== 'agent_prompt_stalled') { + const expectedError = + (expectStalled && error?.code === 'agent_prompt_stalled') || + (expectBlocked && error?.code === 'agent_prompt_blocked') + if (!expectedError) { throw error } sendErrorCode = error.code } let report = await readReport(reportPath, 1_000) let rescueSent = false - if (!report && !expectStalled) { + if (!report && !expectStalled && !expectBlocked) { rescueSent = true await callOrca(cli, ['terminal', 'send', '--terminal', handle, '--enter'], cwd) report = await readReport(reportPath, timeoutMs) @@ -242,7 +255,16 @@ async function parentMain() { report.submitted === false && report.receivedEnters === 1 && report.swallowedEnters === 1 - if (!report.contractOk || rescueSent || (expectStalled && !expectedStallObserved)) { + const expectedBlockObserved = + sendErrorCode === 'agent_prompt_blocked' && + report.receivedBytes === 0 && + report.receivedEnters === 0 + if ( + !report.contractOk || + rescueSent || + (expectStalled && !expectedStallObserved) || + (expectBlocked && !expectedBlockObserved) + ) { process.exitCode = 1 } } finally { @@ -261,6 +283,7 @@ async function fakeAgentMain() { const timeoutMs = parsePositiveInteger('timeout-ms', DEFAULT_TIMEOUT_MS) const pasteFramingRequired = !hasFlag('allow-unframed-paste') const swallowFirstEnter = hasFlag('swallow-first-enter') + const permissionBeforeSend = hasFlag('permission-before-send') if (!reportPath || !marker) { throw new Error('--fake-agent requires --report and --marker') } @@ -269,6 +292,9 @@ async function fakeAgentMain() { } process.stdin.resume() process.stdout.write('OpenAI Codex\nmodel: fake\ndirectory: fixture\n> ') + if (permissionBeforeSend) { + process.stdout.write('\nPermission required\nAllow once\nAllow always\nReject\n') + } let input = '' let countedCarriages = 0 @@ -282,7 +308,9 @@ async function fakeAgentMain() { const writeReport = async (submitted) => { const hasBracketedPasteFrame = input.includes(BEGIN) && input.includes(END) const report = { - contractOk: prematureEnters === 0 && (!pasteFramingRequired || hasBracketedPasteFrame), + contractOk: + prematureEnters === 0 && + (permissionBeforeSend || !pasteFramingRequired || hasBracketedPasteFrame), submitted, prematureEnters, receivedEnters, @@ -307,6 +335,9 @@ async function fakeAgentMain() { } const timeout = setTimeout(() => process.exit(8), timeoutMs) + if (permissionBeforeSend) { + setTimeout(() => void writeReport(false), 250) + } process.stdin.on('data', (chunk) => { input += chunk.toString('utf8') if (!renderScheduled && input.includes(marker)) {