Skip to content

fix(native-chat): queued messages carry on in order after any turn, and nothing sends by itself after a restart - #24586

Merged
brennanb2025 merged 40 commits into
mainfrom
brennanb2025/chat-queue-continues-after-stop
Oct 7, 2026
Merged

brennanb2025 merged 40 commits into
mainfrom
brennanb2025/chat-queue-continues-after-stop

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 38 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2005 $\color{#cf222e}{\Huge{\mathbf{−}}}$​282 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1723
Prod 53 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​835 $\color{#cf222e}{\Huge{\mathbf{−}}}$​239 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​596

ELI5

In a native chat you can queue follow-up messages while the agent works. Each one waits as a card above the message box and sends by itself, in order, when the agent finishes. After you press Stop or run /clear, Orca pauses the queue: a row above the cards says why ("Queue paused because you interrupted"), with a Resume button.

What was wrong.

  • With the queue switched on, queued messages stayed stuck after a restart even when the chat carried on. If Orca restarted while messages were queued, the queue showed "Queue paused because Orca restarted". When you resumed the chat, Orca sent the agent its own "carry on" message and the agent picked up where it stopped. When that turn finished, the queued messages still did not send. They sat there until you noticed and pressed the queue's Resume.
  • Quitting with messages queued could lose the chat's "carry on" offer. While Orca was shutting down, the queue still tried to send the next message. Orca refused it during the shutdown, and that refused message made Orca think the chat had already moved on, so after the relaunch the chat no longer offered to carry on. Main has the same bug today.
  • After a Stop, Orca's own messages did not set the queue going again. When Orca itself started a turn (for example an orchestration message), the agent went back to work and your queued messages kept waiting.
  • The chat flickered between queued turns. As the queue sent one card after another, the send button and the "working" status blinked to idle for a moment in between.
  • Nothing asked before you sent past a paused queue, and there was no Resume on the empty message box.

How the queue works now.

  • While the agent works: cards wait and send in order when it finishes. Each has Steer, Delete and More. Nothing flickers between them. Orca's own messages to a busy chat (from coordinating agents) can wait as cards too, under the same rules.
  • After Stop: the cards that were waiting stay paused under "Queue paused because you interrupted", along with anything queued behind them, so the order never changes. If the queue was empty, a correction you type right after Stop sends as soon as the stopped turn ends.
  • After a restart nothing sends by itself. When you resume the chat or send a message, your queued messages follow it in order.
  • In a chat that receives Orca's coordinator messages, an unread one that Orca sends again after a restart counts as a turn, so your queued messages follow it.
  • After /clear: the cards carried into the new conversation stay paused under "Queue paused after you cleared the conversation".
  • What sends paused messages again: Resume, or any new turn the agent accepts, whoever sent it: your message, Steer on a card, or one of Orca's own messages (an orchestration message, a launch prompt). The paused row goes as soon as that turn is on its way, and comes back if the agent refuses it.
  • Resume: on the paused row, and (new) as the main button of the empty message box while nothing is running. It sends the paused cards in order.
  • "Send message?" (new): when you send a message over a paused queue while nothing is running, Orca first asks "You are about to send a message. Do you want to clear the 2 messages previously queued?". Clear queue deletes the cards, then sends yours. Send message sends yours first, and the cards follow it in order. Esc sends nothing. If the pause ends while the question is open (for example Orca's own message starts a turn), the question closes and your text stays in the box.

Who sees this: the message queue ships switched off today, so none of this reaches users until it is switched on.

What Changed

Before (main).

  • After a restart: "Queue paused because Orca restarted" with Resume. Only a turn you sent, or Resume, released the cards; Orca's "carry on" turn did not.
  • A quit with messages queued could withdraw the chat's "carry on" offer (a bug main still has).
  • After Stop or /clear: only a turn you sent, or Resume, ended the pause. The paused row stayed up until the agent accepted that turn.
  • Between queued turns, the chat blinked to idle.
  • Sending over a paused queue asked nothing; the empty message box offered a disabled Send.

After.

  • After a restart nothing sends by itself. When you resume the chat or send a message, your queued messages follow it in order. The cards look like ordinary waiting cards, and Steer on one sends it now, with the rest following.
  • A quit keeps the chat's "carry on" offer.
  • After Stop or /clear: the same paused row with Resume, ended by Resume or any accepted turn. The row goes as soon as such a turn is on its way.
  • Between queued turns, the chat stays working: Stop (disabled until there is a turn to stop) and Steer throughout.
  • Over a paused queue, with nothing running and no question from the agent waiting: the empty message box offers Resume, and sending a message asks "Send message?" first. The question closes, sending nothing, if the pause ends while it is open.

Mechanism.

Any accepted turn (host). The reducer keeps latestAcceptedTurnSequence: the submission row of the latest turn the agent accepted, from any sender. Stop's and /clear's pauses end once such a turn comes after them, or on Resume. Main counted only submissions recorded as a person's; the submission's origin field and the host-local userSend flag that fed it are removed. Journals written by earlier builds keep an origin key that nothing reads.

After a restart (host). While a card written by an earlier Orca process waits, every waiting card waits until a turn is accepted in this conversation, from any sender. Meanwhile the host publishes no pause (a Stop's or /clear's row from before the restart included) and no next card, so the cards look like ordinary waiting cards and opening, reconnecting or reopening the chat sends nothing. The 'restarted' reason is removed from the wire type, the desktop row text, the six locale catalogs and the mobile label; no user ever saw that row, because no client can queue a card without the queue's capability.

Quit (host). Host teardown now stops the queue (StructuredAgentSessionQueuedMessageDrain.dispose, called where teardown already stops conversation delivery), so nothing is handed off while the host quits. Before, a card handed off during the quit was refused, and its refused send counted as the chat having moved on past its restart offer.

The paused row while a turn is on its way (host). The host publishes no pause while a turn sent after the pause began is still waiting for the agent's answer (queuePauseLiftOnItsWay): your message, Steer on a card, or Orca's own. That turn's acceptance lifts the pause; a refusal shows the row again. A send made before a Stop never counts. The host decides this, so it covers your message, Steer and Orca's own messages on every client, the phone included.

Strict order (host, unchanged from main). The queue sends the oldest waiting card, and stops at the first paused or refused card; nothing overtakes it.

The next card (host and client). The queue publication carries nextQueuedMessageId: the card the queue will send next, from the queue's own pick through its own gate (nextStructuredQueuedMessage, which the queue step itself calls). It is empty whenever the host would not send: a pause, a returned card ahead, a waiting question, a conversation a /clear replaced, a rewind whose outcome is unknown. The desktop reads "about to send" from it, so the working status, the buttons and the cards do not flip to idle between a turn's end and the next card.

Resume on the message box (client). nativeChatComposerPrimaryAction picks one primary button: Stop while a turn runs; Resume when the box is empty and Resume is offered; otherwise Send. It calls the existing agentSession.queuedMessagesResume, and shares one in-flight guard with the row's Resume.

"Send message?" (client). While the message box offers Resume, the controller also offers queueHold (the card count and a clear that deletes each shown card in turn, stopping at the first failure). useNativeChatHeldQueueComposerSend wraps the composer's send, so Enter and the send button open NativeChatQueueSendConfirmDialog instead of sending. Commands Orca runs itself (/clear, /compact, /model) skip it. The waiting choice remembers the hold it was asked under: if that hold is gone (the pause ended, or the cards changed), the dialog closes and no choice sends; the next Enter sends or asks again. A choice is taken once, and only the sent text leaves the composer once the message is accepted (main's draft rule).

When the message box offers Resume and the question (client). Only while the paused row shows, nothing is running, and no question or approval from the agent is waiting. The row's own Resume stays available. While a turn runs, Enter queues as usual behind the paused cards. A waiting question holds the host's queue too; the message box shows beside one only when this build cannot answer it.

Why

The goal is the normal queue behaviour: it does not matter who sent a message to the agent, and the rules stay simple. A restarted chat sends nothing until it carries on, by Orca's carry-on or your own message, and then the queued messages follow in order.

Alternatives considered:

  • Letting a card queued after a Stop send ahead of the paused ones. Rejected for strict order: it needs a per-card pause field on the wire and shows a list whose order differs from the send order.
  • Counting only your own turns (main). Rejected: Orca's "carry on" after a restart then never released the queue.

Differences from the common pattern

Each item says what Orca does and whether it is intended or temporary.

  • After a restart nothing sends by itself; the chat's next turn, yours or Orca's carry-on, runs first and the queued messages follow. Intended. Safe because nothing reaches the agent that the person did not start or resume, and nothing is lost: the cards stay in place, and Steer or Delete still work.
  • A queued card sends only while its conversation is open in Orca; a closed chat's cards wait until it is opened. Intended, unchanged from main. Safe because the cards are kept, not dropped, and the next open sends them under the same rules.
  • A card typed while a stopped turn is still winding down waits behind the paused cards. Intended. Safe because the order never changes, and Steer on that card sends it at once and releases the rest.
  • Resume on the message box and "Send message?" are not offered while a turn runs or a question from the agent waits; the paused row keeps its Resume. Intended. Safe because the row still offers Resume, and Enter queues as usual behind the paused cards.
  • Clear queue deletes every queued card shown, including any not paused (for example one sent back after a refusal). Intended. Safe because the question states the count it clears, and that count is the list you see.
  • Commands Orca runs itself (/clear, /compact, /model) send without the question. Intended. Safe because they are not messages to the agent and leave the cards as they are.
  • A returned (refused) card still blocks the ones behind it. Intended, unchanged from main. Safe because it says why it came back, and each card keeps Send, Edit and Delete.
  • The phone shows the paused row and Resume, but has no Resume on the message box and no "Send message?", and it still drops to idle for a moment between queued turns because it does not read the next card yet. Temporary. Follow-up: a mobile change that reads nextQueuedMessageId and adds the message-box Resume and the question; not yet opened.

Linked Issue

N/A (internal native-chat queue behaviour)

Visual Proof

The message queue ships switched off today. It was switched on in the test build only, so these screens show what users get once it ships.

Captured on the Windows test machine in an isolated, hidden test build with a stand-in Claude agent, at 7c561ee503b. Every restart below is a real quit (the app's own Quit) followed by a relaunch of the same profile. A long reply was running, and "Second message" and "Third message" were queued.

After a restart

  • Nothing sends by itself. After the relaunch, Orca offers to resume the interrupted chat. There is no paused row, the queued cards look as usual (Steer, Delete, More), and the message box shows Send. Nothing was sent during a 60-second wait.
    queued-before-quit
    chat-after-relaunch
    idle-60s
  • Resume the chat. Orca's "carry on" turn runs first, then "Second message", then "Third message". No paused row or question appears.
    restart-offer
    after-resume
  • Send your own message. "Fourth message" goes straight to the agent with no question, then "Second message" and "Third message" follow.
    fourth-sent
    after-own-message
  • Steer on a card. Steer on "Third message" sends it now, then "Second message" follows.
    after-steer-restart

After Stop

  • The paused row. "Queue paused because you interrupted" appears with Resume; the cards keep Steer; the empty message box shows Resume (play icon).
    after-stop
  • Resume. "Second message" then "Third message" send in order, and the row doesn't come back.
    after-resume-stop
  • Sending a new message asks first: "Send message?" / "You are about to send a message. Do you want to clear the 2 messages previously queued?"
    send-confirm
    • Send message: the new message runs, then the queued ones follow in order.
      after-send-message
    • Clear queue: the queued cards are removed and the new message sends once.
      after-clear-queue
    • Esc: nothing is sent and your text stays in the message box.
  • Steer on a card after Stop. It sends now. The row goes in the same update as the turn starts, and the rest follow.
    after-steer-stop

The build shows no other new pop-up, banner or label. The "Enjoying Orca?" prompt seen in some shots is existing Orca UI.

Testing

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

A real quit (new structured-agent-session-queued-quit.test.ts): cards queued behind a running turn, then the host's own quit teardown and a relaunch as a new process. Nothing was handed off during the quit, the chat still offers to carry on, nothing sends by itself, and carrying on sends Orca's carry-on, then A, then B.

After a restart (structured-agent-session-queued-card-holds.test.ts, through the real host with a test client folding every update as the chat does):

  • idle: nothing is handed off across opening, commits and a close and reopen; no pause and no next card are published; the chat reads idle with ordinary waiting cards (no caption, Steer), no row, no Resume, no question;
  • Steer on a card sends it now, and the rest follow it in order;
  • a Stop's row from before the restart is not shown, and Orca's carry-on releases every card, in order;
  • a card written after the restart waits while any card from before it waits; once those are deleted it sends like any queued card;
  • Orca's carry-on goes straight to the agent, then A, then B (B only after A's turn), with every update reading working and no row or question at any point;
  • your own message, sent with the composer's queue delivery, goes straight to the agent, then A, then B;
  • a hand-off the quit cut short (recorded, never given to the agent) goes back to waiting in first place, and nothing sends;
  • an epoch replacement (a rewind or a legacy import) before any turn: the cards stay held, nothing published.

The paused row while a turn is on its way (same file): your message over a paused queue, and Steer on a paused card, each hide the row from the moment the host records them; the cards follow once the agent accepts; a refusal brings the row back over the cards still paused.

What sends paused messages again.

  • queued-message-pause.test.ts: any turn sent after the Stop and accepted lifts it, Orca's own mail included; one sent before the Stop does not; any accepted turn lifts /clear's pause, a launch prompt included.
  • structured-agent-session-queued-stop-row.test.ts: Orca's mail accepted after a second Stop releases the cards; your send made before that Stop does not.
  • structured-agent-session-queued-messages.test.ts: an Orca-sent turn after a Stop lifts it once accepted; after a restart nothing sends or shows across a reopen, and your next send releases the cards.
  • structured-agent-session-queued-pause-lift.test.ts and structured-agent-session-queued-stop-row.test.ts: after a restart a Stop from before it shows no row; a turn, or a Resume from an older client, releases the cards.

Strict order. queued-message-pause.test.ts (main's: the queue's own send refuses a newer card behind a paused one); structured-agent-session-queued-stop-row.test.ts and structured-agent-session-queued-card-holds.test.ts (a card typed while the Stop lands waits behind the paused one; Resume sends both in order).

Desktop client.

  • use-structured-agent-session-queued-messages.clear.test.tsx: the question is not offered while a turn runs, though the row shows.
  • native-chat-queue-send-confirm.test.tsx: the open question closes when the pause lifts under it, sends nothing and keeps the draft; the next Enter sends, and a later pause does not bring the old question back (with the previous dialog code this test fails); only the sent text leaves the composer.
  • use-structured-agent-session-queued-messages.resume.test.tsx: no message-box Resume or question while a question from the agent waits; an idle chat with cards and no published pause shows no row, Resume or question.
  • structured-agent-session-queued-cards.test.ts, NativeChatQueuedMessageList.test.tsx: a paused queue holds every card under one row; no row without a published pause.
  • native-chat-queue-send-confirm.test.tsx, native-chat-composer-field-resume.test.tsx, native-chat-composer-primary-action.test.ts, use-structured-agent-session.queued-gating.test.tsx: the message-box Resume, "Send message?" (each choice, taken once, Clear queue's ordering and failure) and the no-flicker working state.

Ablations at 7a61051caf2, each run once and restored (tree verified clean after each).

  • No turn ends a pause: 25 tests fail (including Steer after a restart).
  • A card overtakes a paused one: 3 fail.
  • The restart hold never ends: 7 fail.
  • No restart hold: 16 fail.
  • A rewind's restated Resume ends the restart hold: 2 fail.
  • A Stop's row shown after a restart: 4 fail.
  • The queue not stopped at quit: 1 fails (the quit test).
  • The row kept while a turn is on its way: 9 fail.
  • The question offered while working: 3 fail.
  • The message-box Resume offered beside a waiting question: 1 fails.
  • Every card shown as paused: 1 fails.

Runs (vitest, explicit files). At 7cc68c2e68d (main merged at 5b8a982f8f7): 505 files, this PR's list plus every test main changed under native chat, the runtime and shared agent-session code, including main's new rewind tests and every test of the chat session hook. 5044 tests passed, none failed. 34 files could not load locally because main's stream-json dependency is not installed in this checkout (CI installs it). The mobile tests could not run locally (no mobile install in this checkout); CI covers them. The ablations above ran at 7a61051caf2; the paused-row ablation was re-run at ac624ba91db, and the quit ablation at 738270b49b6.

Checks. At 7cc68c2e68d: the web, node and both CLI typechecks (run without the incremental cache) report only main's stream-json/stream-chain modules, which this checkout lacks. oxlint, oxfmt, the changed-code quality gate (React Doctor: no new findings) and the three localization checks pass. An earlier merge commit, 4b50771f30e, failed CI's typecheck on one test that still passed a composer prop main had renamed; 439545e1242 fixed it.

CI. On 7cc68c2e68d: 18 pass, 14 skipped, none failed.

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

  • Security: no change.
  • Cross-platform: nothing platform-specific.
  • SSH/remote: one optional field, nextQueuedMessageId, beside the queued list on the existing stream and history replies, with no new message type; clients read it only when present. The queue's rules run wherever the conversation's host runs. Resume and Clear queue use the existing requests.
  • Localization: "Send message?", its question (singular and plural), "Clear queue" and "Send message" are new in all six languages; the restart row's sentence is removed from all six.
  • Mobile: the restart label is removed. Two test files are edited: a table row and an assertion that used it are dropped, and one test now uses the Stop reason.
  • Backwards compatibility:
    • Journals from earlier builds may carry a submission origin; it is ignored.
    • The queue ships switched off, so no released client and host pair runs any of this yet.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

A Stop, a restart or /clear holds the queued cards. The hold stays; only the
header row naming why, and its Resume button, go. A held card shows no
caption, and its own Steer, or any new message, releases the queue.
Steer vs Send now follows whether a turn is running, not the card's hold,
so a card held after a Stop, a restart or /clear reads Send.
… behind them

A card queued after a Stop (or written after a restart or /clear) sent only
once the cards held before it were released; with no header to explain or
release the hold, it sat silently. The next sendable card now skips held
cards; a returned card still blocks what is behind it.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 2, 2026 19:49
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 91382530-8be7-4169-ac57-93c1b95d2f0f
📥 Commits

Reviewing files that changed from the base of the PR and between 4ef532e and 31995e4.

📒 Files selected for processing (46)
  • config/tsconfig.node.json
  • src/main/native-chat/agent-session-journal/queued-message-pause.test.ts
  • src/main/native-chat/agent-session-journal/queued-message-pause.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-queued-card-holds.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-publication.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-stop-row.test.ts
  • src/renderer/src/components/native-chat/NativeChatComposer.tsx
  • src/renderer/src/components/native-chat/NativeChatComposerActions.test.tsx
  • src/renderer/src/components/native-chat/NativeChatComposerActions.tsx
  • src/renderer/src/components/native-chat/NativeChatComposerField.tsx
  • src/renderer/src/components/native-chat/NativeChatQueuedMessageCard.tsx
  • src/renderer/src/components/native-chat/NativeChatQueuedMessageList.test.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx
  • src/renderer/src/components/native-chat/native-chat-composer-field-resume.test.tsx
  • src/renderer/src/components/native-chat/native-chat-composer-primary-action.test.ts
  • src/renderer/src/components/native-chat/native-chat-composer-primary-action.ts
  • src/renderer/src/components/native-chat/native-chat-composer-types.ts
  • src/renderer/src/components/native-chat/structured-agent-session-queued-cards.test.ts
  • src/renderer/src/components/native-chat/structured-agent-session-queued-cards.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.resume.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session-transport-state.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session.queued-gating.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
  • src/shared/agent-session-queued-message-wire.ts
  • src/shared/agent-session-wire.ts
  • src/shared/protocol-version.ts
  • src/shared/structured-agent-session-coalescer.test.ts
  • src/shared/structured-agent-session-coalescer.ts
  • src/shared/structured-agent-session-queue-publication-field.ts
  • src/shared/structured-agent-session-reducer.queued-messages.test.ts
  • src/shared/structured-agent-session-reducer.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/zh.json
  • src/renderer/src/i18n/locales/en.json
  • src/main/native-chat/agent-session-journal/queued-message-table.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.


📝 Walkthrough

Walkthrough

Queued cards now retain their client or host origin, and queue drains use that origin when submitting cards. Queue selection skips held waiting cards but still stops at returned cards. Queue publication reports per-card holds and the next sendable card. The renderer uses queue and turn state to select Send, Steer, Stop, or Resume, and no longer displays a queue-pause row.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 31995

The queued-message hold and Resume changes show no confirmed production defect. One test double could mislead a future test about whether a queued card shows Send or Steer. Fix it in a small follow-up; it need not block merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 31995

The inspected controls preserve holds and prevent duplicate submissions while allowing newer instructions to proceed. No introduced security issue was demonstrated, but compatibility and recovery coverage is incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is queued instruction execution within the addressed conversation: changing hold eligibility can start agent work and release its earlier cards. Storage and consumption are session-scoped. The reviewed paths do not establish the maximum downstream asset or credential privileges of that agent work.

Trust Boundaries and Controls

  • observed — The host derives heldBy from journal pause state. Renderer Send and Resume requests do not submit that classification as authority. Both enter the serialized mutation-admission path with caller identity and an envelope; Send rechecks current host gates, and Resume records a journal transition before draining resumes. This counters a stale-renderer-state bypass in the inspected path without proving end-to-end transport authentication.

Resilience and Maintainability Implications

  • observed — A pre-consumption drain failure leaves the card waiting and attempts to persist a send-failed hold rather than automatically retrying. Returned cards block subsequent selection. Before selecting another card, the drain attempts to reconcile owed settlement and delivered-echo bookkeeping, supporting containment of repeated or out-of-order instruction execution.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 80 functions across 59 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the queue-ordering changes and the restart behavior that prevents automatic sends.
Description check ✅ Passed The description covers the required sections with detailed behavior, rationale, visual proof, testing, compatibility notes, and completed checklist items. The linked-issue section gives the stated int…
Full details: Docstring Coverage

Explanation

Docstring coverage is 47.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 80 functions across 59 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@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: 720fa501-70e4-4259-890d-cf90f276083a

📥 Commits

Reviewing files that changed from the base of the PR and between 306b457 and faaefcc.

📒 Files selected for processing (22)
  • src/main/native-chat/agent-session-journal/journal-queued-messages.ts
  • src/main/native-chat/agent-session-journal/queued-message-pause.test.ts
  • src/main/native-chat/agent-session-journal/queued-message-pause.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-pause-lift.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-stop-row.test.ts
  • src/renderer/src/components/native-chat/NativeChatQueuedMessageCard.tsx
  • src/renderer/src/components/native-chat/NativeChatQueuedMessageList.test.tsx
  • src/renderer/src/components/native-chat/NativeChatQueuedMessageList.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx
  • src/renderer/src/components/native-chat/structured-agent-session-queued-cards.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.resume.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
💤 Files with no reviewable changes (1)
  • src/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.resume.test.tsx

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

stop: mocks.stop,
queuedMessages: {
cards: mocks.queuedCards,
turnRunning: mocks.turnId !== null,

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

🔎 Supported by static analysis

🏁 Script executed:

rg -n 'isWorking|turnId|turnRunning' src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx src/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.ts src/renderer/src/components/native-chat/NativeChatStructuredSession*.test.tsx

Repository: stablyai/orca

Length of output: 3759


🏁 Script executed:

set -eu
printf '%s\n' '--- harness structure and relevant slices ---'
ast-grep outline src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx
sed -n '40,215p' src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx
sed -n '300,340p' src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx
printf '%s\n' '--- harness setters/usages ---'
rg -n -C 4 'mocks\.(isWorking|turnId|canStop)|set.*Working|turnRunning|queuedMessages|Send|Steer' src/renderer/src/components/native-chat --glob '*test*' --glob '*.tsx'
printf '%s\n' '--- production queued controller and callers ---'
sed -n '1,190p' src/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.ts
rg -n -C 8 'useStructuredAgentSessionQueuedMessages|queuedMessages|turnRunning|isWorking' src/renderer/src/components/native-chat --glob '*.tsx' --glob '*.ts'

Repository: stablyai/orca

Length of output: 45665


🏁 Script executed:

set -eu
h='src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx'
printf '%s\n' '--- harness exact lines ---'
sed -n '70,210p' "$h"
sed -n '310,335p' "$h"
printf '%s\n' '--- assignments and queued assertions in structured-session tests ---'
rg -n -C 6 'mocks\.(isWorking|turnId)\s*=|queuedCards|turnRunning|Steer|Send' \
  src/renderer/src/components/native-chat/NativeChatStructuredSession*.test.tsx
printf '%s\n' '--- production controller binding ---'
rg -n -C 10 'useStructuredAgentSessionQueuedMessages|queuedMessages|turnRunning|isWorking' \
  src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx \
  src/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.ts \
  src/renderer/src/components/native-chat/use-structured-agent-session*.ts

Repository: stablyai/orca

Length of output: 42172


🏁 Script executed:

set -eu
printf '%s\n' '--- queued pane consumers ---'
rg -n -C 10 'turnRunning|Send|Steer' src/renderer/src/components/native-chat \
  --glob 'NativeChat*Queued*' --glob 'NativeChatStructuredSession.tsx' --glob '*goal-dock.test.tsx'
printf '%s\n' '--- exact queued-card files ---'
git ls-files 'src/renderer/src/components/native-chat/*Queued*' 'src/renderer/src/components/native-chat/*queued*'

Repository: stablyai/orca

Length of output: 41696


Derive turnRunning from mocks.isWorking.

When mocks.isWorking is true and mocks.turnId is null, the harness currently sets turnRunning to false. The queued-message pane then shows Send, while production passes isWorking and shows Steer. Use the same state contract in the mock.

Suggested fix
-              turnRunning: mocks.turnId !== null,
+              turnRunning: mocks.isWorking,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
turnRunning: mocks.turnId !== null,
turnRunning: mocks.isWorking,

@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 critical issues — minor suggestions inline.

Reviewed changes

  • Queue-paused header and Resume removed — NativeChatQueuedMessageList no longer renders the status row or its button; the controller drops pause/resume/resuming, and the now-unused copy leaves all six locales.
  • Send vs Steer follows the turn — queuedMessageCardSendNow(card, turnRunning) shows Steer only while a turn runs; turnRunning is threaded from the session's isWorking.
  • Drain sends past held cards — nextSendableQueuedCard skips held cards instead of stopping at the first, while a returned card still blocks; the drain pick, its consume guard, and admission all read the one function.
  • Tests — header-removal, Send-vs-Steer, and send-past-held cases added or rewritten; the client-side Resume test removed.

The host-side change holds up under adversarial tracing: no sequence sends a card past a returned one, strands a waiting card, or lets the queue's own origin: 'host' send lift a pause.

ℹ️ Nitpicks

  • src/renderer/src/components/native-chat/NativeChatQueuedMessageList.tsx:37 — the outer box's divide-y divide-border is now a no-op (the box encloses only the ul, which carries its own divide-y); the updated test already moved that class expectation to the list. Drop it from the box.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

… held cards follow it

After a Stop, a card queued later sent past the held cards, but the queue
recorded that send as Orca's own turn. It never ended the Stop's pause, so
the held cards then waited forever with nothing on the card saying why.

A queued card is always something the person wrote: only the client send
RPC may now create one. The queue's send of it is therefore recorded as the
person's turn, which ends the Stop's pause once the agent takes it, and the
held cards then drain in order.

@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 code issues found in the new commits — one description note.

Reviewed changes

Reviewed the delta since pullfrog's prior review (faaefcc → f91c1ae), which reverses the drain's own send to count as the person's turn.

  • The queue's own drain now sends as the person's turn — the drain's appendSubmission writes origin: 'client' (was host), so once the provider accepts a card the queue sent past held cards, notePersonTurnAccepted advances latestPersonTurnSequence and the held cards follow it.
  • Only a person's send becomes a card — maybeQueueStructuredAgentSessionSend now requires the host-local userSend flag (set solely by the client agentSession.send RPC), so a host-internal send that asks to queue runs immediately instead of writing a card.
  • Tests and docs updated — the drain's consume seam now identifies its claim by settledByOp === null rather than origin === 'host', and the stop/restart/pause-lift tests assert the held cards follow the drained one.

I traced the origin change through notePersonTurnAccepted, journalQueueStopHolds, restartEnded, and the resume path; the drain's submission is indistinguishable from a person's queued card, which is the commit's stated intent, and no reader outside the pause derivation consumes a submission's origin.

ℹ️ The PR description still documents the behavior this commit reverses

The body's Mechanism bullet (The queue's own send of that later card is not a person's turn, so the earlier held cards stay held.), its "Differences from the common pattern" follow-up, and its Testing bullets ("...which stay held", "...the older one stays held") all describe the pre-f91c1ae behavior. After this commit the queue's own send lifts the hold and the earlier cards follow, so the description now contradicts the code and the tests it names no longer assert what it says.

Technical details
# Update the PR description to match `f91c1ae`

## Affected sites
- PR body, "The queue sends past held cards (host)" bullet — "The queue's own send of that later card is not a person's turn, so the earlier held cards stay held."
- PR body, "Differences from the common pattern" — "Sending a later card from the queue does not release the held cards ahead of it. Temporary." (listed as not done)
- PR body, Testing — "a card queued after the Stop sends past the cards it holds, which stay held" and "the older one stays held"

## Required outcome
The description and its testing section should state that a drained card's send is now the person's turn, so the cards it passed over follow it once that turn is accepted.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

…ue-continues-after-stop

# Conflicts:
#	src/renderer/src/i18n/locales/en.json
#	src/renderer/src/i18n/locales/fr.json
#	src/renderer/src/i18n/locales/ja.json
#	src/renderer/src/i18n/locales/ko.json
#	src/renderer/src/i18n/locales/zh.json

@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 — this run reviewed only the merge of main into the branch; the prior review stands.

Reviewed changes

Reviewed the delta since pullfrog's prior review (f91c1ae → 7a28cd5), a Merge remote-tracking branch 'origin/main' with conflicts in five locale files.

  • main merged in, PR changes intact — main's drain-barrier removal, journal write-queue typing, and new locale copy are all present, while the PR's userSend / origin: 'client' host changes and its queuePaused* / resume locale removals survive unchanged. Confirmed by diffing the merge against each parent.
  • Locale conflict resolution kept both sides — the five conflicted files (en/fr/ja/ko/zh) keep the PR's removals and add main's structuredSessionHostDeclined copy; es.json auto-merged identically. verify:localization-catalogs passes.
  • Focused suites are green — the host queue/stop/pause suites (95 tests), the renderer queued-list suite, and the two main-modified stop-row suites (59 tests) all pass on the merged tree.

The prior review's one open thread on NativeChatStructuredSession.test-harness.tsx:195 (the harness derives turnRunning from turnId rather than the production isWorking) is untouched by this merge and remains open.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

…nd of it is that author's turn

Main now lets Orca's own sends ask to queue (sendAgentTurn's 'queue' delivery),
so "every card is a person's" no longer holds by refusing host sends. Each card
records who wrote it (the submission's client/host vocabulary) in a new nullable
column; the drain records that origin, so a person's card ends a Stop's pause
and Orca's does not. /clear carries the author. Rows from before the column
read as a person's. The userSend-only admission gate is removed.

@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 delta. One prior thread (NativeChatStructuredSession.test-harness.tsx:195, the harness deriving turnRunning from turnId rather than isWorking) is untouched by these commits and remains open, so this stays a comment-only review.

Reviewed changes

Reviewed the delta since pullfrog's prior review (7a28cd5 → 4ef532e), which records each queued card's author so the queue's send of it is that author's turn.

  • A queued card records its author — queued_messages gains an origin column; QueuedMessageRow carries it, /clear carry preserves it, and the schema ALTERs it in at every writable open (no user_version bump). Legacy rows with no value read as 'client'.
  • The drain sends as the card's author — appendSubmission now writes origin: next.origin instead of a constant, so a person's card releases a Stop's hold once its turn is accepted, while a host-authored card's send does not. maybeQueueStructuredAgentSessionSend records origin from userSend and drops the f91c1ae guard that had made host sends bypass the queue.
  • Tests — new structured-agent-session-queued-card-author.test.ts covers a host card not lifting a Stop's hold vs a person's card lifting it, and /clear carrying the author; the store suite adds the legacy-default and missing-column-heal cases; the stop/restart/pause suites are updated to the per-author behavior.

I traced origin through insert, /clear carry, the drain submission, notePersonTurnAccepted, and the store row mapping, and ran the touched host suites (86 tests) on the merged tree — all green.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review summary for this PR (head 0809787d9b2)

The problem. In a native chat you can queue follow-up messages while the agent works. If you press Stop, those messages are held instead of sending on their own. Orca showed a header row above them, "Queue paused because you interrupted" (or "…because Orca restarted", "…after you cleared the conversation"), with a Resume button. That row is an extra status line for a state you just caused and can already act on. The held cards also offered "Steer", although nothing was running to steer into.

What changes for you (once the message queue ships; it is switched off today):

  • No queue-paused header and no Resume button. Held messages simply stay in the queue.
  • A held card's button says Send when nothing is running, and Steer only while a turn is running.
  • Sending anything yourself releases the held messages, and they follow in order: a new message, Send on a card, or a card you queued after the Stop that the queue sends.

What the review fixed.

  • Held cards could be stranded (P2). The first version let a card queued after the Stop send ahead of the held ones. But the queue's own send didn't count as your turn, so the older cards then stayed held forever, with no header left to explain it. Now each queued card records who wrote it. When the queue sends a card you wrote, that counts as your turn, and the held cards follow. This holds up in the database for old, new and rolled-back builds.
  • Conflict with a change that just landed on main (found when merging main). An intermediate fix allowed only your own messages to be queued. That silently broke a new option on main that lets Orca queue its own agent-to-agent messages, and one CI shard on the merge commit was red. Replaced by the "who wrote it" record above: Orca's own queued messages queue again, and they don't release a hold you set.
  • PR description corrected. It now describes the common pattern accurately and labels every remaining difference as intended or temporary.

Deferred or pending.

  • Pending a product decision (P2): after a restart, the cards that were waiting stay held until you send something. A message you write while a turn Orca started is running after the restart is held with them. /clear likewise holds the cards it carries. Both are labelled Temporary.
  • P3: a card queued after a Stop loses its "Waiting for your answer" caption while an approval is open, because the host publishes the hold for the whole queue, not per card.
  • P3: a card records only "you" or "Orca", not which agent queued it, and an Orca card looks like yours. Nothing in Orca queues its own messages today.
  • P3: two stale code comments, and a possible one-frame Send/Steer flicker between turns (not measured).
  • Mobile still shows its own header and Resume (separate change).

Verified.

  • Host and renderer tests pass on the reviewed head: 70 files, 765 tests in a review run. CI on 0809787d9b2 is green (19 checks pass, none failing).
  • Web and node typechecks pass.
  • Lint, code-quality and localization checks pass.
  • Before/after screenshots were retaken on a remote Windows machine in isolated test builds with the queue switched on. They show the header gone, Send vs Steer, and the restart case.

Not verified.

  • A card queued in the moment before a Stop lands could not be produced from the screen; host tests cover it.
  • Not tested with a real Claude or Codex agent: the screenshots used a stand-in agent.
  • Mobile and SSH were not exercised.

@brennanb2025
brennanb2025 marked this pull request as draft October 4, 2026 05:49
…n idle held queue offers Resume

A restart's pause held every waiting card, including one a person typed after the restart while
Orca's own continuation ran, and nothing released it except a per-card Send. It now holds only
cards another host process wrote, the same way a Stop holds only cards queued before it.

The composer's primary button becomes Resume (Play) while nothing is typed, no turn runs and the
host holds a card Resume would send, whatever held it (Stop, restart or /clear). It calls the
existing agentSession.queuedMessagesResume, guarded against a second press in flight.

A card nothing holds keeps the run going between a turn's end and the queue's send of it, so its
Steer no longer flips to Send for the frame in between.
The host published one pause for the whole queue, so a client held every waiting card while it was
set. Between a turn's end and the queue's send of a card queued after a Stop or restart, the
composer could flash Resume and the cards Send, and a card queued after a Stop lost its
"Waiting for your answer" caption.

Each published card now carries an optional `heldBy`: the pause holding it, or null, derived from
the same rule the drain reads. A client holds only those cards; against a host without the field
it falls back to the queue-level pause.
… Resume returns focus

After Resume, the host lifts the hold in one update and sends the first card in a later one. In
between nothing was running, so the composer's button flashed a disabled Send. A card nothing
holds now keeps the queue's run going for the button too: an empty composer shows Stop, disabled
until the turn starts. Not when the host refuses every send (a rewind whose outcome is unknown,
read from its status), where nothing is coming. The same fix removes the Stop, Send, Stop flip
between queued turns.

Resume disables the button, which dropped keyboard focus; focus now returns to the composer.
…e chat stays working across the gap

A turn's end, or a Resume, and the queue's send of the next card commit as two host updates. In
between nothing was running, so the working status, timer, pickers and composer button flipped
for one update. The client guessed the drain from its own copy of the host's gates, which missed a
/clear-replaced source and covered only the button.

The queue publication now carries `nextQueuedMessageId`: the drain's own next card through the
drain's own gate (`nextStructuredQueuedMessage`, which the drain step now calls), null whenever the
host would refuse the send. The client derives one fact, the queue is about to send, and every
working reader follows it; Stop stays disabled until a turn can be stopped. The client-side copy of
the gates and the status-feed rewind read are removed.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 4, 2026 09:44
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Follow-up review summary (head 31995e48a1d)

What was still wrong after the first review. After Orca restarted, held messages looked exactly like messages waiting for the current turn, but they never sent. Messages you typed after the restart were held too, and Orca's own continuation turn never released them. After Stop or /clear, held messages sat in an idle chat with nothing on screen saying how to send them. Between queued turns, and right after releasing the queue, the composer button flickered (Stop → Send → Stop) and the working timer blinked off.

What changes for you (once the message queue ships; it is switched off today):

  • Restart hold: a restart holds only the messages written before it. A message you queue afterwards sends when the current turn ends, and the older ones follow it.
  • Composer Resume: when the queue is held and the composer is empty and idle, the composer's main button becomes Resume (a play icon). Pressing it sends the held messages in order. It turns back into Send as soon as you type. There's still no header and no caption on the cards.
  • No flicker: while the queue is sending its messages, the chat stays "working" throughout. The button stays Stop, the timer stays on, and no card flips to Send.
  • Caption fixed: a message queued after a Stop keeps its "Waiting for your answer" caption while an approval is open.

How (in short). The host now publishes which hold covers each queued card, and which card it will send next. The client reads both instead of guessing. Both are new optional fields, so older clients and hosts keep working.

Deferred (P3).

  • While a rewind blocks sending, Resume can still appear; pressing it sends nothing.
  • While the connection is re-establishing, Resume stays enabled, as Send does; a failed press shows one error.
  • The model picker can briefly disappear (about 90 ms) as the first released message starts.
  • Keyboard Cmd/Ctrl+Enter still sends the newest held card, while Resume sends the oldest first.
  • Mobile still shows its own header and Resume (separate change).

Verified.

  • Seven review rounds, the last ones narrow, plus a second readiness pass. No open P0–P2.
  • Over 1,000 tests pass. Web, node and CLI typechecks pass, and CI is green on the final head.
  • Before/after screenshots were retaken on remote test machines (Windows and Mac) with the queue switched on. They include 50 ms recordings showing the button and the working status stay steady through every hand-off.

Not verified.

  • A real Claude or Codex agent (the screenshots used a stand-in).
  • Mobile and SSH weren't exercised.
  • A message queued in the instant before a Stop lands could not be produced on screen; host tests cover it.

@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 critical issues — one open harness-fidelity thread re-anchored.

Reviewed changes

Reviewed the delta since pullfrog's prior review (4ef532e → 31995e4), which adds per-card holds, narrows the restart hold, adds the composer Resume, and has the host name the card its queue sends next.

  • Per-card holds — DerivedQueuePause becomes a union (stopped/cleared/restarted); the published cards carry a new heldBy, and the client falls back to the whole-queue queuePause only when the host predates the field.
  • Restart hold narrowed — a restart now holds only cards another host process wrote (card.hostInstance !== pause.hostInstance), never one queued since; queuedBeforePause is the single rule.
  • Composer Resume — the primary button becomes Resume (play icon) on an empty composer with a held queue and nothing running; the controller gates it through queuedMessagesResumable and a ref guards a double press.
  • Host names the next card — nextQueuedMessageId rides the queue publication, derived from the drain's own pick/gate; the client reads queueSendsNext and keeps a disabled Stop across the gap between a turn's end and the next queued send.
  • Tests — new/updated composer, resume, per-card-hold, restart-hold and next-card suites; all focused suites pass on the head.

I traced heldBy/nextQueuedMessageId through the host derivation, publication gate, subscriber frame, coalescer, reducer and client projection, and confirmed the drain and the publication share the same fence and host instance, so the named next card matches what the drain sends. Ran the focused delta suites (composer primary-action/field/actions, host pause/card-holds/stop-row, resume/gating/session, reducer + coalescer) — all green.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025
brennanb2025 marked this pull request as draft October 4, 2026 21:10
…2025/chat-queue-continues-after-stop

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
…ue-continues-after-stop

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.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-send.ts
#	src/renderer/src/components/native-chat/NativeChatComposerActions.test.tsx
#	src/renderer/src/components/native-chat/NativeChatComposerActions.tsx
#	src/renderer/src/components/native-chat/NativeChatComposerField.tsx
#	src/renderer/src/components/native-chat/use-native-chat-structured-composer-send.ts
#	src/shared/agent-session-journal-types.ts
…sked about; tests follow main's draft props

- The open dialog closes when the queue's pause lifts under it (Orca's mail, another client's
  Resume, any accepted turn): nothing is sent, the draft stays, and the next Enter sends as
  usual. The pending choice records the hold it was asked under; nothing new is stored.
- The composer-field Resume test passes main's dropScopeKey/draftScopeKey.
- The dialog test expects main's rule: only the sent text leaves the composer.
…2025/chat-queue-continues-after-stop

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-restart-interruption-test-harness.ts
@brennanb2025 brennanb2025 changed the title fix(native-chat): a paused message queue never strands cards, offers Resume in the composer, and confirms before sending past it fix(native-chat): queued messages carry on in order after any turn, and nothing sends by itself after a restart Oct 6, 2026
@brennanb2025
brennanb2025 marked this pull request as ready for review October 6, 2026 06:59
@brennanb2025

brennanb2025 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Review summary: the queue now behaves like a plain queue (head 70a37e35820)

What was wrong. With the message queue switched on (it ships switched off today):

  • Stuck after a restart. Messages you had queued could stay stuck forever after Orca restarted. Only a turn you started released them, and Orca's own "carry on" turn after you resumed the chat didn't count.
  • The Resume offer disappeared. Quitting Orca with messages queued made the queue try to send during shutdown. Those attempts were refused, and the refusals made Orca withdraw the chat's Resume offer. Main has the same bug today.
  • Flicker between turns. Between queued turns, the button and the working timer flickered.
  • No question before sending past a pause. Nothing asked before you sent a new message past a paused queue.

How the queue works now

  • While the agent works: queued messages (yours, and Orca's own messages to a busy chat) send in order, one per turn.
  • After Stop: "Queue paused because you interrupted" with Resume; the cards keep Steer; the empty message box also shows Resume. Any turn the agent accepts (yours, a card's Steer, or one of Orca's own messages) or Resume ends the pause, and the rest follow in order.
  • After a restart: nothing sends by itself. When you resume the chat or send a message, your queued messages follow it in order.
  • After /clear: the messages it carries over wait the same way as after Stop.
  • Sending a new message while paused asks "Send message?":
    • Clear queue deletes the waiting messages, then sends.
    • Send message sends, and the rest follow.
    • Esc sends nothing and keeps your text.
    • If the pause ends while the question is open, it closes and your text stays.

What the review changed. It found and fixed:

  • the restart rule (dropped the "who sent the turn" condition);
  • the quit bug above;
  • a double send from the dialog;
  • a race in Clear queue;
  • the dialog outliving the pause;
  • several merge conflicts with main.

The design was simplified at the same time: one rule ends every pause, the queue keeps strict order, there's no per-message author field, and the "paused" row hides while a turn that will lift it is on its way.

Deferred (P3):

  • Mobile still shows its own header and drops to idle between queued turns.
  • A press of Send during Clear queue's brief window is ignored without feedback.
  • In a chat that receives Orca's coordinator messages, an unread one re-sent after a restart counts as a turn, so your queued messages follow it.

CI on 70a37e35820 (merge of main 83cf7cf5e28, which includes main's own lint fix #25933): 18 passed, 14 skipped, 0 failing.

  • Before the re-run, the unit test shards failed once each on tests this PR doesn't touch, and differently on each run. One run failed structured-agent-session-journal-corruption.test.ts, the next structured-agent-session-restore-without-import.test.ts, and one shard lost its runner (CI infrastructure). Both tests pass three times out of three locally on this head, and re-running only the failed jobs passed.
  • This merge also fixed one real failure: main's new mobile test for a host-kept card published a restarted queue pause, which this PR's wire type no longer lists. The test now stands in a Stop's pause, because this host never publishes a restart pause.

What the main merges changed here. Main's #24369 adds a "Stopping…" state to the Stop control; the message box keeps this PR's Resume beside it (Stopping… / Stop / Resume / Send), and Stop stays live only once a turn can be stopped. Main's #24660 keeps a message you sent but the agent never received (Orca quit, crashed, or the chat closed first) as a card. To pick those messages, it reads which side asked for each send: you, or Orca itself. This PR had removed that record as unused. It is back, but it only decides which unsent messages become cards; any accepted turn or Resume still ends a pause. Main's new tests for those cards now expect the restart's hold to stay unpublished, as this PR has it.

Verified:

  • 3913 tests pass across 378 files, and every CI check other than main's known failures above passes.
  • Typechecks are clean apart from a dependency this checkout lacks.
  • Every restart case was checked after a real quit, on a Windows test machine with the queue switched on: nothing sends while idle; Resume runs the carry-on turn, then the queue; your own message goes first, then the queue; Steer works. Stop + Resume and all three "Send message?" choices were checked there too.

Not verified: a real Claude or Codex agent (QA used a stand-in), mobile, and SSH.

…r-stop

Conflict in use-structured-agent-session.ts: kept main's rewind-gated retry
and the PR's isWorking (queue sends next) and queueSendsNext. Trimmed two
lines in that hook and collapsed the approval options map in
NativeChatStructuredSession.tsx to stay within max-lines after main's rewind
additions; no behaviour change.
…r-stop

Main's #24660 keeps a person's unsent message as a card after a quit, crash or
close, and it reads the submission's origin (client or host) to decide which
sends to keep. This branch had removed origin and the client send's userSend
flag as unused, so they are restored as main has them; comments now say origin
only decides what a restart or a close keeps, never what lifts a pause (any
accepted turn or Resume still does). The queue drain's quit gate is kept once,
with main's re-check right before the hand-off.

Main's new kept-card tests are adjusted to this branch's rule that the
restart's hold is never published (queuePause null; the cards still wait).
…r-stop

Main's #24369 adds a Stopping state to the composer's Stop control and its own
stop controls; the composer keeps this branch's Resume action beside it
(Stopping… / Stop / Resume / Send), and Stop stays live only once a turn can be
stopped. Main's new Stopping test passes the primary action this branch requires.
The composer's queue field props come from the queue's composer hook as one
object, to stay under the file's line limit.
…is host publishes no restart pause

Main's #24660 test published queuePause 'restarted', which this branch's wire
type no longer lists, so the mobile tests typecheck ratchet failed.
brennanb2025 added a commit that referenced this pull request Oct 6, 2026
…s-after-stop' into brennanb2025/stop-queue-after-stop

Composer: Stop reads Stopping… while this client's Stop is in flight (#24369) and gives way
to Resume when the queue is held and idle (#24586). Stop is offered while the queue sends next
but live only once a turn can stop. queued-stop-row test takes #24586's version.
…r-stop

Main's #25888 shows which agent a queued message is from: the sender moves onto
the message body (`from`), and a person's send carries `userSend`. The card
keeps this branch's Steer/Send rule beside main's sender line; the queue's card
projection imports both. The test rig and its card test take main's `from`;
a card-holds test no longer passes the removed `source` field. Locale catalogs
keep both sides' new keys.
…r-stop

Brings main's own fix for the duplicate resolveLaunchArgs key (#25997). No conflicts.
…r-stop

Main's #24514 reveals the transcript on a send and on a Resume that lifts the
pause. This branch's composer Resume goes through the same reveal; the queue's
Resume answers whether it lifted the pause on both surfaces. The composer's
awaitable structured send keeps main's command reveal at the press, and main's
new navigation test fires that send without awaiting it.

Line limits after the merge: the composer's working/Stop rule moves into the
structured stop controls; the queue controller takes the transport's fields by
spread; the option picker request type is named; test harness resets combined.
@brennanb2025
brennanb2025 merged commit 0c96550 into main Oct 7, 2026
71 of 76 checks passed
brennanb2025 added a commit that referenced this pull request Oct 7, 2026
…ard-ordinary

The sender moved off the queued row onto the message body on main (#25888): the row's source plumbing goes, this branch's derived holds stay.
brennanb2025 added a commit that referenced this pull request Oct 7, 2026
…nary

Main squashed #24586 (this branch's base) as 0c96550; merged against #24586's final head so its own changes are not re-resolved.
brennanb2025 added a commit that referenced this pull request Oct 7, 2026
…ueue-after-stop

Rebuilt as main plus this branch's own changes against its old base: the branch carried #24586's
unsquashed history, so main's squash conflicted on every shared file. The person's-Stop mail
check now reads the message body's sender (`from`, from #25888) and the separate card `source`
field is gone; the moved queue step takes main's fingerprint and its replay answer.
brennanb2025 added a commit that referenced this pull request Oct 7, 2026
…brennanb2025/nc-host-send-queue

- Queue UI (#24586) taken as main has it: paused header, Resume, Steer, clear-queue question,
  `queueSendsNext` working state, composer Resume and queue hold.
- The chat's send answers 'queued' when the transcript draws it as a card (main's rule, read through
  the in-memory sends' own projection), so the composer moves the reader only for a bubble.
- The failed-start send wrapper passes that answer through; the submit reveal hook drops the
  per-message Retry this branch removed (launch Retry still reveals).
- The pending-send projection takes any send shape, so the node project's queued-card test does not
  reach the renderer store.
- Deleted outbox files stay deleted; #24514's queued-card admission test is ported to the sends hook.
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