Repository navigation
feat(native-chat): mid-turn messages wait as editable cards above the composer - #23731
Merged
Merged
Conversation
No client ever called agentSession.requestHandoff or mounted the handoff chrome. Delete the handoff coordinator, the terminal-owner runtime, the proof write path and the unmounted UI. Keep agentSession.handoffStatus, which released desktop clients read for worktree activation, and let records an older build left mid handoff reconcile through the ordinary restart and recovery paths.
Eviction now drains delivered events before quit's resume-offer snapshot. An unbounded wait there sits ahead of the provider stop, so a sink whose journal write stalls kept the child running until the step deadline aborted the eviction. The offer is advisory: bound the drain and stop the child regardless. Co-Authored-By: Claude <noreply@anthropic.com>
`claudeAuthEnvCarriedForward`, `isPathWithinDirectory` and
`queryWindowsProcessRowsFresh` lost their last caller with the handoff. The
fresh-scan tests now go through `queryWindowsProcessDescendants({ fresh: true })`,
the teardown path that still depends on that contract.
Co-Authored-By: Claude <noreply@anthropic.com>
Six comments still named the handoff coordinator, a handoff suspend, or a terminal-owned session as live participants in the flows they describe. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ement The removed restart handoff test pinned this branch; nothing else did. Co-Authored-By: Claude <noreply@anthropic.com>
The handoff removal dropped the per-session queue from `handoffStatus`, so a read landing mid-start reported the reservation (no owner) instead of the settled chat owner, and shipped desktop clients blocked worktree activation on it. The read is queued again, as it was before the removal. Co-Authored-By: Claude <noreply@anthropic.com>
The gate only refused a write when a PTY had been bound to a chat session, and the only code that ever bound one was the terminal handoff this branch removes. With it gone, every admit/readmit returned "admitted" unconditionally, so the checks on the renderer write path, the runtime controller backstop, terminal.send, agent prompts, preview input and orchestration pointers, the refusal fields on terminal.send and worker-start receipts, the plugin and CLI refusal copy, and the adopted-pane orchestration routing could no longer run. Ordinary writes take the same path in the same order as before. Co-Authored-By: Claude <noreply@anthropic.com>
…alled appendLegacyTranscriptMessages fed the terminal transcript catch-up and proveClaudeTranscriptBranch backed the terminal owner's exit proof. Both lost their last caller with the handoff. Their tests now go through the live entry points instead: the roster bounds through the legacy import, the pinned-read and growth tests through the ancestry replay the history window uses, and the marker rules through the string proof in their own file rather than the session-file resolver's. Co-Authored-By: Claude <noreply@anthropic.com>
A send refused because the chat's owner is not settled showed "The session is mid-handoff (<stage>)." in the composer. With the handoff gone, the stages that reach it are a chat that is still starting, or one whose previous agent process has not yet been confirmed stopped. The message now says which of the two it is. The refusal code is unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
With the terminal handoff gone, the module named codex-tui-rollout-proof holds only the pinned rollout lookup that structured Codex launches use to resume a thread, so the name described code that no longer exists. Rename the module and its options type. Also drop a mobile allowlist assertion that pinned the removed agentSession.requestHandoff method, which no longer exists to allow.
The handoffStatus reply type still listed the terminal handoff's fields and states (terminal placement, host label, proof retry, queued and waiting phases, the to-terminal direction). No host writes them any more and the only client reader parses the reply as unknown, so they described nothing. The reply on the wire is unchanged.
…t decode Nothing in this build writes a terminal owner (`runtimeKind: 'tui'`) or the handoff's `preparing` / `old-owner-stopped` stages, but the in-memory types still admitted them, so readers across the host kept branches for values no path produces and the compiler could not point at them. The store now validates the on-disk shape, which still accepts those values so an older record is not quarantined, and maps them once while parsing: - `preparing` and `old-owner-stopped` become `recovering` - a `tui` lease becomes `native`; when it records a process it also becomes `conflicted`, the claim every build probes but never stops. A plain native owner would be stopped by restart recovery, here and in older builds. Revisions are taken over the normalized state on both sides of every compare, and the mapped record reaches disk with the store's first transaction, the same way the tab-id backfill does. The in-memory types narrow to what this build writes, and the branches that existed only for the removed values go. Structured-worker identity keeps its verdict for a former terminal owner by refusing a conflicted claim rather than a non-native kind.
…ation A reservation only ever names a native owner now, so the request no longer carries a kind and the reserved lease records `native` directly. The attach params keep `runtimeKind`: agentSession.ensure and create accept it, and the operation fingerprint stored in the ledger covers it.
…at changes nothing else Hiding a tab also committed the visibility index, so the no-op transaction wrote the file even when its open-time revision was wrong. Committing the index first leaves the pending rewrite as the only reason to write.
…ration A write carried the fence of the last frame the pane read, and the host refused it unless that fence was still current. An idle release and the restart after it each move the fence, and the release publishes nothing, so a send after a release was refused "Expected runtime fence 1; the session is at 3", and a Stop queued behind a cold start was refused as stale. Every write already names what it acts on: a send its conversation, a cancel its turn, a prompt answer its item revision, a rewind its epoch; an option is last-writer-wins. So admission stops comparing the client's fence, and the rebase that papered over one restart (admitAtResumedFence, resumedFromFence) goes with it. The writer-lease check stays, and so does the attach's compare-and-swap. Frames now stamp the fence read when each frame is sent instead of a copy each subscriber kept, which went stale on the same release.
A journal write and its delivery to open readers were two calls, and some writers made only the first. A failed start whose lease could not be handed back, a provider revision with no frame behind it, and eviction's settlement were all journaled without reaching an open chat. A journal handle now reports every durable change, and the host's session map binds that report to the session's readers when the handle is set. Writers no longer publish what they append; the per-writer publish calls are deleted.
…dle enters only through the map
…ackfill cannot supply its rewrite The seeded record had no surface tab id, so the next open backfilled one and that rewrite alone made the no-op transaction write. The test passed with the legacy-lease rewrite signal removed.
The handoff removal deleted it alongside the terminal-owner tests, but it covers the surfaced-PTY block that still guards resume, including an agent whose ownership is unknown.
Each commit now delivers itself, so the publish a provider frame still sends afterwards found every reader caught up but still read rows and rebuilt the timeline for each one. A caught-up reader now skips the read.
# Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-subscribers.ts
A send to a chat with no running agent restarted the agent inside the send
call, before the message was recorded, so the client waited for the whole
start and a failed restart refused the message. Claude held prompts sent
during startup, and those could settle as "unconfirmed".
A send is now accepted inside the session's serialized queue: one ledger row
and one submission row marked handoverRecorded, published, answered pending.
A per-session delivery loop exists while a message is queued. It starts the
agent through the same serialized attach a hold uses, waits outside the queue
for a Claude child to prove its start, and hands the oldest queued message
over as its own serialized step, writing dispatch{pending} before the adapter
call. A start it needed and did not get writes one error-tone row and rejects
every queued message with the same words; a start Stop cancelled writes none.
Settlement follows from the rows. A queued message is provably unwritten, so a
close, an eviction or an exit rejects it. A handed-over message stays in doubt.
A queued row at or below the sequence a handle found when it opened was left
by an earlier process and is rejected at open, with no latch. Stop withdraws
queued messages with no writer lease and no fence. An attach failure keeps the
conversation open, and the attach adopts its journal. Owed work counts the
loop and queued rows.
A compaction or rewind found prepared when a conversation opens was started
under a child this process no longer has, so the open settles it rather than
leaving it to refuse every send until a view attaches. The open cursor is
scoped to its epoch, because sequences restart when an epoch is replaced.
Deleted: restart-before-admission, recordFailedRestart, the fence rebase,
Claude's startup gate, the attach's forget on failure and its own crash
boundary. Clients without agent-session.accepted-send.v1 get their reply held
until the handover; the desktop and paired desktop lists advertise it.
A child that proved its start and then exited before its message was handed over left the message queued: the exit settlement returned early when nothing else was in flight. Delivery then started another child for it, and a child that died the same way started another, without end and without a row. A retried settlement for an earlier generation, run by the attach that delivery started, did the opposite: with that generation's turn unfinished it rejected the message queued for the child being attached. The settlement now takes the rejection for queued messages from its caller. The unexpected exit and the eviction pass one, and it applies even with no other work in flight; the retry for an earlier generation passes none.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…when that settlement was skipped The Stop checked the draft table for a card to pause. When the per-row hook that settles a withdrawn hand-off was skipped, that card was still 'dispatched', so the Stop recorded no pause; the drain later healed it back to waiting and sent it, although the user had pressed Stop. Retirement had the same blind spot and could drop a pause while such a card was owed. What a pause holds back is now one predicate, judged inside the transaction that records or retires it: a waiting card with no hold of its own (one SQL EXISTS), or a dispatched card whose consumed submission was rejected with a settlement back to waiting (read against the journal's submissions). The Stop first runs the owed settlement, as the drain does; if that fails, the owed card still counts, so the pause is recorded rather than skipped. recordPause now checks inside its own transaction and returns whether it recorded, and any draft-table write (and the per-row hook, the consume and the open-time repair) retires a pause that no longer holds anything back.
…judgement when no pause exists The retirement test recorded its second pause over a queue with nothing to hold back, so the recording returned false and the "retired" assertion proved nothing; ablating the per-row hook's retirement passed every test. The hold case now asserts the pause was recorded, and a new test has a delivered echo, through the per-row hook, withdraw the last card a recorded pause holds back. Retirement runs on every appended journal row, so it now checks the pause row by key first and judges nothing when no pause is recorded. Two comments were brought in line with the owed-hand-off rule and rewrapped.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
ℹ️ No new issues found in this delta. Two earlier inline notes remain open and unaddressed, so this is not an approval.
Reviewed changes
Re-reviewed the commits since the prior pullfrog review (b4da190 → e4e6365): the queue-level pause redesign and its follow-up fixes.
- The Stop's per-card hold became a queue-level pause derived from the journal. A new
queued_message_pausesrow records where a Stop (or a/clearcarry) took effect;structuredQueuePausederives the pause from that fact plus whether a person's provider-accepted turn has started since, so no hold is stored on the cards. - Resume.
agentSession.queuedMessagesResumelifts whichever pause holds the queue, with a fresh operation id per press; a/clear's replacement queue starts paused ascleared, not as an interrupt. - Restart pause. A waiting card written by another host process pauses the queue as
restarted, adopted into the current instance once a post-open person turn starts or Resume is pressed. - Publication. The host publishes
{queuedMessages, queuePause}together, gated onhasResumableQueuedMessageso a Resume header never offers to send nothing; the reducer and coalescer keep the pause with its list, and a pause-only frame at an unchanged cursor still updates state. - Card header.
NativeChatQueuePauseRowrenders one reason row above the list with Resume; cards keep Steer, and a per-card caption remains only for a failed conversion.
I traced the Stop → withdrawal → pause-record, restart → adopt, /clear carry, returned-card-blocking, and Resume paths, and ran the delta's suites (structured-agent-session-queued-messages, structured-agent-session-queued-pause-lift, queued-message-store, the reducer/coalescer tests, and the renderer list/resume tests — all pass). I could not construct a path where a paused draft auto-sends without the user acting, or where the pause cannot be lifted.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…tooltip spaces its shortcut The pause row, when shown, is the box's first row and each card a row below it, divided rather than individually bordered. The Steer tooltip groups its hint and the shortcut chips with the house gap, so the chips no longer touch the text.
#23059 makes /compact a message the chat sends and reads turn facts from the turn record. The queue follows it: - /compact is now a queued message and then a turn, not a command holding the session's lane for its whole provider call. A capable send during it becomes a card held by the ordinary `working` hold, and the drain waits behind the compaction's turn like any turn. So the compact side lane (structured-conversation-command-lane.ts), draft-only admission, Send-now's up-front refusal and the queue gate's `command` hold are gone. Delete, Resume and Send-now run on the session's lane, which a compaction no longer holds, so they answer at once; Send-now's card is accepted and handed over after the compaction, the order every send keeps. A /compact from the command RPC is the person's own turn (origin 'client'). - The queue gate's `blocked` hold is now main's send block itself, so a clear in doubt, a superseded source and an uncertain rewind hold the queue by one rule. - Submission folding moved to journal-submission-fold.ts on main; the draft link, origin, the latest person's turn and the shared settlement guard are carried there. The /clear carry is re-applied to main's clear flow. - To stay within max-lines, the row writer's cursor-returning append replaces the store's appendRow, the open-time draft repair rides the restore, and the queue wiring reads the host's mutation context.
…send-result union
# Conflicts: # src/renderer/src/components/native-chat/use-structured-agent-session.ts
…he opener's bar A Steer hands the draft over under a fresh submission id, linked by queuedMessageId, and the host scopes its row to the running turn; the transcript keeps it inside that turn with no bar of its own, settled or running.
Contributor
Author
|
Status: ready to merge after #23726. Head
|
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 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 (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughAdds host-aware follow-up queueing for structured chat. The change stores queued-message state and delivery details, projects host drafts into cards, and adds Resume, Steer, Edit, and Delete actions. It also adds a queue setting, capability checks, composer steering shortcuts, and translations. Outbox handling now reconciles queued-message hand-offs and holds certain attempted sends after Stop until an explicit retry. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The supplied changes identify no concrete merge-blocking issue. Queue controls and composer integration appear consistent with the intended behavior, subject to normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The queue changes when messages can reach the agent and how interrupted sends recover. Inspected controls preserve session identity and prevent several duplicate-send and unintended-resend scenarios. No introduced security weakness was established, but reconnection and ownership-transfer behavior remains only partially verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Contributor
There was a problem hiding this comment.
ℹ️ No new issues found in this delta. Two earlier inline notes remain open and unaddressed, so this is not an approval.
Reviewed changes
Re-reviewed the commit since the prior pullfrog review (429909d → 64d9ecf): one authored commit that changes where the queued-message area renders.
- Queued cards moved above the status area.
NativeChatQueuedMessageListnow renders beforeNativeChatStructuredSessionStatusinstead of between the thread-goal banner and the prompt cards, so the background-task strip (running shells and agents) sits below the cards.NativeChatStructuredSession.tsxis the only production file touched. - A DOM-invariant test pins the new order.
NativeChatStructuredSession.goal-dock.test.tsxadds a case asserting a queued card precedes the task strip and that the strip still docks directly on the goal tab. Reverting the component placement makes the new assertion fail (4vs0), so it can fail for the right reason.
I ran the four NativeChatStructuredSession* suites (47 tests) at this head: all pass.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Contributor
Author
|
Status: ready to merge. Head
|
7 of 9 tasks
2 tasks done
This was referenced Sep 30, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Where this fits: the plan for messages sent while the agent is working
The problem. In a structured (native) chat, a message you sent while the agent was still working went straight to the agent. What happened next depended on the agent:
Orca then drew the conversation wrong:
The plan. A message sent mid-turn waits as a card above the message box, with three actions:
Otherwise it sends by itself when the turn ends. Only Steer puts a message into a running turn. With queueing turned off in Settings, mid-turn messages still go straight in, as they do today.
The work is split into six PRs:
Steer on Codex also needs Codex's turn-steer support from #21062, which is still open.
The queue stays switched off, meaning the host does not advertise it and every client behaves as today, until PRs 1–6 and #21062 have landed. Turning it on is then a one-line change.
This PR is step 5: the desktop cards for the queue in #23726.
ELI5
On desktop, a message you send while the agent is still working now waits as a card just above the message box, instead of going straight into the agent's current work. You can Steer it (send it now, into the running turn), edit it, or delete it. Otherwise it sends by itself when the turn ends. This is the desktop half of the queue from #23726.
What Changed
Before: pressing Enter mid-turn sent the message into the running turn immediately, and it appeared in the transcript as if it had already been answered in order.
After, against a host that queues messages (#23726):
Every action hands focus back to the composer.
/clearsend exactly today's requests.Mechanism. The cards render the host's published
queuedMessageslist: the client stores no queue of its own. Card actions are the host's queued-message RPCs.The send outbox asks for queue delivery only when the host advertises the capability. Each outbox entry records what its first attempt actually sent (
sentDelivery), and a retry of the same id replays exactly that, so the host's record of the operation always matches. While the host's support is still unknown, a new send simply goes out as today; nothing waits on it. A send the host visibly holds as a draft frees the outbox's single in-flight slot, so the next message isn't stuck behind it.The desktop never compares a draft id with a submission id. The host names the draft on every submission it makes from one (
queuedMessageId, #23726), and the desktop reads that link to hide a card once its message is on its way, to drop its own outbox entry, and to decide what Stop may hand back.Stop and a send already on its way. A queued send that already went out is never handed back to the composer by Stop, because the host may hold it as a paused card, and handing it back would put the same text in two places. It is marked instead: nothing resends it automatically, it shows "Message delivery is unconfirmed" with a Retry, and Retry replays the same id. A refusal that arrives after Stop is handled exactly as it would be without Stop.
Stop and
/clearstay plain requests. The host keeps the drafts, and no draft text travels back to the client.Why
Rendering the host's list means the desktop, the phone and a second window always show the same queue, and it survives a reload or restart.
The alternative, a client-side queue, was rejected. Devices would disagree, a reload would lose it, and it would need its own restore logic. Earlier versions that returned draft text to the composer on Stop kept producing restore bugs, so Stop now simply pauses the cards.
Linked Issue
None; follows the mid-turn send problems found during QA of #23537 and #23573.
Visual Proof
Queueing is switched on in a local build only; the capability stays dark until the stack lands.
/clearTesting
Latest live QA (current head,
64d9ecf3d4e). Second Mac, real Claude CLI, queueing switched on in a local build only. Both scenarios pass, with nothing touched while the agent worked:Earlier live QA (queue-level pause). Live desktop QA on a second Mac with a real Claude CLI, queueing switched on in a local build only: 6/6 scenarios pass at
e4e63654a1c(tooltip and icons; Stop with two cards then Resume; Stop then a new message; Steer while paused;/clear; a Stop with an empty queue leaves no pause), and the grouped-box restyle was re-shot ata7658beb4f9.Live desktop QA ran on a second Mac with a real Claude CLI, 8/8 scenarios passing:
/clearkeeping a paused card;That run was at an earlier head of this stack, before it was rebuilt on #23026's final Stop design and before four review rounds of fixes (the draft link, what Stop keeps, and the recorded wire field). The card flows are the same; the Stop and lost-answer cases changed since, and are covered by the automated tests below. The phone QA for #23726 and PR4 also checked that the desktop shows the same cards as the phone at every step.
Automated tests cover:
/clearanswer;/clearsending today's exact requests to an old host;queuedMessageId;The PR's 19 test files (142 tests) pass, as do main's outbox, Stop, withdrawal and delivery-notice suites next to them. Web and node typecheck, oxlint, the anti-slop and code-quality lints, the changed-code quality gate, and the localization catalog and extraction checks all pass. The strings are in all six catalogs.
AI Disclosure
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
/clearis saved).