Skip to content

feat(native-chat): hold mid-turn messages in a host-owned queue - #23726

Merged
brennanb2025 merged 165 commits into
mainfrom
mid-turn-queue-host
Sep 29, 2026
Merged

brennanb2025 merged 165 commits into
mainfrom
mid-turn-queue-host

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 20 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3610 $\color{#cf222e}{\Huge{\mathbf{−}}}$​8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3602
Prod 62 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3721 $\color{#cf222e}{\Huge{\mathbf{−}}}$​255 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3466

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:

  • Claude folds it into the work it is already doing, at its next step.
  • Other agents start a new turn or refuse it.

Orca then drew the conversation wrong:

  • The "Working for" bar jumped to the newest message.
  • A single piece of work showed up as two turns (for example "Worked for 17s" and "Worked for 52s"), with the time counted twice.
  • On the phone, the running turn's rows could disappear.

The plan. A message sent mid-turn waits as a card above the message box, with three actions:

  • Steer: send it now, into the running turn.
  • Edit.
  • Delete.

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:

# PR What it does Why the queue needs it
1 #23573 (merged) The turn's bar stays on the message that opened the turn. A steered message still lands mid-turn, and so does every mid-turn message with queueing off.
2 #23671 (merged) Chat rows are grouped by the turn that produced them, not by the nearest message above them. A steered message sits among its turn's rows. A turn the agent opens by itself gets its own bar.
3 #23621 (merged) Claude: a message folded into the running turn is recorded as joining that turn, not as a new turn. Steer on Claude is exactly this fold. Without it, every Steer splits the turn in two; desktop QA of the queue showed that split.
4 #23726 (on main) The queue itself, held by the computer running the agent. The core.
5 #23731 (on #23726) Desktop cards. The desktop interface.
6 #23736 (on #23726) Phone cards. The phone interface.

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 4: the queue itself. The host holds mid-turn messages, sends each exactly once, and publishes the list that the desktop (#23731) and phone (#23736) cards render. Steer turns a card into a normal send immediately. On Claude it folds into the running turn, and #23621 makes that fold count as the same turn.

ELI5

When you send a message while the agent is still working, Orca now keeps it in a queue on the machine running the agent, instead of pushing it into the agent right away. Each queued message waits as a card you can send now, edit, or delete, and it goes out on its own when the current turn ends. This PR is the host side. The desktop and phone cards are separate PRs stacked on this one, and the whole feature stays switched off until they all land.

What Changed

Before: a structured chat sent a mid-turn message straight to the agent. What happened next depended on the agent. Claude folded it into the running turn at its next step, and other agents started a new turn or refused it. The message showed up in the transcript as if it had been answered in order, and the running turn's bar and timing attached to whichever message was newest.

After, for a client that asks for queueing (delivery: 'queue-if-active') against a host advertising agent-session.queued-messages.v1:

  • A mid-turn send becomes a draft. It goes into a new queued_messages table in the session's own journal database, not the transcript, and the send answers queued. It becomes a draft when a turn is running or older drafts are still waiting (so order is kept).
  • Messages with images send right away, as they do today. Drafts are text only.
  • Drain at turn end. When the turn ends, the oldest waiting draft becomes an ordinary submission, one per turn. That happens in one SQLite transaction on the draft and submission tables, so a draft is sent exactly once. From there, today's delivery path runs unchanged.
  • Send now (the card's Steer) turns the draft into a submission immediately, through the same step.
  • Delete removes the draft. Edit needs no host work: the client copies the text into the composer and deletes the draft.
  • Stop pauses the queue; it never throws drafts away. The pause belongs to the queue as a whole and is published once per chat as queuePause: { reason: 'stopped' | 'restarted' | 'cleared' }. Clients show one row above the cards ("Queue paused because you interrupted") with a Resume button; the cards keep Steer, Delete and Edit.
  • How the pause is decided. It is worked out from the chat's own history rather than stored as an on/off flag: a Stop records the journal position where it took effect, and the queue is paused until a turn the user asked for starts after that point, or the user presses Resume (a new agentSession.queuedMessagesResume RPC). Each submission now records who asked for it (origin: 'client' | 'host'), so orchestration mail, restart continuations and a host-sent launch prompt never lift the pause. Steer on one card sends that card; once its turn starts the rest drain after it.
  • A pause covers only the cards it paused. A Stop over an empty queue records nothing, and the pause is retired in the same database write that removes the last card it holds back, so it can never catch a message typed long after. The row is shown only while Resume would actually send something (not while a returned card blocks the queue).
  • A draft caught mid-hand-off rejoins the queue. If Stop, a restart or closing the chat withdraws a draft that had just been handed off but never reached the agent, the draft goes back to waiting in its original position instead of becoming a blocking card. It then drains in order after your next message.
  • Stop runs first. The pause is recorded inside Stop's own serialized step, as Stop takes effect, so no draft can slip out between Stop and its pause. A Stop that fails before taking effect records nothing.
  • Restart and /clear. After an app restart the queue reads paused with reason restarted (derived from which process wrote the drafts). /clear carries drafts to the new conversation paused with reason cleared. Resume or the user's next turn lifts either. A /compact the user sends counts as a turn they asked for, so after a Stop it lifts the pause too, and the waiting cards send once the compaction ends.
  • Refused drafts come back. If the provider refuses a sent draft, it returns as a card carrying the same typed reason a refused message carries (returnedRejection, plus the sentence), so the card says why and the text is never lost silently.
  • Each submission names its draft. A submission made from a draft carries queuedMessageId, and every hand-off uses a fresh submission id. Clients use that link, never id equality, to tell a draft from the message it became.
  • Sent exactly once. The journal refuses a second submission under an id it already holds. A replayed send answers from the draft or its linked submission instead of sending again. A late echo proving that a withdrawn hand-off actually reached the agent withdraws the paused copy.
  • Bookkeeping never blocks the chat. Queue bookkeeping runs in its own savepoint, so a failure there is logged and repaired later instead of undoing the chat's own journal write.
  • During /compact. Since fix(native-chat): turn facts come from the turn record, and /compact is a message the chat sends #23059, /compact is itself a queued message that then runs as its own turn, so the queue treats it like any other turn: a send made while it runs becomes a card and drains after it, Delete completes at once, and Steer is accepted and handed over once the compaction finishes. The queue no longer has any /compact-specific handling; the special holding lane earlier revisions of this PR needed was removed when this PR was brought up to date with fix(native-chat): turn facts come from the turn record, and /compact is a message the chat sends #23059.
  • Publication. Every change to the draft table publishes through the journal's existing commit listener. Every subscriber (desktop, phone, a reconnecting client) sees the same queuedMessages list in live frames and in catch-up.

The capability is defined but not advertised, so every client and host behaves exactly as today until the rollout change. What remains before switching it on: #21062 (Codex steer) and the desktop (#23731) and phone (#23736) PRs. #23573, #23621, #23671 and #23026 have landed.

Why

The queue lives on the host, as a separate draft store that becomes an ordinary message in a single transaction when it is sent. That avoids a whole class of bugs by construction. A draft is not in the transcript, so nothing reorders, no turn bar attaches to it, and no delivery bookkeeping has to guess whether it was answered. Keeping the queue on the host also means the desktop, the phone and a remote (SSH) host all see the same queue, and it survives an app restart.

Alternatives considered:

  • Queueing in the client. Two devices would disagree, and a restart would lose the queue.
  • Storing drafts as submissions with a "queued" flag. Earlier drafts of this design did that. Every delivery reader then had to learn to skip them, and reviews kept finding readers that didn't.
  • Having Stop and /clear withdraw drafts and hand their text back to the composer. This kept producing restore bugs across devices and restarts. Holding the drafts is simpler, and nothing is ever lost.

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 in this PR.

Desktop: Stop pauses the queue Desktop: Resume drains it, one per turn Desktop: after /clear
Phone: the same paused queue Phone: after /clear A Stop with an empty queue leaves no pause

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Latest 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 at a7658beb4f9.

Live phone QA in the iOS Simulator paired to a dev host at 7ae9f138a97, phone bundle at 38f06968472: 5/5 scenarios pass (paused row with Resume; Resume drains one per turn; Steer while paused; /clear; empty-queue Stop leaves no pause), cross-checked against the desktop.

Live QA, with a real Claude CLI:

  • These runs were at an earlier head, before this PR was rebuilt on fix(native-chat): Stop is there from the moment a message is sent #23026's final Stop design and before three review rounds of fixes. The card flows are the same; the Stop and restart edge cases changed since, and are covered by the automated tests below.
  • Desktop, on a second Mac, 8/8 scenarios pass: queue and drain, Delete, Edit, Steer, Stop pausing two cards then draining after the next message, /clear keeping a paused card, turning queueing off, and Cmd+Enter.
  • Phone, in the iOS simulator paired to a dev host, 7/7 scenarios pass (same flows, cross-checked against the desktop).

Automated: 564 tests across 52 files pass after bringing this PR up to date with main (including #23059): this PR's host tests, main's Stop, cancel, compaction, turn and journal tests next to them, and the cross-version wire tests. They cover:

  • the draft store, including exactly-once consume;
  • the queue gate and every hold reason;
  • Stop and restart holds, and what lifts them;
  • the /clear carry, including the payload fingerprint re-scoped to the new conversation;
  • a send, Delete and Steer during /compact, which the queue now treats as an ordinary turn;
  • refused drafts returning as cards;
  • publication to live and catching-up subscribers;
  • the mobile RPC allowlist and the old-client compatibility path;
  • the draft link and fresh ids on every hand-off, the duplicate-id guard, and the replay answers;
  • a Stop-caught draft rejoining the queue in order, the failed-Stop undo, and the late-echo withdrawal;
  • a bookkeeping failure that must not undo the journal write, and its later repair.

Removing any single fix fails its own test. Node, web and mobile typechecks and oxlint pass.

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

brennanb2025 and others added 30 commits September 24, 2026 23:36
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.
…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.
…ly send

A waiting card behind a returned one is blocked until the user acts on the
returned card — the drain never sends past it — so a pause over only such
cards still offered a Resume that sent nothing. The rule for "a card Resume
would send" is now one function: waiting, no hold of its own, and not behind a
returned card. The publication, a Stop's record and the fact's retirement all
read it; retirement reads the rows in position order inside the same
transaction as the write that took the last such card.
…the pause but keeps it

The last change retired a Stop's pause as soon as a returned card blocked every
paused card. Deleting that returned card then sent the cards behind it at once,
with no Resume — not what the user asked for.

The two rules are now separate. The pause is KEPT (recorded by a Stop, retired
in the same transaction as the write that takes the last one) while any waiting
card with no hold of its own exists, wherever it sits. It is PUBLISHED only
while such a card is not behind a returned one, so the header never offers a
Resume that sends nothing. Deleting the blocking card shows the pause again,
and the cards behind it wait for Resume or the user's next turn.
…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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 58a2f30d-12d9-42d0-bcb0-0b51c524301f

📥 Commits

Reviewing files that changed from the base of the PR and between 0b2a926 and ce5b984.

⛔ Files ignored due to path filters (1)
  • src/shared/rpc-contract/rpc-params-catalog.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing (50)
  • src/main/native-chat/agent-session-journal/journal-dispatch-reducer.ts
  • src/main/native-chat/agent-session-journal/journal-dispatch-settlement.test.ts
  • src/main/native-chat/agent-session-journal/journal-dispatch-settlement.ts
  • src/main/native-chat/agent-session-journal/journal-queued-messages.ts
  • src/main/native-chat/agent-session-journal/journal-reducer.ts
  • src/main/native-chat/agent-session-journal/journal-row-builders.ts
  • src/main/native-chat/agent-session-journal/journal-row-schema.ts
  • src/main/native-chat/agent-session-journal/journal-store-contracts.ts
  • src/main/native-chat/agent-session-journal/queued-message-delivered-echo.test.ts
  • src/main/native-chat/agent-session-journal/queued-message-holds.ts
  • src/main/native-chat/agent-session-journal/queued-message-pause-table.ts
  • src/main/native-chat/agent-session-journal/queued-message-retention.ts
  • src/main/native-chat/agent-session-journal/queued-message-schema.ts
  • src/main/native-chat/agent-session-journal/queued-message-settlement.ts
  • src/main/native-chat/agent-session-journal/queued-message-store.test.ts
  • src/main/native-chat/agent-session-journal/queued-message-table.ts
  • src/main/native-chat/agent-session-wire/agent-session-subscriber-frame-fields.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-client-delivery.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-mutation-plans.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-command.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-mutations.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-pause.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-publication.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-send.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-stop.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-wiring.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-withdrawn-draft.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-subscribers.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts
  • src/main/runtime/mobile-rpc-allowlist.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-gate-classification.test-fixture.ts
  • src/main/runtime/rpc/methods/structured-agent-session-queued-methods.ts
  • src/main/runtime/rpc/methods/structured-agent-session-schemas.ts
  • src/main/runtime/rpc/methods/structured-agent-session.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts
  • src/shared/agent-session-journal-types.ts
  • src/shared/agent-session-queued-message-wire.ts
  • src/shared/agent-session-wire.ts
  • src/shared/protocol-version.ts
  • src/shared/rpc-contract/structured-agent-session-params.ts
  • tests/e2e/cross-version-wire/structured-agent-session-host-fixture.ts
  • tests/e2e/cross-version-wire/structured-agent-session-surface-manifest.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/shared/protocol-version.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

Comment on lines +45 to +49
function restartPending(journal: PauseJournal): boolean {
return journal.queuedMessages
.list()
.some((row) => row.state === 'waiting' && row.hostInstance !== hostInstance)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

A card held by send_failed from the previous process should not pause cards typed after a restart.

restartPending counts every waiting row whose hostInstance differs from the current one, including rows held with holdReason === 'send_failed'. That hold is per card. Only an explicit Send releases it, and Resume does not.

Here is how it fails:

  • A card's conversion fails, and the card gets the send_failed hold.
  • The host restarts. The held card keeps the old hostInstance.
  • In the new process, the user queues a new card while a turn runs.
  • structuredQueuePause returns { reason: 'restarted' }.
  • oldestActionableQueuedMessage returns null, so the new card does not drain when the turn ends.
  • hasResumableQueuedMessage is true because of the new card, so the restarted pause header is published.

The user then sees a restart pause over a card that was written after the restart. The card sends only after Resume or after the user's next turn starts. The test "a restart over only a card held by its own failed send publishes no pause" shows the intent: a send_failed card by itself does not raise a restart pause.

Leave send_failed rows out of restartPending. Keep the legacy 'stopped' hold rows in the check, because only adoption clears that hold.

🐛 Proposed fix
 function restartPending(journal: PauseJournal): boolean {
   return journal.queuedMessages
     .list()
-    .some((row) => row.state === 'waiting' && row.hostInstance !== hostInstance)
+    .some(
+      (row) =>
+        row.state === 'waiting' &&
+        row.hostInstance !== hostInstance &&
+        // A failed conversion holds only its own card; an explicit Send releases it.
+        row.holdReason !== QUEUED_MESSAGE_PAUSED_SEND_FAILED
+    )
 }
import { QUEUED_MESSAGE_PAUSED_SEND_FAILED } from '../../../shared/agent-session-wire'

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues found in this round of commits — the queue-pause rework holds up under tracing; nothing in this delta needs changing.

Reviewed changes

Re-read the 10 commits pushed since the previous pullfrog review (0b2a926205..ce5b9840b3) plus the journal, reducer, publication and Stop paths they rework. This round replaces the per-row stop hold with a derived, queue-level pause:

  • Queue-level pause fact. A Stop (or a /clear carry) now records (reason, epoch, sequence) in a new queued_message_pauses table instead of hold_reason='stopped' on every card; structuredQueuePause derives the pause from that fact plus the journal, retireQueuePauseIfNothingHeld retires a fact that holds back no card, and liftPause compare-and-clears the exact fact it judged.
  • Person-turn origin. Submissions record origin: 'client' | 'host'; the reducer keeps latestPersonTurnSequence, so a turn a person asked for (a client send, or a card sent now) ends a Stop's or a restart's pause, while orchestration mail, restart continuations and the queue's own drain never do.
  • Resume. A new agentSession.queuedMessagesResume RPC (STRUCTURED_AGENT_SESSION_METHODS 31→32, mobile allowlist, cross-version manifest) lifts any pause; queuePause rides with queuedMessages on history pages and live frames, published only when Resume would actually send a card.
  • Restart pause derived from the sending process. A hand-off stamps host_instance at consume, so a card re-sent after a restart belongs to the process that sent it and raises no restart pause.
  • Cleanup. awaitUserSendTurn/userSendsAwaitingTurn, releaseStopHolds and the 'stopped' per-card marker are gone; the per-card hold is now only send_failed.

Tracing the pause derivation confirms the drain's sole conversion point (oldestActionableQueuedMessage) returns null whenever the queue reads paused, a pause fact can never be recorded without a card it holds back or outlive its cards, and the hasResumableQueuedMessage publication gate matches the drain's selection. The two replay-reconstruction threads from the earlier review remain open and are not re-raised here.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

#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.
Comment on lines +108 to +114
return (
sequence > spent.since &&
structuredAgentSessionPayloadFingerprint({
method: 'agentSession.send',
sessionId: state.sessionId,
fields: { body }
}) === spent.draft.fingerprint

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Identical echo withdraws waiting draft If a handed-off draft is rejected and the user later enters the same text directly in the agent, an unclaimed provider echo of that separate input can match the waiting draft. This code uses only the text fingerprint and journal order to treat the echo as proof of delivery, so it withdraws the card even though the draft was never delivered. Open-time repair can also withdraw multiple identical waiting drafts from one echo.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues in this round of commits — the /compact reconciliation holds up under tracing, and nothing in this delta needs changing. The two replay-reconstruction threads from the initial review are unchanged by these commits and remain open; they are not re-raised here.

Reviewed changes

Re-read the 4 commits pushed since the previous pullfrog review (ce5b9840b3..7f7df31bad: two merges of main including #23059, one test-shape commit, and one fix) plus the queue, journal and command code they rework. This round deletes this PR's own mid-/compact special case now that main makes /compact an ordinary queued message and then its own turn:

  • /compact is no longer a queue hold. StructuredQueueHold drops its command arm (now blocked / working / prompt); blocked is derived from the immediate path's structuredAgentSessionSendBlock (rewind in doubt, clear in doubt, cleared source), and a compaction holds the queue as working through isStructuredAgentSessionMainAgentWorking.
  • Lane machinery deleted. structured-conversation-command-lane.ts (compactInFlightContext, conversationOperationWaitRefusal), the draftOnly send flag, and the /compact branch in StructuredConversationCommandController.send are gone; controller.pending now holds only a clear.
  • Steer during /compact is accepted and waits. Send-now overrides working, so a card sent during a compaction becomes a submission at once; the delivery loop refuses to hand over while structuredAgentSessionCommandRunning(journal) holds and a child exists, so it goes out when the compaction ends.
  • Compaction settlement read through the send-result union. 7f7df31bad narrows settled.value before reading submission, so a non-submission arm yields the "not started" reply instead of a type hole.
  • Journal refactors from the merge. Submission folding moved into journal-submission-fold.ts; the journal's restore now chains queuedMessages.repairAndPruneAtOpen(), and appendRow gave way to rowWriter.append.
  • Test alignment. turnScope added to appendItem/resolveDispatch calls to match main's required field, and a finishCompact rig helper ends a compaction's turn.

Tracing the reconciliation: the drain's sole conversion point still returns null while the queue is paused; a compaction's client-origin submission both gates the queue as working and, once handed over, lifts a Stop's pause; and the loop's command gate keeps a Steer from overtaking a running compaction.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Status: ready to merge (merge this first, then #23731 and #23736). Head 4da7df5b04d, CI all green, no conflicts.

  • Why it conflicted: fix(native-chat): turn facts come from the turn record, and /compact is a message the chat sends #23059 landed on main and made /compact an ordinary queued message that runs as its own turn. This PR's own /compact handling (a separate holding lane, and Steer refusing during a compaction) was removed, since main now covers it. What changes for you: a message sent during /compact waits as a card and sends when the compaction ends, and Steer during /compact is accepted and goes through when it ends. The description is updated.
  • Also merged: refactor(shared): bring constants.ts back under the max-lines limit #23923, which fixed main's line-limit lint failure on src/shared/constants.ts. That failure had turned this PR's first CI run red.
  • Checks: a separate review of the main merge found nothing above minor, and dropped nothing from either side. The node typecheck and 564 of 566 tests across 52 files pass locally. One failure was an unrelated timeout that passes alone. The other is below. CI: typecheck, all five unit-test shards, packaging (macOS, Windows), cross-version wire compatibility and static analysis all pass.
  • To watch: structured-agent-session-claude-compact-stop.test.ts, a new test on main, failed 3 of 46 local runs on this branch, all while the machine was heavily loaded, and 0 of 34 on main. No cause in this PR was found, and it passed in CI.
  • Behaviour to note: after a Stop, a /compact you send counts as a turn you asked for, so it lifts the pause, and the waiting cards send once the compaction ends.

@brennanb2025
brennanb2025 merged commit 29c49ae into main Sep 29, 2026
35 checks passed
brennanb2025 added a commit that referenced this pull request Sep 29, 2026
#23524 landed as the squash 134a220, whose change equals the branch
this PR was stacked on, so the merge uses that branch head (c9b008b)
as its base.

Resolved against #23726 (host-owned message queue):
- /clear's draft carry now runs after the commit and opens the new
  conversation only when there are drafts to carry, since the new
  conversation has no transcript until something opens it. It stays
  bookkeeping: a failure, or a crash before it, leaves the drafts on the
  cleared source, where nothing sends them.
- The subscriber hook serves the remembered command list alongside the
  queue publication.
brennanb2025 added a commit that referenced this pull request Sep 29, 2026
…ow is

Three queue tests from #23726 are red on main 29c49ae itself:
- Two expected an older build's unconfirmed ("prepared") /clear to hold
  sends and drafts back. #23524 made such a clear inert, because it
  changed nothing; Send-now and the drain now go past it, like any send.
- One held /clear in flight by holding the new conversation's start,
  which /clear no longer makes. It now holds the one store write, and
  still sees a send refused and no draft left on either conversation.
brennanb2025 added a commit that referenced this pull request Sep 29, 2026
Brings in the host-owned queue for mid-turn messages (#23726) and the /clear and
Codex rewind unlock (#23524), expressed on the one host journal database:

- Queue transactions run through runJournalTransaction on the shared connection,
  and every appended row keeps the draft hooks inside that same transaction.
- The open-time draft repair writes nothing for a chat with no draft, so a chat
  whose first-use copy is owed stays uncopied until its first real write.
- A newer Orca's journal takes no new draft, and Send-now, Delete and Resume on
  it refuse with the same update words a send gets.
- Main's queue tests open the shared database; a table-healing case restarts
  the host around its edit, since the table is ensured when the database opens.
brennanb2025 added a commit that referenced this pull request Sep 29, 2026
…hed clear holds nothing (#23940)

#23726 and #23524 merged minutes apart. #23726's queue gate takes its "blocked"
answer from the shared send check, which #23524 changed: a /clear that never
committed changed nothing the chat reads, so it refuses no send. Three #23726
tests still seeded a "prepared" /clear record and expected it to hold the queue,
and the /clear carry passed a possibly-undefined replacement id where #23726's
helper takes a string, so main failed its unit tests and tc:node.

The gate code already follows the new rule; the tests now say it: a /clear an
older build left prepared holds no draft and Send-now sends it, and a send made
while a /clear is starting its replacement is still told to wait. The carry reads
the replacement id from the ids it already checked.
brennanb2025 added a commit that referenced this pull request Sep 29, 2026
Brings in #23940, which fixed on main the same three #23726 queue tests
this branch had adapted. Resolution:
- The two tests where an older build's unfinished /clear holds nothing
  take main's version.
- The in-flight /clear test keeps this branch's version: it holds the one
  store write, because a /clear no longer starts the new conversation's
  agent, so holding that start would never engage.
- The draft carry keeps this branch's shape: it runs after the commit and
  opens the new conversation, under its own lock, only when there are
  drafts to carry.
brennanb2025 added a commit that referenced this pull request Sep 29, 2026
…3789, #23812, #23852) into brennanb2025/codex-home-single-writer

#23744 moves every agent's folder pre-trust into the PTY spawn hook (and
the structured Codex chat create), removing Codex's launch-prep and
resume-prep trust writes. markCodexProjectTrusted now takes the config
files to write.

Conflicts:
- agent-trust-presets.ts: #23744's configFiles signature and nested lane
  acquisition, with this branch's lane rationale (hook installs read and
  rewrite across awaits); main's "capture->restore window" wording
  described the grant snapshot this branch removed.
- agent-trust-write-deadline.ts: the Codex writer waits only on
  managed-home installs' inline grant; the real-home background approval
  holds no lane. #23744's SSH/SFTP writer is gone, so it is no longer
  named; its 1.5 s short deadline is kept.
- codex-trust-config-concurrent-launch.test.ts: this branch's tests
  (a launch's write lands while a grant session runs; the renamed
  sibling-failure case) with #23744's new call signature.

Outside the conflicts: codex-launch-during-real-home-grant.test.ts passed
launchAgent/workspacePath into launch prep, which #23744 removed; its
launch helper now runs launch prep and then applyAgentWorkspaceTrust, as
the spawn builder does, so it still proves a launch and its project trust
land while the real-home approval hangs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant