Repository navigation
fix(terminal): refuse to submit a CLI prompt over an unsent composer draft - #16290
luisgonzaleznf wants to merge 17 commits into
Conversation
…draft `orca terminal send --text ... --enter` into a Claude Code or Codex session pastes at the composer caret and presses Enter without looking at the composer. When the user has a half-typed draft there, the draft and the agent's message are submitted as one glued prompt. The host runtime now reads the rendered composer (the same emulator state `terminal read --screen` returns) inside the serialized agent-prompt submission, before any byte is written. Typed text left of the caret, or undimmed text right of it (both agents draw their placeholder dim), counts as a draft and the send is refused with `accepted: false`, `refusedReason: 'pending-input'` and the draft in `pendingInput`. An unreadable screen never blocks a send, and native ConPTY panes skip the style signal because SGR-dim fidelity there is unverified. `--force` (RPC `allowPendingInput`) opts back into appending. Orchestration dispatch/inject/worker preambles share the guard and fail with `agent_prompt_pending_input` instead of corrupting the draft. Fixes stablyai#16289 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthrough
Priority: ➖ Normal 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c5383d9-5272-4650-a138-30ec404d6f37
📒 Files selected for processing (19)
skill-guides/orca-cli.mdsrc/cli/bundled-skill-guides.tssrc/cli/handlers/terminal.test.tssrc/cli/handlers/terminal.tssrc/cli/help.tssrc/cli/specs/core.tssrc/cli/terminal-format.test.tssrc/cli/terminal-format.tssrc/main/daemon/headless-emulator.test.tssrc/main/daemon/headless-emulator.tssrc/main/daemon/terminal-cursor-line-context.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/errors.test.tssrc/main/runtime/rpc/errors.tssrc/main/runtime/rpc/methods/terminal.tssrc/main/runtime/rpc/terminal-send-pending-draft.integration.test.tssrc/shared/agent-composer-pending-input.test.tssrc/shared/agent-composer-pending-input.tssrc/shared/runtime-types.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
…g drafts Run the composer check after the permission check so a trust/permission dialog whose selected option also starts with `❯` still yields agent_prompt_blocked, and never treat a numbered option row as the composer. Cross blank rows so multi-paragraph drafts count, stop at transcript markers instead, and drop dim placeholder cells from every row, not just right of the caret. Prefer the detected foreground agent over the launch agent after an agent swap, and retry a provider frame discarded by the user's own keystroke echo before falling open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 84200b5b-3732-4292-a7b6-d0e28e2b88e2
📒 Files selected for processing (6)
src/main/daemon/headless-emulator.test.tssrc/main/daemon/terminal-cursor-line-context.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/terminal-send-pending-draft.integration.test.tssrc/shared/agent-composer-pending-input.test.tssrc/shared/agent-composer-pending-input.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
Re-apply the terminal.send refusal mapping and allowPendingInput param onto the split rpc/methods/terminal/* modules, and the RuntimeTerminalSend fields onto runtime-terminal-contracts.ts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A draft may legitimately start with `1.` or continue with a bullet, so a numbered glyph row is a dialog only when a numbered option row sits directly under it (the cursor context now carries two rows below), and a row ends the draft only when its content starts at column 0 — both agents indent every continuation row. The proceeds-cases in the integration test now assert the accepted response and the pasted payload. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Regenerate the bundled skill guides from the merged orca-cli.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 87cd84d2-6e85-4e0d-8dae-9b74bcf14d18
📒 Files selected for processing (20)
skill-guides/orca-cli.mdsrc/cli/bundled-skill-guides.tssrc/cli/handlers/terminal.test.tssrc/cli/handlers/terminal.tssrc/cli/help.tssrc/cli/specs/core.tssrc/cli/terminal-format.test.tssrc/cli/terminal-format.tssrc/main/daemon/headless-emulator.test.tssrc/main/daemon/headless-emulator.tssrc/main/daemon/terminal-cursor-line-context.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/errors.test.tssrc/main/runtime/rpc/errors.tssrc/main/runtime/rpc/methods/terminal/terminal-send-method.tssrc/main/runtime/rpc/methods/terminal/unary-schemas.tssrc/main/runtime/rpc/terminal-send-pending-draft.integration.test.tssrc/shared/agent-composer-pending-input.test.tssrc/shared/agent-composer-pending-input.tssrc/shared/runtime-terminal-contracts.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- src/cli/specs/core.ts
- src/main/runtime/rpc/errors.test.ts
- src/cli/handlers/terminal.ts
- src/main/runtime/rpc/methods/terminal/terminal-send-method.ts
- src/main/runtime/rpc/methods/terminal/unary-schemas.ts
- src/cli/handlers/terminal.test.ts
- src/cli/terminal-format.test.ts
- src/main/runtime/rpc/errors.ts
- src/shared/runtime-terminal-contracts.ts
- src/cli/help.ts
- src/cli/bundled-skill-guides.ts
- src/cli/terminal-format.ts
- src/main/daemon/headless-emulator.ts
- src/main/daemon/headless-emulator.test.ts
- skill-guides/orca-cli.md
- src/main/daemon/terminal-cursor-line-context.ts
- src/shared/agent-composer-pending-input.test.ts
- src/main/runtime/orca-runtime.ts
- src/main/runtime/rpc/terminal-send-pending-draft.integration.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
…ow text A numbered-list draft (`❯ 1. First step` / ` 2. Second step`) has the same row shape as a trust or permission dialog, so any text rule either misses that draft or misreads the dialog. Captured from Claude Code 2.1.246 and Codex 0.149.1: both agents show the terminal cursor whenever the composer has a caret and hide it (DECTCEM) while a dialog is open — evidence a draft cannot fake. The detector now answers null on a hidden cursor and drops the option-list heuristic and the rows-below lookahead. The four raw captures ship as fixtures and replay through the real emulator in terminal-cursor-line-context.test.ts. Ink paints gaps as empty cells, so typed text now renders those as spaces instead of losing them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keep the pending-draft check ahead of the paste-ingest sizing (stablyai#16586) that now feeds createAgentPromptRenderGate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop this branch's composer-draft detector and cursor-line-context module in favour of stablyai#16711's detectTerminalComposerDraft: the guard now refuses on the same draft projection terminal.read exposes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rafts Claude Code writes a mode label into the composer's top rule (`──── ultracode ─`), which COMPOSER_FRAME_LINE rejected, so every such session lost draft detection for terminal.read and for the send guard. The four raw captures from Claude Code 2.1.246 and Codex 0.149.1 now replay through the real emulator as regression coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Reconciled with #16711 (merged yesterday), which added the same Replaying real captures through that detector turned up one gap, fixed in d80e6cc: Claude Code writes a mode label into the composer's top rule ( |
…ding-draft # Conflicts: # src/cli/bundled-skill-guides.ts # src/cli/handlers/terminal.ts # src/cli/root-help-text-secondary.ts # src/cli/specs/core.ts # src/cli/terminal-format.test.ts # src/cli/terminal-format.ts # src/main/runtime/orca-runtime.ts # src/main/runtime/rpc/methods/terminal/terminal-send-method.ts # src/main/runtime/rpc/methods/terminal/unary-schemas.ts # src/shared/runtime-terminal-contracts.ts # src/shared/terminal-composer-draft.ts
|
@coderabbitai resume |
✅ Action performedReviews resumed and review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 3c7b4374-2174-4dce-9f79-e9146e82724d
📒 Files selected for processing (21)
skill-guides/orca-cli.mdsrc/cli/bundled-skill-guides.tssrc/cli/handlers/terminal-send.tssrc/cli/handlers/terminal.test.tssrc/cli/help.tssrc/cli/root-help-text-secondary.tssrc/cli/specs/terminal-send.tssrc/cli/terminal-format.test.tssrc/cli/terminal-format.tssrc/main/daemon/terminal-composer-draft-captures.test.tssrc/main/runtime/orca-runtime-write-terminal-agent-prompt.tssrc/main/runtime/rpc/errors.test.tssrc/main/runtime/rpc/errors.tssrc/main/runtime/rpc/methods/terminal/terminal-send-method.tssrc/main/runtime/runtime-terminal-contracts.tssrc/shared/agent-prompt-pending-input-error.tssrc/shared/rpc-contract/terminal-unary-params.tssrc/shared/runtime-terminal-contracts.tssrc/shared/runtime-terminal-visual-layout.tssrc/shared/terminal-composer-draft.test.tssrc/shared/terminal-composer-draft.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- skill-guides/orca-cli.md
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
Merge conflicts resolved — now
|
| Area | Resolution |
|---|---|
orca-runtime.ts |
Took main's re-export shell; the PR's 55 lines moved into orca-runtime-write-terminal-agent-prompt.ts, which now owns the write path. Verified readVisibleTerminalState, ptysById and providerSnapshotPreferredPtys all resolve on ancestors in the 134-deep mixin chain. |
terminal-send-method.ts |
Kept both option spreads and both catch arms; this PR's arm first, main's acceptedPromptCheckpoint fallback intact. |
unary-schemas.ts, cli/handlers/terminal.ts, cli/specs/core.ts |
Took main's extractions; allowPendingInput / --force re-homed into the new per-command modules. |
terminal-format.ts (+ test) |
Took main's rewritten formatTerminalSend, grafted the pending-input arm ahead of the generic refusal; tests moved into main's describe and updated to its new refusal wording. |
terminal-composer-draft.ts, runtime-terminal-contracts.ts, root-help-text-secondary.ts |
Both sides kept — main's new types/flags plus this PR's. |
bundled-skill-guides.ts |
Regenerated, never hand-merged. |
Guard ordering is unchanged in substance: the composer-draft check still runs after the permission check, and now sits after agentSessionPtyWriteGate.assertAdmitted so the cheap lease gate refuses before paying for a screen read.
Two changes that are NOT conflict resolution — please review these
max-linesrefactors. Two files sit at exactly the 300-line limit on main, so any addition trips the gate, and AGENTS.md forbids amax-linesdisable. Both were shrunk instead:terminal-send-method.ts— hoisted the repeated{ handle, accepted: false, bytesWritten: 0 }payload into onerefusedconst, collapsing five catch arms to one-liners. Arm order and behaviour unchanged.runtime-terminal-contracts.ts— moved the sixRuntimeTerminalVisual*layout types to a newsrc/shared/runtime-terminal-visual-layout.ts, re-exported from the barrel. Pure type move; no importer changed.
- Two test fixes. The PR's tests stubbed the client as
{ call }, but main's handler now runs a prompt-delivery preflight — they failed withTypeError: client.getCliStatus is not a function. Switched to main's ownpromptClient(call, true)helper plus the prompt receipt it requires.
Validation
pnpm tc clean · repo-wide oxlint clean · verify:rpc-params-catalog, verify:bundled-skill-guides, verify:skill-bundle-manifest, check:max-lines-ratchet, check:ts-nocheck-ratchet all pass. Vitest: 1068 files / 11359 tests across src/main/runtime/rpc + src/shared, 129 files / 1328 tests across src/cli, plus terminal-send-pending-draft.integration.test.ts 6/6 end-to-end through the re-homed guard.
No failure was written off as pre-existing — the only two were the real fallout above, and they are fixed.
One note for anyone merging older branches in this repo
git merge first refused with "unrelated histories". The checkout is a shallow clone (boundary ≈ 2026-09-08), so origin/main looks parentless and shares no ancestor with a branch cut in August. git fetch origin main --shallow-since=2026-06-01 fixes it. The pre-deepen "951 commits behind" is an artifact — the real number was 2093.
CodeRabbit had been paused on this branch since August (its status comment said so, and the 28 Aug push drew no review), so it never saw the last few pushes. Resumed it — a fresh review should land on this merge.
…draft before the paste Two review findings on the composer-draft send guard. A labelled top rule is no longer evidence on its own. `──────── build ─` above an ordinary `❯ ls` shell prompt matched the widened frame regex, so the detector read `ls` as composer input and the guard could refuse a legitimate send. A labelled rule now only counts as the composer's top border when the closing border is below the prompt — the box an agent draws, which a section divider in a shell transcript does not have. A plain rule still stands alone, so main's behaviour is unchanged. The composer is also re-read after `beforeWrite`. The write path already re-validates request activity, generation, permission state and PTY admission across that hook; composer state is an invariant of the same standing, and it was the only one not rechecked, so a draft started during the hook would have been pasted onto. Both are covered by tests that fail without the change. The real captured Claude Code 2.1.246 screen still reads its draft, which is what rules out over-tightening. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ding-draft # Conflicts: # src/cli/bundled-skill-guides.ts # src/cli/terminal-format.ts # src/main/runtime/orca-runtime-write-terminal-agent-prompt.ts # src/main/runtime/rpc/methods/terminal/terminal-send-method.ts # src/shared/runtime-terminal-contracts.ts
… writes `RuntimeAgentPromptWriteOptions.inputKind` became required on main. The draft-during-beforeWrite regression test drives `sendTerminalAgentPrompt` directly, so it states the `driving` kind `terminal.send` uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ding-draft # Conflicts: # src/cli/bundled-skill-guides.ts
This branch has not been deployed
1 participant
ELI5
When one agent messages another terminal with
orca terminal send --text ... --enter, Orca pastes the text and presses Enter without looking at the target's input box. If the user had a half-typed prompt there, both get sent as one prompt. Now Orca looks first: if there is unsent text, it refuses and tells the sender what the draft is.--forcekeeps the old behavior on purpose.What Changed
OrcaRuntimeService.writeTerminalAgentPrompt(the Claude Code / Codex paste-and-submit path used byterminal send --text --enter— now inrpc/methods/terminal/terminal-send-method.tsafter the module split onmain— and by orchestration dispatch/inject/worker preambles) now checks the rendered composer inside the serialized submission, right after the existing permission check and before any byte is written. Evidence isreadVisibleTerminalState— the same host-side emulator stateterminal read --screenreturns — so it works for local, daemon, SSH-provider andorca servehosts.detectTerminalComposerDraft— the samedraftthatterminal.readnow exposes — read fromreadVisibleTerminalState(ptyId).draft, so the guard shares one projection with reads (headless emulator, provider snapshot, and renderer-serializer fallback). No detector of its own any more.──── ultracode ─), whichCOMPOSER_FRAME_LINErejected, so any such session lost draft detection for bothreadand this guard. The frame regex now allows a label between rule characters; the capture is the regression test.terminal.sendRPC maps the refusal to{ accepted: false, bytesWritten: 0, refusedReason: 'pending-input', pendingInput: <draft> }and accepts an optionalallowPendingInput: true.agent_prompt_pending_inputis registered as a passthrough error code for the orchestration callers, which fail the dispatch instead of corrupting the draft.terminal send --force→allowPendingInput;formatTerminalSendprints the refusal and the draft instead ofSent 0 bytes; refusals exit non-zero.orca-cliskill guide documents the rule.Related (no overlap found)
writeTerminalAgentPrompt), so expect a textual merge conflict, not a semantic one. It also notes mobile native chat never reaches this seam — consistent with the scope note above.dispatch --injectinto a pane parked on an interactive prompt, before the dispatch row is created. Complementary: this PR runs afterassertAgentPromptPermissionSafeand additionally never reads a numbered dialog option row (❯ 1. Yes…) as a composer.orca terminal readoutput as if typed #12358 — phantom composer drafts interminal read. The runtime's screen read does not composite GUI-layer drafts, so they don't feed this detector. Caveat: if those phantoms are Claude ghost-text drawn undimmed right of the caret, this guard would refuse the send (loud, non-destructive) — the after-caret rule relies on placeholders rendering dim, which is what Claude Code 2.1.241 and Codex 0.149.1 do today.terminal sendentries inhelp.ts/specs/core.ts; merge-order conflict only.Follow-ups (not in this PR)
--force. A softer variant — refuse the first attempt, then concede when the same sender retries the same text against the same unchanged draft — would let unattended agent flows recover without a flag. Happy to do that in a separate PR if you want it.terminal.sendText, agent-teams tmux dispatch,--textthen--enterin separate calls) and to more agents once their composer glyphs are pinned.Why
An agent in terminal A cannot see terminal B's composer, so it cannot know a draft is pending. Verified on Orca 1.4.188 (macOS) with both agents: draft
Refactor the login page so that it+ sentStatus update from the other terminal: the build is green.→ the target ranRefactor the login page so that itStatus update from the other terminal: the build is green.as one prompt. The contract that is violated — "this text lands as its own prompt" — lives in the host runtime's agent-prompt submission, so the guard lives there and every sender inherits it. The refusal is structured (Rule 1 ofdocs/reference/remote-wire-compatibility.md: new optional fields/params only): old CLIs against a new host seeaccepted: false; new CLIs against an old host keep today's appending behavior.Linked Issue
Fixes #16289
Visual Proof
CLI behavior, no UI change. Screen reads from the reproduction (Claude Code 2.1.241; Codex reproduces identically with
›):Before — draft present, then
orca terminal send --terminal <B> --text "Status update from the other terminal: the build is green." --enter:After (same state):
Testing
src/main/runtime/rpc/terminal-send-pending-draft.integration.test.ts— realOrcaRuntimeServicefed the rendered frame viaonPtyData, foregroundclaude: refuses and reports the draft (fails before the fix: the paste is written); still refuses with the caret moved home (\x1b[3G); pastes into an empty composer, including one showing a dim placeholder, and into a trust dialog's option list without reading it as a draft — each proceeds-case assertsaccepted: true, the pasted payload, and the Enter write; appends withallowPendingInput.src/main/daemon/terminal-composer-draft-captures.test.ts— replays four raw PTY captures (__fixtures__/: Claude Code 2.1.246 and Codex 0.149.1, composer-with-draft and permission/trust dialog) through the realHeadlessEmulatorinto Fix terminal reads that flatten composer drafts into output #16711's detector: both drafts detected, both dialogs rejected via the hidden cursor. The Claude draft capture is the one that exposed the frame-label bug.--force→allowPendingInput, refusal output + exit code,formatTerminalSend;errors.test.tspassthrough code.src/main/runtime/rpc,src/shared,src/cli,src/main/daemon,orca-runtime.test.ts(9.3k tests),typecheck:tsc:node/cli,oxlint,check:code-quality:changed,check:max-lines-ratchet,verify:bundled-skill-guides.tests/e2e/cross-version-wire/cross-version-terminal-wire.unit.test.tsfails onoutputPausecapability negotiation identically with this branch's files reverted toorigin/main(pre-existing in this environment).Scope not covered (same append behavior as today): mobile native-chat sends (already prefix Ctrl+U), the desktop review-note two-phase send, plugin
terminal.sendText, agent-teams tmux dispatch, a separate--textthen--enter, and agents other than Claude Code/Codex or plain shells. On Windows ConPTY a draft with the caret moved to column 1 is not detected (style signal disabled there).AI Disclosure
Investigated, implemented, and tested with Claude Code (Claude Fable 5) under my direction; the reproduction and review were mine.
Review
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
allowPendingInputparam;refusedReason: 'pending-input',pendingInputfields). The renderer's note-send treats an unknownrefusedReasonasnot-writable; mobile reads onlyaccepted.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)Generated with Claude Code | Created with /quick-pr