Repository navigation
fix(native-chat): a prompt card owns the chat input until its answer lands (terminal-backed chat, desktop and phone) - #25761
Conversation
Review summaryThe problem. In terminal-backed native chat, the agent CLI runs in a terminal. While it waits for an approval ("Allow Bash?") or a question answer, anything typed into the chat box was sent as keystrokes into that prompt, so a normal message could approve or deny a tool by accident. What changes for the user.
Fixed in this reviewEarlier rounds fixed F01–F05, R2-D01/M01/M02, CI-C01–C06 and N01, as recorded in the previous version of this comment. This review found and fixed: P2, PR-caused or from earlier loop fixes
P3
Deferred, not regressions on main
Verified
Not verified
The PR remains a draft. Nothing was merged or marked ready. |
A newer desktop or phone talking to a host that predates the write settlement field read every accepted reply as "unconfirmed". Prompt cards never dismissed, the phone showed "Response unconfirmed" on every tap and ordinary chat messages were held as "Delivery unconfirmed". The reader now uses writeSettlement when present and otherwise keeps the host's whole-write accepted/refused verdict, exactly as before this branch. Only prompt answers ask for provider settlement; ordinary callers (follow-up delivery, paste drafts, option commands, composer sends) are back on the original contract, so the legacy-handoff error class, its flag, the sequence-only send helper and the mobile handoff hook are gone.
Every pane's Escape/Ctrl+C goes through pty:writeAccepted. This branch had switched that IPC to wait for provider settlement, which dropped the "remount this pane" signal for a daemon session awaiting recovery and could stall later keystrokes behind a slow daemon acknowledgment. pty:writeAccepted is back to its original local-only, synchronous write. Prompt answers opt into settlement with requireWriteSettlement on the same channel, and a settled refusal while the daemon recovers now sends the same remount signal. Ordinary verified sends regain their original fallback write.
A settled local write split into chunks returned plain false when a later chunk was refused after earlier ones were accepted. Callers read false as "nothing was written", so chat showed "Message not sent" with a prefix already in the agent's input. It now reports the write as unconfirmed, the same verdict the paired host gives for a partial write.
When an approval or question card took the input region, the composer unmounted. A message still waiting for its Enter was cancelled and its bubble deleted after the draft had already been cleared, so the message vanished without a notice; composer history was also wiped each time. The composer now stays mounted but hidden while a card owns input, so its state survives. A send that has not submitted yet is still stopped (its Enter would answer the card), but its bubble stays with "Message not sent" so the text is not lost. The composer ref is detached while hidden, so root typing, paste and reveal focus never reach it.
The "answered" dismissal lived in component state. Toggling chat to terminal and back, a PTY reconnect, or leaving the phone session and coming back while the approved tool was still running brought the answered approval back, and it then took over the input again. Desktop now keeps the answered occurrence per pane outside the view; phone keeps it per chat tab outside the controller. Both still retire it when the pane observes the prompt clear or change, desktop also when the tab retires, and both maps are size-bounded.
Older hosts' accepted answers now dismiss like acknowledged ones, the composer stays mounted under a card, and answered prompts survive a view remount. The gate's invariant, oracle, assertion list, new test files and the two latest evidence runs now describe that contract.
… Send on the phone The chat input could stay locked behind a card the host never closes (for example after a Deny typed in the terminal), and Escape on a focused approval card denied the tool even when the user meant to close a picker. - A Hide control (chevron) on terminal approval and question cards, desktop and phone, hides that prompt occurrence and gives the input back. It writes nothing to the agent and uses the same per-occurrence dismissal as an acknowledged answer, so a new occurrence shows the card again. - On desktop, Escape on a card now does the same Hide instead of Deny, and a card that appears while the user is typing no longer takes focus. - On the phone, a card blocks only Send: typing, dictation and image attach keep working on the draft. The placeholder is back to the normal one.
Since an older host's accepted write now counts as delivered, the requireWriteSettlement comment and the reliability gate's oracle said the opposite of the code. Both now describe the current rule.
…e round-2 gaps Hide removed a card completely, so nothing on screen said a prompt was still waiting, and several edges let the chat type into a live prompt. - Collapse (the header chevron, or Escape on desktop) folds the card to a one-line strip above the composer; the strip's chevron expands it back. Collapsing writes nothing, frees the composer, and is disabled while an answer is still being written. Each pane or tab keeps the occurrence as answered or collapsed, so a remount restores the same view. - Questions now carry the host wait's start like approvals, so an identical question in a new wait shows again (desktop and phone). A transcript-only prompt, which has no wait start, is dropped when the view stops observing it, and a transcript still loading no longer clears a dismissal. - Desktop: while a card owns the input, the hidden composer cannot send or interrupt even if it still has keyboard focus, and the card takes focus in the same commit. A send the card retires no longer types Ctrl+U under it. - Phone: an Ask hides the heuristic card read from the same waiting status, and the dismissal store is scoped by host, worktree and tab.
…n pane Collapsing a question card unmounted it, so expanding it again lost the chosen step, selections and typed "Other" text; Escape typed in that text field collapsed the card. A card arriving while the user typed in another surface (sidebar, notes, a browser URL bar) also took the keyboard. - The collapsed card now stays mounted but hidden (and inert on desktop) under its strip, on desktop and phone, so a partial answer survives collapse and expand. Escape inside the card's text field no longer collapses it. The question card shows the same focus ring as the approval card. - A card takes focus only from inside its own pane (its hidden composer) or from the page body, never from a text field elsewhere. - Desktop and phone share one dismissal store in src/shared, bounded by the existing scope-cache helper, which moves to src/shared with it. - The card send imports the verified helper from its own module, and the phone files are split so each name matches its contents (header action, strip, lane selector).
Since a collapsed card stays mounted, Escape or the chevron left keyboard focus inside the now hidden, inert card. The composer's reveal-focus took that as focus already in the pane and stood down, then the browser dropped focus to the page body, so typed keys went nowhere. Reveal-focus now treats focus inside a hidden or inert subtree as not in the pane and focuses the composer. On the phone, collapsing a card also dismisses the keyboard so a hidden reply field does not keep it.
The question card header spread its three items with justify-between, which put the new chevron in the middle of the header. The title now takes the free space, as in the approval card, so the chevron sits next to Cancel at the right edge.
…2025/nc-terminal-prompt-gate # Conflicts: # mobile/src/session/MobileNativeChatView.tsx
Main now runs Vitest under Bun, which resolves a long data: URL import as a package name, so the SSH delivery test loads its bundled mobile module from a temp file instead. The phone prompt harnesses mock the live line that main's view now renders, and add Platform, which main's text-selection helper reads, the same way main's own view tests do.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (99)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe changes add provider-settled terminal writes and use acknowledgment outcomes when handling native chat replies. Mobile and desktop prompt cards now track prompt occurrences, support collapse and expansion, and connect dismissal to reply outcomes. The composer remains mounted while a prompt card owns its input. Tests cover settlement results, prompt lifecycle, and older-host behavior. An experimental reliability gate records criteria and local evidence, alongside a CI timing-key update. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to A prompt card now owns chat input until its answer is confirmed written to the terminal, and replies from older hosts are still treated as accepted. No concrete defect remains open at this head. Live testing with real agent CLIs, on phone hardware, and on non-macOS platforms has not been done; that is a validation gap, not an identified bug. 🚥 Pre-merge checks | ✅ 4 | ❓ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 46.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 50 functions across 50 files. (48 skipped: 8 unsupported, 40 over the file limit.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
…'s settled write Upstream's SSH delivery suite (stablyai#25761) showed two gaps in the fork's adoption of delivery uncertainty: - Ask-answer groups were paced on fixed timers, so a slow body could be overtaken by the selector's Enter. With a settlement callback each group now waits for the previous group's provider acknowledgment, and settles the answer a submit gap after the last one, as upstream's observed writes do. - The fork's acknowledged transport called its own pty:writeInputAccepted. Upstream's writeAccepted with requireWriteSettlement now answers the same question, SSH included, so the fork uses it; sends that wait on settlement also ask a paired host for it. The SSH suite now drives the fork's target-based API.
ELI5
In terminal-backed native chat, the chat drives an agent CLI (Claude Code, Codex, OpenCode) that runs in a terminal. When the agent stops to ask for approval ("Allow Bash?") or asks a question, the CLI is waiting for keystrokes. Before this PR, the ordinary chat box stayed active, so a message you typed was sent as keystrokes into that prompt. The agent could read it as your answer and approve or deny a tool you never meant to touch.
Now the prompt card owns the input while the agent is waiting. On desktop the card takes the chat box's place. On the phone, Send is disabled, but typing, dictation and attachments still work. You can collapse the card to a one-line strip at any time to get the chat box back. Collapsing sends nothing to the agent, and Escape collapses instead of denying.
What Changed
Before: an approval or question card appeared above the chat box, and pressing Enter in the chat box sent your draft into the agent's prompt as an answer.
After, desktop:
After, phone: while a card is shown, only Send is disabled. You can still type, dictate and attach. The same collapse chevron frees Send for that prompt.
How it works:
src/sharedholds these records for both platforms, and entries are dropped when the prompt clears or changes, or when the tab closes.requireWriteSettlementflag, and current hosts reply with whether the terminal actually accepted the bytes.accepted: trueas before. That is treated as accepted, so a newer phone or desktop works normally against an older host.Why
The bug was that ordinary chat input and prompt answers share one keystroke stream. Making the card the only input while a prompt waits removes that collision on both desktop and phone.
Orca can tell when a prompt opens, but agents send no event when one closes after Deny, Escape or an answer typed in the terminal. A host-side lock on ordinary messages could therefore strand the user indefinitely, so this PR keeps everything in the client's presentation layer and always leaves a way out: collapse. The host refusal of ordinary messages is deliberately not added. That should wait until real prompt-closing events have been captured.
Alternatives considered:
Differences from the common pattern
Linked Issue
Maintainer change; no linked issue. Related work: #25588 and #24934 → #25430 → #25624.
Visual Proof
Remote Electron app on a paired Mac. The app ran fully isolated: its own home and data folders, launched in the background, with screenshots taken through the developer protocol. The prompts were injected through the app's state the same way
tests/e2e/native-chat-ask-user-question-card.spec.tsdoes. The agent was a stand-in that logs every byte it receives. All clicks and keys were real input.Testing
1for Allow and one2for the question answer, and a double-clicked Cancel sent a single Escape. Typing, Enter, Escape, collapse, expand, a new prompt, and a card arriving mid-typing all wrote zero bytes.Review
Reviewed in several rounds, with independent reviewers per area (desktop, delivery and wire compatibility, phone, architecture), a readiness pass, and remote app QA. Details are in the review comment on this PR.
Agent skill upstream boundary
Notes
Known limits, left as follow-ups:
Checklist
a039c40(16 passed, 16 skipped, 0 failed)