Repository navigation
fix(agent-prompt): observe the composer before and after Enter instead of a blind submit clock - #18022
fix(agent-prompt): observe the composer before and after Enter instead of a blind submit clock#18022Tauri-EPO wants to merge 3 commits into
Conversation
…d of a blind submit clock worker-start, dispatch --inject and terminal send --text --enter all reach writeTerminalAgentPrompt, which pasted at once, wrote Enter on a timer and then declared agent_prompt_stalled after a fixed window without ever looking at the composer. In the field that produces a paste that lands on a TUI still booting, a payload parked as `[Pasted Content N chars]` because the TUI folded Enter into the paste burst (Codex reads Windows console input records, so bracketed-paste frames never reach it), and a stall verdict while the agent is visibly working (stablyai#15581, stablyai#16680, stablyai#17066, stablyai#16660, stablyai#16902, stablyai#16377 family). - Before the paste, the prompt path waits for composer readiness with the scanner the startup draft already uses (per-agent marker or quiet window, replayed from recent output), bounded by a 20s budget. Any agent status, a known ready header or a quiet stream passes at once, so idle and busy agents are not delayed. - After the paste, the rendered screen (headless emulator or provider snapshot) is classified as pending / clear / unknown from the emulator's composer draft or the last prompt-glyph line. Enter waits for the payload to show; after Enter, a payload that stays parked gets Enter again on a 1.5s/4s/9s/18s/36s backoff, a payload that was visible and stays cleared for 500ms counts as submission proof with no hook or title needed, and the effect window grows once by 30s while the composer is pending. Only a recognized composer line or an emulator draft ever triggers a retry; an unknown screen never does, and a permission dialog still aborts. - The stall error carries the last composer verdict and the retry count; the vocabulary is unchanged. - Budgets derive from the new maximum so client, attach and fixture deadlines stay nested; the swallowed-Enter e2e now expects the second Enter to submit instead of a stall. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe prompt submission flow detects composer state and waits for composer readiness before pasting. It verifies submission using activity and composer evidence. When the payload remains pending, it retries Enter with backoff and reports retry metadata on stalls. Timing budgets cover readiness, grace, verification, and total submission duration. Runtime and end-to-end tests cover readiness, retries, stalled states, and swallowed-Enter recovery. Merge Risk: ⚪ Minimal · up to The change verifies the composer before submitting and retries only when the payload is visibly still parked; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description is complete and relevant. It covers the ELI5 summary, changes, rationale, linked issues, testing, AI disclosure, review considerations, scope notes, and checklist. It also explains that full lint, end-to-end tests, and build remain for CI.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: 9940d607-67eb-4b7e-8eea-8507ab44fc14
📒 Files selected for processing (16)
src/main/runtime/agent-prompt-composer-pending.test.tssrc/main/runtime/agent-prompt-composer-pending.tssrc/main/runtime/agent-prompt-composer-submission.test.tssrc/main/runtime/agent-prompt-submission-runtime-test-fixture.tssrc/main/runtime/agent-prompt-submission-runtime.test.tssrc/main/runtime/agent-prompt-submission-verification.test.tssrc/main/runtime/agent-prompt-submission-verification.tssrc/main/runtime/agent-prompt-submission-windows-submit-delay.test.tssrc/main/runtime/orca-runtime-controller-knows-pty-is-live.tssrc/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-07.spec.tssrc/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-08.spec.tssrc/main/runtime/orca-runtime-write-terminal-agent-prompt.tssrc/main/runtime/runtime-worktree-startup-readiness.tssrc/shared/orchestration-timing-budgets.tstests/e2e/terminal-send-agent-prompt-submit.spec.tstests/tools/repro-terminal-send-submit.mjs
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Composer verdict detector — new pure
detectAgentPromptComposerVerdict(agent-prompt-composer-pending.ts) maps a rendered screen topending/clear/unknownvia prompt glyphs, paste placeholders, a prompt-fragment match, and the emulator-extracted composer draft. - Verification loop —
verifyAgentPromptSubmissionaccepts a composer observer, re-sends Enter with backoff while the payload stays visibly parked, and resolvescomposer-clearedafter twoclearreads 500 ms apart; stalls becomeAgentPromptStalledErrorcarryingcomposerandenterRetries. - Pre-paste readiness —
writeTerminalAgentPromptwaits for composer readiness before the paste (status, ready header, quiet stream, or the shared draft-paste scanner), observes the composer before the first Enter, and re-runs the caller'sbeforeWritehook plus permission checks before every Enter. - Shared scanner refactor — the startup draft waiter was generalized into
waitForDraftPasteReadySignalso startup and prompt delivery share one implementation. - Timing budgets — new
AGENT_PROMPT_*constants, with federation/worker-start/fixture graces re-derived from the new submission maximum so the deadlines stay nested. - Tests — 15 detector cases, 7 emulator-driven submission cases, 7 observer cases, and the inverted swallowed-Enter e2e (
receivedEnters: 2,submitted: true).
The one contract change worth a deliberate look — the e2e now asserts a second Enter instead of a stall — is sound: no Enter is ever written on a clear or unknown composer, a permission dialog aborts before any retry, and the re-Enter only fires while the payload is visibly parked. The composer-cleared evidence is correctly gated on the payload having been visible before the first Enter (sawPending), so "the text was there and now the box is empty" requires both halves.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
Field data from the same day (20 fresh Codex worker-starts): two returned input_accepted while `[Pasted Content N chars]` sat in the composer for four hours, dispatch-show stayed `dispatched`, zero heartbeats. The agent was already `working` from its session hook and kept painting, so pane output after Enter passed as delivery evidence with the payload still on screen. Activity now proves submission only while the composer's last verdict is not `pending`. When it is, the composer is re-read at most once per confirm window (500 ms) so a retry that lands is noticed quickly, and a due checkpoint owns its iteration's read so the parked payload still gets its Enter. Covered by a verifier unit case and a headless-emulator runtime case that reproduces the false accept before the change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: 6a9b324d-6240-408c-a956-f56cc4dd6c78
📒 Files selected for processing (3)
src/main/runtime/agent-prompt-composer-submission.test.tssrc/main/runtime/agent-prompt-submission-verification.test.tssrc/main/runtime/agent-prompt-submission-verification.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Activity evidence gated on composer release — while the payload still reads
pending, activity no longer proves submission.activityProvesSubmission()re-reads the composer at most once per confirm window (500 ms) and only releases activity evidence once the verdict leavespending;checkpointDue/confirmDueare computed before the activity check so a due checkpoint owns its iteration's read and a parked payload still gets its Enter. - Inverse-failure coverage — the emulator suite now drives a busy agent painting around a parked payload (
[Pasted Content 4087 chars]) and asserts a second Enter clears it before acceptance; the verification suite asserts activity is rejected whilependingand accepted only after aclearread, with exact read/resubmit counts.
The gating directly closes the inverse failure from the field report: an agent already working from its session hook kept advancing pane output, so output-after-Enter previously passed as delivery evidence with the payload still parked. That evidence is now correctly suppressed until the composer visibly lets go of the payload, while hookless/slow-hook agents still resolve through the unchanged composer-cleared double-read path. Tests pass locally (agent-prompt-submission-verification + agent-prompt-composer-submission, 36 tests).
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
… the grace, honor abort after reads Review follow-ups on the composer observer: - An emulator-extracted draft only reads `pending` when it holds the paste placeholder or the prompt's first line; an unrelated draft the operator typed reads `clear`, so another Enter never submits it. - The one-time pending grace is capped at AGENT_PROMPT_VERIFICATION_MAX_MS, the ceiling the downstream budgets derive from. - The abort signal is re-checked after every awaited composer read, so a request cancelled mid-read rejects with request_aborted instead of resolving as accepted; covered by a deferred-read test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Draft now verified against the payload —
detectAgentPromptComposerVerdictroutes the emulator-extracteddraftthroughholdsInjectedPayload(paste-placeholder regex or prompt-fragment match) instead of short-circuiting topendingfor any non-empty draft. An unrelated draft now readsclear, so an operator-typed line is never mistaken for the parked payload and hit with a second Enter. This resolves the open question from the prior review ("draft returnspendingwithout verifying it is the injected payload"). - Abort honored after deferred reads —
verifyAgentPromptSubmissionnow callsthrowIfAgentPromptAbortedimmediately after each awaitedcomposer.read(), so a signal firing during the async read rejects instead of letting the loop return stale evidence. - Grace deadline capped — the one-time pending-composer grace is now bounded to
startedAt + AGENT_PROMPT_VERIFICATION_MAX_MS, keeping the downstream federation/worker-start/fixture budgets nested even with a customtimeoutMsorpendingGraceMs. - Tests — new detector case (
draftholding the payload →pending, unrelateddraft→clear) and a verification case assertingrequest_abortedwhen the signal fires mid-read; both fail against the prior code.
The three changes each tighten an already-reviewed behavior rather than expanding scope, and the grace cap preserves the deadline-nesting invariant verified earlier.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
|
Closing: #16904 (merged 2026-09-06, This PR re-read the composer after Enter and re-sent Enter while the payload was still pending. Upstream chose never to resend: The one piece upstream does not have is the pre-paste wait for the composer to be ready. #18019 covers the Codex boot-header half of that. If the early paste still reproduces on a build carrying #16904, I will open a separate issue with the repro rather than reviving this branch. |
1 participant
ELI5
When Orca hands a task to an agent terminal it pastes the text and presses Enter. Until now it pressed Enter on a timer and then watched the agent for signs of life, without ever looking at what was on screen. If the agent was still booting, or had swallowed the Enter into the paste (Codex on Windows does this because it reads console key events, not paste frames), the task sat in the input box and Orca declared a stall after a fixed window — or, worse, declared success. Now Orca reads the rendered screen: it waits for the composer to exist before pasting, checks that the pasted text is visible before pressing Enter, presses Enter again (with backoff) while the text is visibly still parked, and treats "the text was there and now the box is empty" as proof the agent took it.
What Changed
Verification —
src/main/runtime/agent-prompt-submission-verification.tsverifyAgentPromptSubmissiontakes an optional composer observer (beforeSubmitverdict,read(),resubmit()). At1.5s / 4s / 9s / 18s / 36safter Enter it re-reads the composer: a payload stillpendinggets one more Enter and, once, extends the effect window byAGENT_PROMPT_PENDING_COMPOSER_GRACE_MS(30 s); a payload that was seen pending and readscleartwice 500 ms apart resolves ascomposer-clearedevidence.clearorunknownnever triggers an Enter. Activity, hook and output evidence keep working as before, with one exception: while the composer's last verdict ispending, activity does not prove submission. The composer is then re-read at most once per 500 ms so a retry that lands is noticed, and a due checkpoint owns its iteration's read so the parked payload still gets its Enter.{ evidence, enterRetries }; the stall is nowAgentPromptStalledErrorcarryingcomposerandenterRetries. Message andisAgentPromptStalledErrorare unchanged.Composer verdict —
src/main/runtime/agent-prompt-composer-pending.ts(new, pure)detectAgentPromptComposerVerdict(screen, prompt):pendingwhen the emulator extracted a non-empty composer draft, or when the last prompt-glyph line (❯ › » >) in the bottom of the frame holds a paste placeholder ([Pasted Content N chars],[Pasted text #1 +N lines],[Pasted: N lines],[N lines]) or the prompt's first line;clearwhen that line is empty or unrelated;unknownwhen there is no rendered screen or no recognizable composer line. A bare>only counts when spaced, so Codex's own>_ OpenAI Codexbanner is not a composer.Runtime —
src/main/runtime/orca-runtime-write-terminal-agent-prompt.ts,runtime-worktree-startup-readiness.ts,orca-runtime-controller-knows-pty-is-live.tswaitForAgentPromptComposerReadyreturns at once on any agent status, a known ready header, or a stream quiet for 3 s; otherwise it waits for the agent's composer signal through the samecreateDraftPasteReadyScannerthe startup draft uses (replayed from recent output plus live bytes), bounded bymax(20 s, agent draftPasteReadyTimeoutMs), then pastes anyway with a warning. The startup-draft waiter was generalized intowaitForDraftPasteReadySignalso both paths share one implementation.readVisibleTerminalState: headless emulator or provider snapshot) and, if the composer is readable but empty, polled up to 1.5 s for the payload to appear before the first Enter.beforeWritehook and re-checks that no permission dialog took the pane.sendTerminalAgentPromptgains an optionalcomposerReadyTimeoutMs.Budgets —
src/shared/orchestration-timing-budgets.ts:AGENT_PROMPT_VERIFICATION_MAX_MS,AGENT_PROMPT_COMPOSER_READY_TIMEOUT_MSandAGENT_PROMPT_SUBMISSION_MAX_MS; the federation-attach, worker-start-client and swallowed-Enter fixture graces derive from the new maximum so the deadlines stay nested.Tests — new
agent-prompt-composer-pending.test.ts(15) andagent-prompt-composer-submission.test.ts(8, driving the real headless emulator with Codex-shaped frames: parked payload gets a second Enter then accepts the turn start; a busy agent painting around a parked payload is not accepted until a second Enter clears it; payload visible then cleared is accepted with no hook; parked payload is retried 5× then stalled withcomposer: 'pending'; empty composer stalls after exactly one Enter; paste is held until a booting TUI shows its composer; paste proceeds after the readiness budget; an agent with a status is pasted at once). Verification suite gains 8 observer cases. Existing prompt-timing fixtures seed a ready header so they keep measuring the submit delay. The e2eCLI reports a swallowed Enter without submitting a second EnterbecomesCLI re-sends Enter when the composer still shows the payload after the first one(receivedEnters: 2,submitted: true).Why
Measured on 2026-09-01 (Orca 1.4.194, Windows 11, 16 fresh
worker-start --agent codex): 3 stalls out of 16. Two of them showed[Pasted Content 5033 chars]sitting in the Codex composer with the model already loaded; a manualterminal send --text "" --entersubmitted it and the agent worked, but the dispatch was alreadyfailed / agent_prompt_stalled. The same shape is reported for Codex on macOS (#17066, #15581), Grok ([Pasted: 91 lines], #16902), Hermes and opencode (#16660), and large Codex pastes on Windows (#11343). A second measurement the same day (20 fresh Codexworker-starts, same host) added the inverse failure: two dispatches came backstage=input_acceptedwhile[Pasted Content 4087 chars]and[Pasted Content 4994 chars]sat in the composer for four hours,dispatch-showstayeddispatchedwith zero heartbeats, and one manualterminal send --text "" --entersubmitted each at once. Those agents were alreadyworkingfrom their session hook, so pane output after Enter passed as delivery evidence with the payload still on screen —input_acceptedwas true of the PTY, not of the agent. The common cause is thatwriteTerminalAgentPromptnever observed the composer: Enter went out on a timer, and the verifier accepted only activity evidence, so a hookless or slow-hook agent (Hermes, opencode, Codex's ~20 s first hook) could not prove submission even when it had consumed the prompt. #16590 and #16300 widened the window and stopped revoking; this PR gives the verifier the evidence it was missing and makes the one recovery every report ended with — a second Enter — automatic, but only under the condition that made it safe by hand: the payload is visibly still parked and no dialog is up.The design is vendor-neutral: the readiness signal comes from
TUI_AGENT_CONFIG.draftPasteReadySignal(marker or quiet window), and the composer verdict keys on prompt glyphs and paste placeholders rather than on any one agent. TUIs whose composer this detector does not recognize getunknownand exactly today's behavior.Companion PRs: #18019 (Codex
model: loadingheader is not tui-idle), #18020 (terminal createinferslaunchAgent, #17291), #18021 (dispatch --injectkeeps the dispatch on a stall). Each is independent; together they cover the three symptoms in the field report.Linked Issue
Refs #17066, #16660, #16902, #15581, #16377, #11343, #15976, #16680
Visual Proof
N/A — no UI change. Behavior is visible in the CLI: a
worker-startwhose Codex composer swallowed the first Enter now endsstage=input_acceptedafter the automatic second Enter instead offailed / agent_prompt_stalled.Testing
Not exercised against a live Codex in this session, and the Electron e2e (
tests/e2e/terminal-send-agent-prompt-submit.spec.ts) was updated but not run locally; the integration tests drive the real@xterm/headlessemulator with Codex-shaped frames instead. Ran on Windows 11:AI Disclosure
Written with Claude Code (Claude Fable 5.1) under direction of @Tauri-EPO, who supplied the field measurements, chose the observation-based design over a longer timer, and reviewed the diff.
Review
The one contract change worth a deliberate look: the e2e that asserted "no second Enter after a swallowed one" now asserts the opposite. The safety property behind the old rule is kept — no Enter is ever sent on an
unknownorclearcomposer, and a permission dialog aborts — but a maintainer should confirm that "payload visibly parked" is an acceptable trigger.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
readVisibleTerminalStatealready falls back to provider snapshots; an unreadable screen yieldsunknownand no retry.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred) —tc:node, the suites above, ratchet and gate checks pass locally; full lint, e2e and build left to CI🤖 Generated with Claude Code