Skip to content

feat(orchestration): a native chat gets the orchestration pointer a CLI agent gets, through the same send as your messages - #25078

Merged
brennanb2025 merged 23 commits into
mainfrom
brennanb2025/agent-messages-via-queue
Oct 6, 2026
Merged

brennanb2025 merged 23 commits into
mainfrom
brennanb2025/agent-messages-via-queue

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 17 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​491 $\color{#cf222e}{\Huge{\mathbf{−}}}$​509 $\color{#cf222e}{\Huge{\mathbf{−}}}$​18
Prod 17 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​611 $\color{#cf222e}{\Huge{\mathbf{−}}}$​119 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​492

The problem

When an agent sends orchestration mail to an agent that runs as a native chat (a structured session), the chat did not get it the way a CLI agent does.

The code that tells an agent about new mail had its own path for chats: its own rules for when a chat counts as busy (a turn running, or a question waiting for you), and its own send, outside the chat's send button and queue.

While the chat was busy, you saw nothing. Orca held the notice back and sent it later, by itself, when its own rules said the chat was free.

So a chat behaved differently from a CLI agent and from your own messages, and there was a second set of "is it busy?" rules to keep in step with the chat's own.

What this changes for you

A native chat now gets exactly the line a CLI agent gets: "You have N orchestration messages. Run orca orchestration check." It comes through the same send as the messages you type, and the chat's queue gives it no special handling: it is held, sent, paused and resumed exactly like a message you queue.

Before After
Mail arrives while the chat is idle The notice is sent as a turn by Orca's own send The notice is sent as a turn at once, through the chat's own send
Mail arrives while the chat is busy Nothing is shown; Orca holds the notice and sends it later on its own The notice waits above the composer as a normal queued card (Steer, Delete), and is sent when the turn ends
More mail arrives before the agent has read the mail it was told about Held with the rest It gets its own notice, counting only the mail not yet announced, as a terminal agent does. While the chat is busy, that is a second queued card
You press Stop The notice is held by Orca's own rules, out of sight The card waits under "Queue paused because you interrupted", with Resume, like your own cards
Orca restarts with a notice card waiting (no card existed) The card is held by the restart pause, with Resume, like your own cards
You delete a notice card (no card existed) Nothing is sent in its place. Its mail stays unread, and check still returns it. Later mail gets its own notice
The agent runs orca orchestration check Returns its unread mail Unchanged

Agents running in a terminal are unchanged.

How it works

The code that tells a chat about new mail sends the same text the terminal path types (formatMessagePointer, with the same command name) through sendAgentTurn, the one function for agent-to-agent sends, with delivery queue. That is the composer's own "queue if the chat is busy" send: an idle chat starts the turn, a busy chat's queue holds it as a card. The orchestration code does not know or check which happened, and it never reads, watches or waits on the chat's queue. The separate "is the chat busy?" rules for this path are deleted.

Agent notices get no special handling in the native-chat queue. The queue code is the same as on main apart from storing one column (below); nothing in it looks at who a card is from.

Which mail a notice counts uses the terminal path's rule unchanged. When a notice is sent or queued, its mail is marked as announced (the existing delivered_at marking that keeps the same mail from being announced twice), so the next notice counts only newer mail. The existing rule that no notice goes out while a check batch is waiting to be acknowledged is unchanged.

check, and which mail counts as read, are exactly as before: only check reads mail.

Each card records who it is from in queued_messages.source_json: the senders, and the messages it points at. This is for the next PR, which shows the sender on the card. It is data only: it changes no behaviour, it is stored on this machine only, never sent to any client, and a client's own send cannot set it. /clear carries it with the card.

Why this approach

Brennan's direction: a native chat should behave exactly like a CLI agent for orchestration, reusing the send the chat already has, with no special handling for agent messages in the chat's queue, and without waiting for the chat to be idle.

Earlier versions of this PR sent the mail itself as the turn, then tracked what each card carried and let agent cards skip the restart pause. That needed rules to hide, withdraw and re-check queued cards. Sending the CLI agent's notice through the chat's own queue removes all of that. A later version also held back a second notice while earlier announced mail was unread; Brennan asked to keep that the same as CLI agents for now, so it is removed.

What you might notice

The label says whether it stays (intended) or a follow-up removes it (temporary).

  • Several messages during one busy turn can queue several cards, each "You have 1 orchestration message". A terminal agent instead gets one notice at its next idle moment for all new mail. The first card's check returns everything unread, so a later card's check may find nothing new. Intended: Orca does not wait for the chat to be idle, and a queued card is never rewritten.
  • After a restart or a Stop, a waiting notice card needs Resume, like your own cards. Intended: the queue treats every card the same.
  • If the agent's last turn failed to start in a way Orca cannot confirm, a new notice waits as a card until that clears, just as a message you send then would. Intended: it is the chat's own send rule.
  • Mail that arrives after the agent ran check but did not acknowledge that batch (--ack) is not pointed until it does. Intended: unchanged from before, and the same for terminal agents.
  • In this build, queueing your own messages is not switched on for chats, so a message you type while the agent works goes straight into the running turn, while the agent's notice waits as a card. Temporary: it lines up once your messages queue too, which is a separate setting this PR does not change.
  • The notice appears in the conversation as a right-hand bubble, like a message you sent. Temporary: the next PR labels it with its sender.

Differences from the common pattern

  • The common pattern steers an agent's message into a busy chat; this PR queues it until the turn ends. Intended: Brennan asked for the composer's queue, which already exists so that a turn is not interrupted.
  • The common pattern delivers an agent's message as the turn's text; this PR sends a notice, and the agent reads the mail with check. Intended: it is exactly what CLI agents get, and the mailbox stays the single record of what was read.

Linked Issue

N/A: maintainer change, no issue.

Visual Proof

From the live run on an earlier version of this PR (see Testing). These four states are unchanged by the later simplification.

Busy chat: the notice waits as a normal card while the turn runs.

v3-s1-card-midturn.jpg

The turn ends, the notice is sent once, and the agent runs check.

v3-s1-after-pointer-and-check.jpg

Idle chat: the notice is sent at once and the agent runs check.

v3-s2-idle-pointer-turn.jpg

After Stop: the card waits under "Queue paused because you interrupted", with Resume.

v3-s6-after-stop.jpg

Testing

Automated tests. Each behaviour test below, except the second-card test (new on this head), was checked by removing the code it covers and watching it fail.

  • An idle chat gets the notice at once, the same bytes a terminal agent is typed (real host).
  • A busy chat queues it as a card, with its sender stored, and sends it once when the turn ends (real host).
  • Mail that arrives while an earlier notice waits gets its own card, counting only the new mail; each is sent in turn (unit, and end to end on the real host).
  • /clear carries a card's sender with it; a card from before the column reads as the person's.
  • Stop and restart: the existing queue suites (changed only to pass the new column), since agent cards take no other path.
  • check is unchanged for a chat (the mail stays readable and the card stays) and for terminals (the existing suites, unmodified).

The orchestration and orchestration-messaging test directories and the native-chat queue suites pass. pnpm tc:node and pnpm tc:cli pass on this head.

Live run on macOS against a real Claude chat, at 8f2904d, an earlier version of this PR. Rows marked "changed since" no longer describe this head and were not re-run:

Case Result
Idle chat gets the notice at once Pass
Busy chat: one card, sent once at turn end, then the agent ran check Pass
Three messages in one busy turn: one card Pass; then one extra notice whose check found nothing new. Changed since: each message that arrives while the turn runs now queues its own card
Delete the card: not sent later, check still returns the mail Pass
Stop holds the card with Resume; Resume sends it Pass
Restart with only an agent card waiting: sent without Resume Changed since: the restart pause now holds it, with Resume, like your cards

Platforms: macOS. Linux, Windows and SSH share this code path; nothing here is platform-specific, and remote chats are not reached by this delivery.

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.

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)

…n queue

A mail notice for a busy structured chat used to wait in the orchestration lane's
own invisible "until the chat is free" gate. It now goes through the queue a
person's message uses: sendAgentTurn(..., { delivery: 'queue', source }) makes
the host hold it as a draft card the person sees and can Steer or delete, and
the queue sends it when the turn ends.

- The lane's busy gate (turnRunning / awaitingHuman) is gone; the queue's own
  hold decides. Its parking now only waits for its own send or card to settle.
- A queued card's hand-off (sent under a fresh id) is matched by
  queuedMessageId, so it stamps the mail once and never sends a second notice.
- A card from before an Orca restart is still the lane's: no second card.
- A card the person deleted counts as handled for that batch; newer mail
  notifies again. No stored flag: read off the card and delivered_at.
- `orchestration check` waits for the lane to withdraw a card whose mail it
  read, so a stale notice is never sent; more mail replaces the card.
- Each card records who queued it in queued_messages.source_json (versioned,
  schema-validated): the person, or Orca for agents, with every distinct
  sender as an orchestration party plus host id, and the mailbox, dispatch,
  run and message ids. The restart pause holds only the person's cards.
- sendAgentTurn answers with a snapshot, not the journal's live submission.
@coderabbitai

coderabbitai Bot commented Oct 3, 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: b954f6f7-c16d-495b-9773-57a414af15dd
📥 Commits

Reviewing files that changed from the base of the PR and between 92a5e8d and 5a21eb5.

📒 Files selected for processing (8)
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-agent-card.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-agent-card.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-drain.ts
  • src/main/runtime/orchestration/structured-mail-notice.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-delivery.test.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-delivery.ts
  • src/main/runtime/orchestration/structured-pointer-notice-cards.ts
  • src/main/runtime/structured-chat-coordinator-mail-queue.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/runtime/orchestration/structured-pointer-notice-cards.ts

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


📝 Walkthrough

Walkthrough

Queued messages now store source metadata and use it to distinguish person-authored cards from orchestration cards during pause handling. Queued agent cards can be judged before handoff, then sent as written, restated, or withdrawn. Structured mailbox pointers now use queued cards and journal facts to track changing mail, accepted dispatches, retries, and session cleanup.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 5a21e

The change moves busy-chat mail notices onto the existing chat queue. No concrete merge-blocking issue was identified in the supplied evidence.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 5a21e

Normal delivery retains recipient checks and execution controls, and queued notifications are checked again before sending. Validation failures can still send an older notification, but the review did not establish expanded access or an exploitable authorization bypass. Recovery and compatibility coverage remain incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is mailbox notification state, recipient-session journals, and provider turns reached through the existing structured host. Normal judging binds each notice to its currently resolved recipient session. These paths do not establish broader tenant, environment, credential, or infrastructure exposure.

Trust Boundaries and Controls

  • observed — Normal staging requires readable session facts and a current runtime fence. Submission remains delegated to the structured host with the expected fence and dispatch- or session-scoped caller identity. The mailbox judge separately checks current recipient identity and ordered owed-message IDs.
  • observed — Handoff revalidation is deliberately best-effort: a thrown judge sends the stored card as written, and an absent judge has the same fallback. Production wiring supplies the judge, and a test explicitly exercises database-error fallback. This can send a notice without fresh mailbox checks; the inspected evidence does not establish attacker-triggered failure or unauthorized mailbox access.

Resilience and Maintainability Implications

  • observed — Recovery distinguishes host withdrawal from a person's decline and links queued cards to submissions rather than relying on ID equality. Failed unseen-card handback is best-effort: callback exceptions are logged without immediate retry. This limits local retry amplification but does not prove eventual redelivery after callback failure.

Hardening Proposals

  • proposed — If current mailbox ownership is intended to be a strict handoff control, defer agent notices when judging throws rather than sending them unchecked. Preserve progress for unrelated user cards, and distinguish legacy missing provenance from corrupt agent provenance. This is a hardening proposal, not a demonstrated exploit.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.63% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 108 functions across 57 files. 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 identifies the main change: native chats receive orchestration pointers through their existing send path. It is somewhat long, but remains specific and relevant.
Description check ✅ Passed The description covers the template’s required sections, including the user-facing change, rationale, linked-issue maintainer exception, visual proof, testing, platform impact, and checklist. It also …
✨ 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

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: 214f33c4-f212-42c2-87e0-f7e38e2c7498
📥 Commits

Reviewing files that changed from the base of the PR and between 752871b and 7e7d5f9.

📒 Files selected for processing (41)
  • src/main/claude/claude-structured-history-window.ts
  • src/main/native-chat/agent-session-journal/journal-queued-messages.ts
  • src/main/native-chat/agent-session-journal/journal-submission-queued-link.test.ts
  • src/main/native-chat/agent-session-journal/queued-message-bookkeeping-failure.test.ts
  • src/main/native-chat/agent-session-journal/queued-message-delivered-echo.test.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-journal/queued-message-schema.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/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-queued-message-rig.test-fixture.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-send.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restore-without-import.test.ts
  • src/main/runtime/orca-runtime-get-pty-record-for-pane-key.ts
  • src/main/runtime/orchestration/agent-facing-parity.test.ts
  • src/main/runtime/orchestration/db/messages/structured-pointer-operation-store.ts
  • src/main/runtime/orchestration/orchestration-caller-identity.ts
  • src/main/runtime/orchestration/send-agent-turn-host.test.ts
  • src/main/runtime/orchestration/send-agent-turn.test.ts
  • src/main/runtime/orchestration/send-agent-turn.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-delivery.test.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-delivery.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-host.test.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-host.ts
  • src/main/runtime/orchestration/structured-pointer-operation-id.test.ts
  • src/main/runtime/orchestration/structured-pointer-operation-id.ts
  • src/main/runtime/orchestration/structured-pointer-source.test.ts
  • src/main/runtime/orchestration/structured-pointer-source.ts
  • src/main/runtime/orchestration/structured-session-pointer-delivery.test.ts
  • src/main/runtime/orchestration/structured-session-pointer-delivery.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/check-methods.ts
  • src/main/runtime/structured-chat-coordinator-mail-queue.test.ts
  • src/main/runtime/structured-chat-coordinator-mail-rig.test-fixture.ts
  • src/main/runtime/structured-chat-coordinator-mail.test.ts
  • src/shared/orchestration-party-identity.ts
  • src/shared/queued-message-source.ts
💤 Files with no reviewable changes (1)
  • src/main/runtime/orchestration/structured-session-pointer-delivery.test.ts

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

Comment thread src/main/runtime/orchestration/structured-mailbox-pointer-delivery.ts Outdated

@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.

Important

A /clear that carries an agent's queued mail-notice card orphans it: the chat can be pointed immediately, and the carried card later drains as a second, stale notice. One confirmed behavioral defect (reproduced on the coordinator-mail rig) plus a minor robustness gap; details inline.

Reviewed changes

  • Mail lane rides the chat queue. structured-mailbox-pointer-host.ts now sends the notice with delivery: 'queue' and a host-local source; a busy chat holds it as a card its own queue sends when the turn ends.
  • Private busy check deleted. decideStructuredSessionPointerDelivery, StructuredPointerGateFacts, turn-unsettled and awaiting-human are gone; busy is the queue's structuredQueueHold.
  • Pointer state machine is card-aware. structured-pointer-operation-id.ts gains StructuredPointerFacts (submissions + cards), structuredPointerSendState, and the queued/declined attempts; hand-offs match via queuedMessageId.
  • Withdraw + retire. structured-mailbox-pointer-delivery.ts withdraws a stale card once its mail is read (retire), replaces a card when the batch/session changes (withdrawSupersededCard), and re-derives every row on mail read (onMailRead).
  • Read withdrawal wired into check. check-methods.ts wraps orchestration.check in afterMailRead, awaiting onMailRead so the card goes before the agent's turn can end.
  • source_json persistence. New nullable queued_messages.source_json with a versioned schema in shared/queued-message-source.ts; absent/unreadable reads as the person's.
  • Restart pause narrowed. Only kind: 'user' cards from another host instance wait out a restart; an agent card sends. OrchestrationPartyIdentity moved to src/shared/; send returns a snapshot; test rig split out.

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

Comment thread src/main/runtime/orchestration/structured-mailbox-pointer-delivery.ts Outdated
Comment thread src/main/runtime/rpc/methods/orchestration/messaging/check-methods.ts Outdated
…ds it

A mail notice queued in a busy chat could go out stale or twice: it was
kept true from outside the queue, by orchestration check withdrawing it,
which missed other readers, raced an attempt in flight, lost the card
when /clear carried it (a second notice), and moved it to the back when
new mail replaced it. An agent's card behind a person's paused card never
sent after a restart, so an unattended coordinator stalled.

- The drain asks the card's sender, in its own serialized step, whether
  it still stands: send, restate (count and senders of the mail owed
  now, written onto the card in the hand-off's transaction), or withdraw
  as the host. A failing judge sends as written. Send-now restates but
  never withdraws. Orchestration registers the judge on the host.
- check no longer touches any queue (back to main); onMailRead,
  notifyOrchestrationMailRead and the lane's withdrawals are gone.
- One unsettled card per agent message, enforced at insert; new mail is
  counted when the card sends, never replaces or moves it.
- The lane finds its card by what it is (a notice for the mailbox in the
  session the mailbox reaches now); a decline is a withdrawal a person's
  operation stamped. /clear moves agent cards as the host. Pointer rows
  last only while a direct send is in flight and end with the session.
- Pauses (restart, Stop, /clear) hold only a person's cards, and a held
  card never traps an agent's card behind it, keyed on the message kind.
- The source is named for the message (agent-session-message-source),
  one payload shape per message kind, no relative host id.

@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 — the redesign that fixes the prior /clear finding looks correct and is well covered; one PR-description update suggested.

Reviewed changes

  • Stale-card withdrawal moved from check time to send time. structured-mailbox-pointer-delivery.ts now judges an agent's queued card in the drain's serialized step (judgeQueuedCard): withdraw when the mailbox no longer reaches the session or nothing is owed, restate when the owed batch changed, otherwise send as written.
  • Deleted the lane's earlier stale-card machinery. onMailRead, withdrawSupersededCard, the host's withdraw, and check-methods.ts's afterMailRead/notifyOrchestrationMailRead are gone; retire now only forgets the pointer row.
  • Cards found by identity, not id or session. New structured-pointer-notice-cards.ts matches cards on mailbox plus messageIds and separates a person's withdrawal from the host's, which is what un-orphans a /clear-carried card.
  • Cards restated at hand-off. New queued-message-restatement.ts rewrites body, fingerprint and source inside the consume transaction, so a card sent later records the mail it actually counted.
  • Selection widened past non-held cards. nextSendableQueuedCard lets an agent's card pass a held or returned person card, and sameAgentMessage keeps one unsettled notice per mailbox.
  • Judge wired through the runtime. Runtime deps → host deps → wireStructuredAgentSessionQueuedMessages → drain, with SEND_AS_WRITTEN as the absent fallback.

ℹ️ Nitpicks

  • The PR description still describes the mechanism the fix replaced: it cites notifyOrchestrationMailRead and says a stale card is withdrawn during orchestration.check before the check answers. In 37ead45 that became drain-time judgement, so a card is judged as it would send, not when the mail is read. Its QueuedMessageSource example also shows message at the top level, while the shipped AgentSessionMessageSource puts it under orchestration. Updating those two spots would keep the description matching the code.

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

The notice card read as something the person typed and came and went on
its own, the transient-state message the queue should not show. It is
hidden until B can label who sent it.

- The host leaves mail-notice cards out of the published queue, so the
  list, its count, Steer/Delete/Edit, steer-newest and the paused header
  never see them, on every client. Keyed on the message kind: D6's task
  card will be shown, labelled, in B. They still count toward the queue
  limit (temporary, until B).
- A refusal no longer returns a hidden card for the person to act on:
  the host withdraws it, and the lane points again only after a turn ran
  since, the rule it already had for a refused notice.
- Send-now no longer restates a card: no one can reach a hidden one.
- The stored sender drops the pane key, a mailbox credential.
- The gate covers an abandoned worker: a mailbox that reaches no session
  withdraws its notice (tested).

@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: 39b4cec2-df91-4bfd-849b-952a31345ab2
📥 Commits

Reviewing files that changed from the base of the PR and between 37ead45 and 99a1587.

📒 Files selected for processing (16)
  • src/main/native-chat/agent-session-journal/queued-message-pause.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-wire/structured-agent-session-queued-agent-card.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-agent-card.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-publication.ts
  • src/main/runtime/orchestration/send-agent-turn-host.test.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-delivery.test.ts
  • src/main/runtime/orchestration/structured-pointer-notice-cards.ts
  • src/main/runtime/orchestration/structured-pointer-source.test.ts
  • src/main/runtime/orchestration/structured-pointer-source.ts
  • src/main/runtime/structured-chat-coordinator-mail-queue.test.ts
  • src/main/runtime/structured-chat-coordinator-mail-rig.test-fixture.ts
  • src/main/runtime/structured-chat-coordinator-mail.test.ts
  • src/shared/agent-session-message-source.ts
💤 Files with no reviewable changes (1)
  • src/main/native-chat/agent-session-journal/queued-message-store.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/runtime/structured-chat-coordinator-mail.test.ts

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

@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.

Important

Hiding Orca's mail notice from the published queue leaves a card that a failed conversion held with send_failed with no way to release it — no Send-now, no Resume — and every later notice for the same mailbox dedups onto it. Details inline.

Reviewed changes

Since the prior review (37ead45), 99a15870 takes the notice out of sight:

  • The mail-notice card is no longer published. queueShowsCard hides mail-notice cards from the queue and from the pause header; a person's cards are unchanged.
  • A refused hidden card is withdrawn by the host. settleRefusedQueuedMessage withdraws a hidden card the provider refused rather than returning it to a person, and the lane re-derives it as dispatch-rejected until a later turn runs.
  • Send-now no longer restates an agent card, and the stored sender identity drops paneKey (a mailbox credential).
  • Worker settlement deletes the session's pointer-operation rows, and the mail test rig was split.

ℹ️ PR description still describes a card the person can see

The commit message is "waits in the queue out of sight", and queueShowsCard deliberately hides the notice until a later step can label its sender. But the PR body's "What this changes for you" table still promises "A card in the chat's queue, like your own queued message", "Steer it, or delete it, like your own card", and "You delete that card … Not queued again for the same mail"; none of that holds now. Several "What changed" paragraphs (the lane reading the session's cards, the restart rules) describe the visible-card design too, so a reviewer or future reader will be misled about what this ships.

Technical details
# PR description vs. implementation

## Affected sites
- PR body "What this changes for you" table — advertises Steer/Delete and delete-stays-deleted.
- PR body "The mail lane sends through the queue" / "A card you delete stays deleted" / "Restart" paragraphs — describe a visible card and check-time withdrawal.

## Required outcome
- The description should match the shipped behavior: the notice waits in the queue out of sight until step B can label its sender; a person cannot see, Steer, or Delete it in this PR.

## Open questions for the human
- Is the out-of-sight behavior the intended scope for this PR (with the visible card landing in B), or should the notice stay visible here?

ℹ️ Nitpicks

  • Comments that still promise the person sees the notice, now false under queueShowsCard: structured-mailbox-pointer-host.ts:127 ("a card the person sees"), send-agent-turn.ts:45 ("so the person sees whose message waits"), and structured-chat-coordinator-mail-queue.test.ts:89 ("the person sees it waiting").

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

Comment thread src/main/native-chat/agent-session-journal/queued-message-pause.ts Outdated
Hiding the notice left three paths where it waited on someone who
cannot see it.

- A failed hand-off write put a send_failed hold on it, which only a
  person's Send or Delete clears: that mailbox's notices stopped for
  good. The host now withdraws a hidden card instead and hands its
  mailbox back to the lane through the ordinary redrive, since an idle
  chat gets no other edge. A write that keeps failing is retried once
  per edge.
- A person's Stop while the agent started on the notice put it back to
  waiting, where no pause holds it, so it was sent again at once. The
  host now withdraws it; the lane points again after a turn ran or when
  newer mail makes it a different notice, as on main. A restart still
  puts it back.
- A notice queued before the person's message went first. One order
  rule now: the person's cards go in order and never past one waiting
  on them; a card they cannot see never delays one they can, and goes
  only when none of theirs may.

The drain moves to its own module (structured-agent-session-queued-
drain.ts). Comments that still described the notice as visible are
corrected.

@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.

Reviewed changes

Since the prior review (99a15870), 92a5e8d32f makes a hidden mail-notice card never wait on the person:

  • A failed hidden card is withdrawn, not held. The drain's pre-consume catch now withdraws a card the person cannot see (dropAgentCard) and hands its mailbox back to the lane through the new onQueuedAgentCardDropped host dep, so it re-derives a fresh notice instead of parking send_failed with no Send/Resume to release it. This closes the prior review's finding.
  • A Stop-pulled hidden card is withdrawn too. settleRefusedQueuedMessage now also withdraws a hidden card when a withdrawal-category rejection (a Stop) puts it back to waiting, since nothing would hold it against an immediate resend.
  • Hidden cards never delay visible ones. nextSendableQueuedCard records the first card the person cannot see and returns it only when no card they can see may send.
  • A changed notice re-points at once. awaitsTurn now requires every currently-owed message to have been pointed at by the card, so newer mail makes it a different notice rather than a retried old one.
  • The drain was extracted into structured-agent-session-queued-drain.ts; tests cover the drop, the Stop, person-before-notice ordering, and the persistent-failure path.

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

…dge that cannot look decides nothing

- When the agent opened its mail in the notice's own turn (the normal
  flow), an open batch left nothing owed, so the lane never marked the
  mail the accepted hand-off carried as delivered, unlike an accepted
  direct send. The lane now stamps exactly the ids a handed-off notice
  carried whenever undelivered mail remains, owed or not. A later
  conversation is no longer told again about mail already opened.
- At quit the host registry is cleared before teardown, so the drain's
  judge could not resolve the mailbox and withdrew the notice. A judge
  that cannot read its inputs now defers: the card waits for a step
  that can look. A mailbox that resolves to another session or none is
  still withdrawn.
- The judge, the owed-batch selection and the notice body move to
  structured-mail-notice.ts. A failing hand-back of a dropped card is
  logged.

@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.

Reviewed changes

Since the prior pullfrog review (92a5e8d), 5a21eb527c closes two gaps in the hidden-notice path:

  • An accepted notice hand-off now stamps its mail even when the agent opened it in that turn. A new stampHandedOff step runs when the lane owes no pointer, and marks delivered exactly the undelivered-unread ids a notice card with an accepted hand-off pointed at. This covers the agent running check (without an ack) in the notice's own turn: the resulting open batch makes owedPointerBatch empty, so the accepted hand-off was otherwise never stamped and a later conversation was told again.
  • A judge that cannot look now defers instead of guessing. QueuedAgentCardVerdict gains defer; judgeMailNotice returns it when the database is absent or the host cannot resolve the session (Orca quitting), and the drain leaves the card waiting, unsent and un-withdrawn, rather than sending a notice it cannot re-validate.
  • The lane's judge was extracted into structured-mail-notice.ts (judgeMailNotice, owedPointerBatch, mailNoticeBody), shared by the lane and the queue's judge; the drain also now contains a throw from the drop callback.

Verified locally against HEAD: structured-mailbox-pointer-delivery.test.ts and structured-agent-session-queued-agent-card.test.ts (49 tests) and structured-chat-coordinator-mail-queue.test.ts (11 tests) pass, and the new stamping/defer tests fail without their fixes.

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

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status: head 5a21eb5

The problem. When one agent mails another agent that runs as a chat, Orca sends the chat a short notice ("You have N orchestration messages. Run orca orchestration check"). If the chat was busy, the orchestration code held that notice with a "wait until the chat is free" mechanism of its own, separate from the queue the chat host already has for exactly that job. So Orca had two waiting mechanisms that could disagree about when a chat is free. The goal of this PR is that agent-to-agent messages reuse the chat's own send and queue, and that each queued message records who sent it.

What changes for you. Nothing visible. The notice is still not shown while it waits, and it is still sent once, when the chat is free. Underneath, it now waits in the chat's own queue on the host, and the queue checks it at the moment it would send it: it is dropped if the agent already read that mail, or sent with an updated count if more mail arrived. Terminal agents are unchanged.

What the review changed. The first version showed the notice as a queued card that looked like a message you had typed. Agents usually read their mail mid-turn, so that card would appear and then vanish. The PR now:

  • Hides the notice from the queue view until the next step (B), which shows who sent each message. Until then it can't be steered or deleted, and it still counts toward the queue's 20-message limit (labelled Temporary in the body).
  • Checks the notice when the queue is about to send it. This replaces a cleanup step that ran inside orca orchestration check and missed several cases. check is now identical to main.
  • Fixes eight bugs from the first version:
    • /clear sent the notice twice.
    • After a restart, a notice queued behind one of your paused messages was never sent.
    • A Stop held the notice only until the next mail arrived.
    • New mail moved the notice to the back of the queue, where the steer shortcut could pick it.
    • An error in the cleanup step could fail check.
    • orchestration reset left an orphaned notice.
    • Stale notices were sent after reply or a filtered check.
    • A notice already handed to the agent was never recorded as delivered.
  • Fixes three issues the review's own changes introduced:
    • A failed save could strand a hidden notice for good.
    • Stop didn't stay stopped while the agent was starting on a notice.
    • A hidden notice could go ahead of a message of yours.
  • Changes the saved shape:
    • The record of who sent a message is now named for the message, not the queue, so B can reuse it.
    • Each kind of message gets its own fields, ready for the later task-message PR.
    • It no longer stores a host id or a pane key.

Deferred (P3, recorded, not fixed here):

  • The hidden notice counts toward the 20-message limit (Temporary, B).
  • /clear can be refused with "A message you sent earlier isn't confirmed yet" while Orca's own notice is unconfirmed. Main does the same (Temporary, B).
  • A notice left waiting at quit has no wake-up of its own. No real startup order reaches this.
  • A small extra read of the chat's history once per finished turn, only while the agent holds opened, unacknowledged mail.
  • After a downgrade, an older build shows a waiting notice as your own card.
  • Three comment nits.

Verified:

  • Four review rounds: a full adversarial review, an architecture challenge reviewed against the common pattern, then focused re-checks of each fix. A readiness checklist passed with no P0/P1.
  • Every new behaviour has a test that was proven by breaking the code and watching it fail.
  • Tests: 259 related test files (3,446 tests) plus the 15 cross-version files. Typecheck (node and CLI) and lint pass. CI is green on this head.
  • Live QA on a Mac at this head (5a21eb5): a real Claude chat with an isolated profile, Grok driving, and a fresh chat per scenario with its own run. Each result was checked against the chat's queue rows and the mail records.
Scenario Result
Mail to an idle chat Pass: one notice at once, recorded as delivered
Mail to a busy chat Pass: nothing shown mid-turn; exactly one notice when the turn ends, recorded as delivered
Two mails during one turn Pass: one notice, counting 2; both mails recorded as delivered
Agent reads the mail mid-turn Pass: the notice was dropped unsent; no notice over two minutes
Stop during the turn Pass: one notice once the turn stopped, same as main
Quit and relaunch with a notice waiting Pass: the notice was still waiting after the quit; one notice after relaunch with no action needed

Busy chat, mid-turn: nothing shown while the notice waits.
Busy chat mid-turn, no card

The same chat after the turn: exactly one notice.
One notice after the turn

Two mails during one turn: one notice, counting 2.
One notice counting 2

The agent read its mail mid-turn: no stale notice afterwards.
No stale notice

Quit and relaunch with a notice waiting: sent once after relaunch. "Failed after 1m 52s" is main's label for a turn cut off by quitting.
Notice after relaunch

Not verified:

  • Live: a Stop pressed just as the agent starts on a notice. Both live Stops landed before a notice turn began; tests cover this case.
  • Your own queued messages next to a hidden notice, live. The queue for your messages is still switched off for clients, so it can't be exercised in the app; tests cover it.
  • Remote (SSH) chats. The mail lane only serves chats on the local machine, which was confirmed in code.

…ued like a person's message

A structured chat used to get a derived "You have N orchestration messages,
run check" notice, which could go stale while it waited in the chat's queue,
so the queue judged, restated or withdrew it as it sent. It now gets the mail
itself as the turn, through sendAgentTurn with delivery 'queue', the path a
person's message takes.

- One turn per mailbox batch: the mailbox's unread mail at delivery, in mail
  order, each message led by "[message from <sender>]", its type, subject,
  body, payload and reply hint (what check prints). Mail arriving while that
  card is still in the queue waits for the next card; a card is never edited.
- An idle chat takes it at once; a busy one queues it as an ordinary card the
  person sees and can Steer or delete. Mail is marked read when the chat
  accepts the turn, so check does not return it again; a deleted card leaves
  its mail unread for check, and it is not pushed again.
- The card stores who it is from (source_json): every distinct sender, and
  each message's id, run and sender. Kind 'mail-notice' becomes 'mail'.
- Removed: the send-time judge (QueuedAgentCardVerdict), structured-mail-
  notice, structured-pointer-notice-cards, queued-message-restatement, the
  hidden-card rules, the agent-card exemptions from the restart, Stop and
  /clear pauses, the dropped-card hand-back, and the drain split. An agent's
  card now follows the same pauses as a person's.
- Terminal agents are unchanged: they keep the typed pointer and check.
…estarts hold only the person's cards

- When a structured chat runs `orchestration check` (consuming, --peek or
  --wait), mail an agent's card in that chat's own queue still carries
  (any card not deleted) is left out: it is on its way as a turn, so the
  agent does not read it twice. Derived from the queued rows at read time;
  a deleted card no longer carries it, so it comes back. Terminal callers
  and every other read are unchanged. New batches pass the exclusion to
  getOrCreateMailboxDelivery; peeks filter it.
- A restart's queue pause now holds only the person's cards: an unattended
  coordinator's agent card sends after an app restart without waiting for a
  Resume, since the mailbox is the record and reading is marked. Stop and
  /clear still hold agent cards. One rule, in queuePauseHolding. An agent
  card queued behind the person's held card still waits behind it: the
  queue never reorders.
- The duplicated direct-mailbox snapshot routing in check-run and
  check-worker becomes one helper, which keeps check-worker under its line
  limit.
@brennanb2025 brennanb2025 changed the title feat(orchestration): agent mail to a busy chat waits in the chat's own queue feat(orchestration): an agent's mail reaches a chat as a message, queued like yours Oct 4, 2026
…ssages-via-queue

# Conflicts:
#	src/main/native-chat/agent-session-journal/journal-queued-messages.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts
…at's check takes its waiting cards

A card held an exclusive claim on its mail that nothing reconciled with the
mailbox: queueing it stamped the mail delivered, and a chat's check hid every
card that was not deleted. So a chat's own check --wait in one turn could not
see a result its waiting card held, a hand-off that ended in doubt or came back
"Not sent" stranded its mail, a check racing the card's build read the mail
twice, and a card left behind by run-use still sent and marked it read.

What a card or send holds is now derived each time from the chat's queue and
its sends: an accepted send marks its mail read; a waiting card or an
unanswered send holds its mail; everything else unread is pushed again or
returned by check. A card the person deleted is recorded as not to be pushed,
at the delete. The lane withdraws returned, partly read and moved cards. A
chat's consuming check withdraws the waiting cards holding what it reads, one
at a time with the lane. The sender is stored on the sent submission too
(host-only), so read state survives the card row's prune. A refusal before
anything started ends its operation; a restart pause is raised only by the
person's cards; an unreadable agent source stays an agent's.

@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.

Important

A Run that moves to another chat while the old chat's mail card has already been handed off is not stopped: the old chat still runs the message and the new owner is sent it too, so the same result reaches two agents. Details inline.

Reviewed changes

Since the prior pullfrog review (5a21eb5), the branch replaces the hidden mail-notice card with "the message itself rides the send/queue path a person's message uses" (600b4320, 8171ed09, 5e90c4db):

  • A chat's mail state is derived each pass. New structured-chat-mail.ts computes what a chat's cards and sends still hold (reconcileChatMail, takeChatMail), marks an accepted send's mail read and a deleted card's mail "not now", and lets a consuming check take a waiting card's mail and withdraw the card.
  • The lane sends the message, not a pointer. structured-mailbox-pointer-delivery.ts sends the batch as the turn itself (formatMessageTurn, delivery: 'queue', host-local source): a busy chat queues it as a visible card beside the person's own, an idle one runs it.
  • Sender persistence. New agent-session-message-source.ts plus a nullable queued_messages.source_json and a host-only submission source; a hand-off copies the card's source, and an unreadable value still reads as an agent's.
  • Move handling. run-use now reconciles the old chat and delivers to the new owner (releaseMovedCard / deliverStructuredMailForHandle).
  • Restart pause narrowed. Only a person's pre-restart card holds; an agent's card sends without a Resume, while Stop and /clear still hold it.
  • Check and DB plumbing. previewMailboxDelivery / excludeMessageIds; the check paths consume or peek through a new QueuedChatMail; direct-mailbox-snapshot.ts extracted.

The focused suites are green at HEAD (161 tests across the coordinator-mail, delivery, host, check and pause suites).

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

Comment thread src/main/runtime/orchestration/structured-mailbox-pointer-delivery.ts Outdated
…s, sent through the chat's own queue

A structured chat now receives exactly what a terminal agent is typed: "You have N orchestration
messages ... run `orca orchestration check`" (formatMessagePointer, same CLI name). It goes
through the shared sendAgentTurn with delivery 'queue', the composer's queue-if-active: an idle
chat gets it at once, a busy chat's queue holds it as a normal card and sends it when the turn
ends. The lane's idle gate is gone; the send decides, as for the person's message.

The lane queues no second pointer while one of its cards still waits, read from the queued rows;
mail arriving meanwhile, or after the card is sent, is pointed again as the terminal lane points
new unread mail. A queued pointer counts as delivered, as an accepted one does. `check` and mail
read state are main's: only `check` reads mail.

The card records who it is from (queued_messages.source_json, kind 'agent', message
'mail-notice', with its senders) for the next PR to render. A restart's pause is raised by and
holds only the person's cards; Stop and /clear still hold an agent's.

Removed from the earlier designs: the message-as-turn batching, the check exclusion and taking,
the derived claims, lock and reconciliation, the sender on submissions, and their tests.

@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.

Important

The PR title and description still describe the "the message itself rides the send/queue path" design, but the head commit reverts the lane to sending the pointer a terminal agent gets. Several behaviors the description promises — the chat receiving the message itself, a "Not sent" card being removed and its message re-sent automatically, and a moved Run withdrawing the old chat's card — are not implemented by the code at HEAD.

Reviewed changes

Since the prior pullfrog review (5e90c4db), 8f2904d reverts the message-itself design back to the pointer:

  • The lane sends the pointer again. structured-mailbox-pointer-delivery.ts builds formatMessagePointer and sends it with delivery: 'queue', so a busy chat holds it as an ordinary card; formatMessageTurn, the batch-as-turn body, and the read/derive machinery (structured-chat-mail.ts, structured-agent-session-agent-mail.ts) are gone.
  • Session facts replace the idle gate. readGateFacts/decideStructuredSessionPointerDelivery are removed; readSessionFacts returns the recorded submissions plus pointerCardWaiting, and a waiting agent card retains with queued rather than gating on turn state.
  • Source persistence narrows to the card. queued_messages.source_json keeps the versioned AgentMessageSource, but the schema stores only mail-notice and reads any unreadable value as the person's; structured-mail-source.ts names the senders; the host-only submission source and the check-time card withdrawal are deleted.
  • Restart pause narrowed. A restarted pause is raised only by a person's waiting card from another host instance and holds only the person's cards; an agent's pointer card sends without a Resume.

⚠️ The title and description describe a feature that is no longer in the diff

The body's opening ("An agent's message to a chat now arrives as the message itself, as one turn … the message's type, subject, body, any structured payload") and its "What this changes for you" and "Differences from the common pattern" sections describe the design that 8f2904d removes. At HEAD a chat is sent You have N orchestration messages. Run \orca orchestration check …``, not the message, and the card in the queue holds that pointer text, not the mail.

Technical details
# PR description vs. implementation

## Affected sites
- PR title ("an agent's mail reaches a chat as a message") and body "What this changes for you" — describe the chat receiving the message type/subject/body/payload.
- Body "If an agent's card comes back 'Not sent' … Orca removes that card and sends the message again by itself" — no such handling exists at HEAD; a queued card is marked `delivered_at` at queue time and a `returned`/`send_failed` agent card is not re-pointed.
- Body "If the Run moves to another chat (`orchestration run-use`) while an agent's card waits in the old chat, Orca removes that card and the chat that now owns the Run gets the message" — the net diff contains no `run-use`/`deliverStructuredMailForHandle` change.

## Required outcome
- The title and description should describe what ships at HEAD: a chat gets the PTY pointer, queued through its own queue; the agent reads the mail with `check`. Remove or restate the message-itself, auto-resend, and run-use-move claims.

ℹ️ Mail is marked delivered when the card is queued, not when it is taken

attempt calls db.markAsDelivered(staged) as soon as the host reports the pointer queued (or accepted). If that card later comes back returned or is held send_failed (for example the provider starts failing between the turn ending and the hand-off) and no further mail arrives, the lane will not re-point it — getUndeliveredUnreadMessages excludes delivered_at — so an unattended agent gets no nudge until a person Sends the card. A later new-mail pointer does carry the old mail too and check still returns it, so this is a delay rather than a loss; please confirm the unattended case is acceptable, since the description's auto-resend promise is not backed by the code.

Technical details
# A queued-and-dropped pointer card is not re-pointed

## Affected sites
- `src/main/runtime/orchestration/structured-mailbox-pointer-delivery.ts` — `outcome.kind === 'queued'` falls through to `db.markAsDelivered(staged)` at the same site as an accepted `now` send.
- `src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts` — the drain holds a card `send_failed` on a pre-consume failure (no automatic retry), and a `returned` card is skipped by `nextSendableQueuedCard`.

## Open questions for the human
- Is the queue-time `delivered_at` intended (the card itself is the nudge, and a drop is recovered by `check` / later mail), or should the lane re-point a returned or held agent card as the PTY lane retries on the next idle edge?

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

…ueue unsent

The lane holds new mail while a pointer card waits, and retried it only on the chat's next
status change. A card that leaves the queue without a turn after it (the person deletes it
while Stop holds it, or its hand-off comes back) made none, so that mail sat unpointed until
something unrelated happened. The shared queue wiring now tells its host when an agent's card
stops waiting, read only when the queue changed, and the runtime redrives that chat's parked
mail. The runtime forwards the new host dep like the others.

Tests: that case end to end, and /clear carrying an agent card keeps who it is from. Agent card
bodies in tests are the pointer text; the restart pause's header says why an agent card may send.

@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.

Reviewed changes

Since the prior pullfrog review (8f2904d), 055e27d makes a chat's parked pointer mail follow a card that leaves the queue without a turn after it:

  • Card settle redrives parked mail. structured-agent-session-queued-wiring.ts detects when a waiting agent card left the queue (agentCardSettled, gated on the journal's queued-message revision) and calls the new StructuredAgentSessionHostDeps.onAgentCardSettled. The runtime forwards it to notifyStructuredSessionJournalActivity, which re-delivers mail parked while that card waited. Previously only a status change redrove it, so a delete (or a returned hand-off) with the chat idle left that new mail unpointed.
  • New end-to-end coverage. A test deletes a Stop-held agent card with the chat idle and asserts the mail that arrived while it waited is pointed; a /clear test asserts the carried agent card keeps its sender source and waits under the cleared pause. Pause-test agent card bodies now use the real pointer text.
  • Wiring. onAgentCardSettled added to StructuredAgentSessionHostDeps and StructuredAgentSessionRuntimeDeps, wired in orca-runtime-get-worktree-ps.ts and the coordinator-mail rig.

Verified the delta suites locally (the new delete-repoint and /clear tests, plus the coordinator-mail, delivery, host, session-pointer and queued-messages suites): 97 tests pass.

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

@brennanb2025 brennanb2025 changed the title feat(orchestration): an agent's mail reaches a chat as a message, queued like yours feat(orchestration): a native chat gets the orchestration pointer a CLI agent gets, through the same send as your messages Oct 5, 2026
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Status: ready for review at 43e90b5

The problem. A native chat (structured session) did not get orchestration mail the way a CLI agent does. The code that tells an agent about new mail had its own "is the chat busy?" rules and its own send, outside the chat's send button and queue. While the chat was busy you saw nothing, and Orca sent the notice later on its own.

What changes for you. A native chat now gets exactly what a CLI agent gets: "You have N orchestration messages. Run orca orchestration check." It comes through the same send as your messages. An idle chat gets it at once. A busy chat shows it as a normal queued card (Steer, Delete) that is sent when the turn ends. Stop holds it under "Queue paused because you interrupted", with Resume. After a restart, an agent's card sends without waiting for you. check, and what counts as read, are unchanged. Terminal agents are unchanged.

v3-s1-card-midturn.jpg

v3-s1-after-pointer-and-check.jpg

v3-s6-after-stop.jpg

How we got here. The earlier version sent the mail itself into the chat as the turn, and tracked which mail each card carried. Review found cases where mail was hidden from check or delivered twice. The design was then changed to the simpler rule: give a chat exactly what a CLI agent gets, through the chat's own send. The branch was rebuilt to that (8f2904d), which removed the earlier tracking, hiding and re-checking code. A narrow review of the rebuild found two things, both fixed in 055e27d. First, a notice card deleted while Stop held it left later mail without a notice until something unrelated happened; the chat's queue now tells Orca when an agent's card stops waiting. Second, /clear keeping an agent card's sender had no test; one was added. Main is merged in (0a8c572).

Accepted for now (also in the PR body, each labelled intended or temporary):

  • A waiting card's count can be out of date. check returns all the mail. (intended)
  • A burst of mail can produce one extra notice whose check finds nothing new. (intended)
  • A deleted card's mail is not pointed again, but check still returns it. (intended)
  • A notice waits as a card behind a turn whose start Orca could not confirm, as your own message would. (intended)
  • Mail after an unacknowledged check is not pointed until it is acknowledged. This is the same as before, and the same for terminal agents. (intended)
  • Queueing your own messages is off for chats in this build, so your busy-time message goes into the running turn while the agent's notice queues. (temporary)
  • The notice shows as a right-hand bubble like yours until the next PR labels the sender. (temporary)

Verified.

  • Automated tests cover each behaviour, and each was checked by removing its code and watching it fail.
  • tc:node and tc:cli pass. The orchestration and native-chat test directories pass in full: 443 files, 3810 tests.
  • Lint, the code-quality gate and the reliability gates pass.
  • The readiness review passed with no P0, P1 or P2 findings.

Live run on macOS against a real Claude chat (at 8f2904d; the later fix 055e27d only adds the card-stops-waiting signal and is covered by tests, not re-run live):

Case Result
S1 busy chat: one card, sent once at turn end, then check Pass
S2 idle chat: the notice at once Pass
S3 three messages in one busy turn: one card Pass (then one extra notice, accepted above)
S4 delete the card: not sent later, check still returns the mail Pass
S5a restart with only an agent card: sent without Resume Pass
S5b restart with your card and an agent card Blocked: your messages do not queue in this build, so there was no card of yours to hold
S6 Stop holds the card; Resume sends it Pass

Not verified. After a crash restart, that the agent itself starts again in the real app. The tests show the card is handed to it at startup with no window open.

…ssages-via-queue

# Conflicts:
#	src/main/runtime/structured-chat-coordinator-mail.test.ts
… chat queue

A structured chat's orchestration notice is now sent through the chat's own send
like any message: an idle chat takes it as a turn, a busy chat's queue holds it
and sends it at turn end, under the same Stop, restart and /clear pauses as the
person's cards. Native chat no longer branches on who a card is from; the card
only records it.

- Restore main's queue pause logic (no restart exemption for agent cards).
- Remove the queue watch that redrove mail when an agent card left the queue,
  and the host's queued-row read the lane used for it.
- The lane reads nothing of the chat's queue. It sends no second notice while
  mail it already pointed at is unread, read off the mailbox alone.

@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.

Reviewed changes

Since the prior pullfrog review (43e90b51), faffb2b removes the special handling agent mail notices had in the chat queue:

  • One pointer decided from the mailbox alone. structured-mailbox-pointer-delivery.ts replaces the pointerCardWaiting gate with db.hasPointedUnreadMessages(mailboxHandle) (new query in message-inbox.ts: read=0 AND delivered_at IS NOT NULL AND delivery_contract='current_delivery'), retaining pointer-unread. Mail is marked delivered_at when the pointer is queued or accepted, so a queued notice suppresses further notices until the agent's check reads that batch and the turn-end journal edge re-drives the mailbox.
  • The card-settle redrive is gone. onAgentCardSettled (agentCardSettled plus the WeakMap of waiting agent-card ids), queuedMessageRows on the host, and their wiring through StructuredAgentSessionHostDeps, StructuredAgentSessionRuntimeDeps, orca-runtime-get-worktree-ps.ts and the coordinator-mail rig are deleted; the mailbox rule subsumes them.
  • A queued send is a queued outcome, not unknown. structured-mailbox-pointer-host.ts maps case 'queued' to { kind: 'queued' } and consumes the mail there; readGateFacts/StructuredPointerGateFacts and the queued retain reason are removed (the shared gate facts survive only for @idle).
  • Restart / /clear special handling reverted to base. queued-message-pause.ts is identical to main again, so a restart pause holds every waiting card from a dead host instance, not only the person's.
  • Tests updated. The delete-then-repoint e2e test and the agent-card restart / /clear pause tests are removed (their behavior no longer exists); new coverage asserts the unread gate, the queued hand-off, /clear source carry, and the restructured readSessionFacts.

All prior Pullfrog threads are resolved, and the PR description now matches the shipped pointer/queue design, closing the earlier title/body finding. Verified locally at HEAD: the delta suites (168 tests) pass, and pnpm tc:node / pnpm tc:cli are clean.

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

…ssages-via-queue

# Conflicts:
#	src/main/runtime/structured-chat-coordinator-mail.test.ts
…r pointed mail is unread

Drops the native-chat-only rule that held back a notice while the agent had not yet read mail it
was already pointed at. Also fixes main's stop-note test, which still built a provider handle in
the shape #24991 replaced.

@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.

Reviewed changes

Since the prior pullfrog review (faffb2b), 9f2004b drops the native-chat-only "one pointer at a time" rule so a chat is pointed at new mail exactly as a terminal agent is:

  • Removed the pointed-unread gate. structured-mailbox-pointer-delivery.ts no longer consults hasPointedUnreadMessages before selecting a batch, and the pointer-unread retain reason plus the message-inbox.ts query are deleted. New mail arriving while earlier pointed mail is still unread now gets its own pointer through the normal delivery path, counting only the not-yet-delivered mail; delivered_at is still stamped at queue time, so no message is pointed twice.
  • Tests updated to the new rule. The delivery unit test asserts a second pointer fires only for genuinely new mail and counts one message, and the coordinator-mail queue e2e asserts two cards sent as two turns before the queue empties. Both fail under the removed gate.
  • Fixed a stale test handle. structured-agent-session-stop-note-opened-turn.test.ts now builds codexProviderHandle('thread-1'), matching the shape #24991 introduced and unblocking the suite on this branch.

Verified locally at HEAD: structured-mailbox-pointer-delivery.test.ts (19), structured-chat-coordinator-mail-queue.test.ts (3), and structured-agent-session-stop-note-opened-turn.test.ts (2) pass — 24 tests. The PR body now documents the resulting cadence (several cards per busy turn, each "You have 1 orchestration message") and divergences from the terminal lane. All prior review threads are resolved.

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

@brennanb2025
brennanb2025 merged commit dd39dcb into main Oct 6, 2026
30 of 33 checks passed
brennanb2025 added a commit that referenced this pull request Oct 6, 2026
brennanb2025 added a commit that referenced this pull request Oct 6, 2026
…as another build would

#25078 made a card's source required, so the tests that insert a card pass the
person's. The hold's unknown-kind and unreadable-source cases now rewrite the
stored row the way a newer build would leave it, instead of casting a type.
brennanb2025 added a commit that referenced this pull request Oct 6, 2026
… held card (#24660)

* fix(native-chat): keep a message accepted before a quit or crash as a held card

A send the host accepted while the agent was still starting, and never handed
over, was rejected unseen at quit or at the next open after a crash. The next
open now keeps a person's message (typed, or a launch's first prompt) as a
waiting card at the head of the queue, held until Resume, Send now, Edit or
Delete; quit no longer rejects it. Each submission records its source so a
restart knows which leftovers to keep. A direct send's replay answers from its
own record, never the queued arm. Cards shown without the queue capability
hide Turn off queueing and the steer chord.

* fix(native-chat): keep an unsent message as a held card at every close, not only a restart

The host now keeps a person's message it accepted and never handed over with one
rule wherever it can no longer hand it over: a quit or crash (settled at the next
open) and a close of the chat (tab close, worktree teardown, orchestration stop).

- The hold is card state: a new per-card hold_reason 'kept', published as the
  existing pausedReason, instead of a fake host_instance value. host_instance
  means the owner again, and the pause clause, adoption filter and /clear carry
  special cases are gone. A kept card holds the cards behind it until the
  person sends, edits or deletes it; a send_failed card still does not.
- A card hand-off rejected by a restart or a close returns as a kept card
  (rejectedDraftSettlement), so a person's next message can never release it.
- One hold function, parameterized by cause (hostRestarted / chatClosed),
  replaces the close path's plain rejection.
- The phone shows published cards and per-card holds whatever the queue
  capability says; only queueing a new send stays gated.
- source gains 'dispatch' for the orchestration preamble (still rejected); an
  unknown source is kept as written and never takes the legacy rule.
- Kept cards from earlier settlements stay ahead of a batch's new ones.

* test(native-chat): the dispatch preamble records its source

* fix(native-chat): skip a kept card like a failed one, and make a quit leave the queue as a crash does

- A kept card is held on its own, as a send_failed one is: the queue sends the
  cards behind it, and the "a message ahead needs attention" caption no longer
  appears behind it (desktop and phone).
- Quit disposes the queue's drain together with delivery, so it mints no
  hand-off that only the next process could settle.
- Only a Send the person asked for (origin client) that a restart or close cut
  short returns kept; the queue's own hand-off returns where it stood, under the
  restart's pause, as on main.
- Comments that said only a capable host gets cards or the card actions now say
  the capability gates only queueing a new send.

* refactor(native-chat): one dispose gate in the queue drain's step

* test(native-chat): the downgrade test names the older build's /clear exception

* fix(native-chat): re-check the drain's quit gate right before it appends a hand-off

A drain step already past its first check when quit begins no longer makes a
hand-off. Adds regression tests for that, for Send now on an ordinary card cut
short by a quit, and for the open-time repair deriving kept from the hand-off's
origin; fixes the pause and settlement comments that called a kept card one the
queue never passes.

* fix(native-chat): a card held on its own starts no restart pause for the others

After a second restart a kept (or send_failed) card is another process's, but it
waits for its own Send, so it no longer pauses every other card under
"Queue paused because Orca restarted".

* fix(native-chat): record on a kept send which card holds it, so no trace survives an Edit or Delete

The rejection row of a send the host kept as a card now names that card
(`keptAsQueuedMessageId`), in the same transaction that writes the card, and the
fold publishes it on the submission. The shared projection draws such a send
only as its card: once the card is sent, edited or deleted, neither the send
nor the sending desktop's local copy of it shows.

* chore(native-chat): leave the unused submission schema as main has it

No client parses published submissions with it (they arrive as typed frames,
and the host's history pages carry the field, as the Edit/Delete test reads);
listing the field put the file over its line budget.

* fix(native-chat): retire a kept send's local copy instead of only hiding it

The outbox reconcile and the send disposition drop an entry whose submission
the host rejected as kept as a card, as they already do for a Stop's
withdrawal, so the copy never comes back as "Not sent / Retry" once the
submission falls out of the loaded page. The projection reads the reconciled
outbox, so its separate filter goes.

* test(native-chat): move the queued-message rig's scripted provider into its own fixture

The rig fixture grew past the 300-line limit once main's changes merged in.

* fix(native-chat): hide a kept send by its own record, not by its card still existing

The transcript hid a rejected send while a queued card held it under its id, so the card's Edit or
Delete brought back a "Not sent" row. It now reads the send's own keptAsQueuedMessageId, which the
host records with the rejection, and the live card list is no longer threaded to the transcript or
the delivery notices.

* refactor(native-chat): move a sent message's row writes into their own journal collaborator

The journal store went past its line limit once main's ledger receipt joined this branch's
transaction hook. The submission and dispatch-transition writes, and what commits in their
transaction, now live in JournalSubmissionWriter; the store's methods delegate to it unchanged.

* fix(native-chat): list a kept send's card id in the submission schema

The schema drops keys it does not list, so a reader that kept a parsed submission would lose
keptAsQueuedMessageId and source, both persisted with the row. The submission schema and the
failure fact it shares with item bodies move to their own modules, with room for both fields.

* test(native-chat): match the transcript and outbox hook signatures main and the swap changed

* test(native-chat): import the journal types once in the queue-delivery test

* fix(mobile): a resend the host kept as a card shows no error and returns no text

The host answers a resend of a message it kept as a card with that
message's rejected submission, marked keptAsQueuedMessageId. The phone read
it as any rejection: "Message not sent" and the text back in the composer,
while the card showed the same text. It now answers like a queued send, as
the desktop's send disposition does: the id is spent, no error, and the card
holds the text.

* test(native-chat): pin that quit's first step stops the queue's hand-off

Quit now stops delivery, the queue drain included, at its first step
(stopDelivery), before teardown drains recovery. The drain-step quit test
runs from that step as well as from the flush.

* test(native-chat): give cards their source and store unknown sources as another build would

#25078 made a card's source required, so the tests that insert a card pass the
person's. The hold's unknown-kind and unreadable-source cases now rewrite the
stored row the way a newer build would leave it, instead of casting a type.

* refactor(native-chat): stop delivery and the queue drain in one line at quit

Main's #25159 left the host at its line limit; quit's stop now disposes both in one
expression instead of a block.

* test(native-chat): hold the drain step without reading the call stack

Bun formats a method's stack frame without its class ("at step"), so the quit
test's caller check never matched, the step was never held, and both cases timed
out once CI ran Vitest on Bun (#25840). Only the drain step heals owed queue
bookkeeping, so the hold needs no caller check.
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