Repository navigation
fix(orchestration): native-chat agents get their mail notice right away, the same way for every agent - #28540
Conversation
|
A busy native-chat coordinator previously waited until its entire turn ended to see worker-mail notices, because they became queued cards. This PR sends the notice through the ordinary immediate-send path for every agent. Agents that restart for a new message also restart for mail, just as they do for a person's message; terminal delivery is unchanged. Mail remains unread while the host sees a pending approval or question, without creating a queued card or an early chat message. The fresh review reproduced two races in that hold: a prompt opening after an earlier snapshot could be cancelled by mail, and an answer arriving before retry registration could leave mail waiting for an unrelated event. Commit The common pattern uses current-owner admission and rechecks remaining human interactions after settlement. Intended differences: all providers use ordinary immediate send, including interruption where that is how a person's send behaves; held mail stays invisible and unread. A prompt opening after host admission retains the same provider-handoff timing window as a person's ordinary send. This review does not claim stronger provider-level atomicity. Fresh correctness and readiness source reviews found no remaining proven defect. The final Sol xhigh decider independently traced the final production path and withdrew the earlier retry-loop and map-fanout concerns: no correctness defect or material resource regression is proven. Repeated prompt activity and large concurrent mailbox/history workloads remain deferred, unmeasured scale concerns. Verified on the committed correction: four targeted Node suites, 52 tests passed, and the mandatory commit hook passed after consolidating duplicate missing-session/fence handling and redundant park ownership checking. The ACP host contract proves that a pending question holds mail without accepting a message, writing a send receipt, or cancelling the prompt. A delivery-lane contract proves one release after an answer arrives during the held send; a separate production-host fixture traces a real answer and one notice. These automated fixtures do not establish a complete real ACP provider exchange or rendered behavior. CI: PR Checks run 38816106437 has static analysis and typecheck passing. The Windows package job failed in the existing installer process probe at Real-agent Electron QA is blocked. The M4Air and Windows attempts ended before launching Orca because no authorized signed-in agent was available in the isolated test homes. No current-head screenshots or real-provider rendered flows were produced. Busy native delivery, approval/question hold and post-answer delivery, and ACP behavior still need authenticated remote QA; older fixture screenshots below cover an earlier revision only. The PR remains a draft and is not ready to merge. Historical controlled-provider screenshots — old head 4f1d256, not current-head QAThese images were captured with an isolated Codex fixture on M4Air during the earlier design review. They show the queued baseline and immediate notices, but do not validate the final human-prompt hold or authenticated providers. Nothing was merged or synchronized with main during this correction. The separate mail-content PR #28567 remains outside this review and unready. QA on b4cd51cReal signed-in Codex (GPT-6.1-Sol) and Grok ACP (Grok 4.7), driven by
The Grok question proof used a fresh Run on the same chat, excluding an outstanding unacknowledged delivery from the earlier busy test. Mail came from real test-app shell workers through their own Orca CLI; no provider fixtures or injected rows were used. Both QA source worktrees, throwaway homes/profiles, test apps, and daemons were removed; their ports have no listeners. No PR source was edited, pushed, or merged. No lint or typecheck command was run. Real-file hashes are reported without attributing concurrent shared-machine changes: real Codex Real configuration SHA-256 before / after
This is rendered QA evidence only; it does not change the CI merge gate or mark the PR ready. Final sync (4495dbd)Main merged at the end (current main, after #29809 fixed main's lint break from #28729). The merges add only main's changes plus whitespace-only reformatting of three main files by the commit hook; no change to this PR's logic since the reviewed head |
…ing turn A native-chat coordinator's mail notice always queued as a card until the whole turn ended, so a coordinator busy in a long command never saw its workers' reports. Agents that take a message mid-turn (Claude, Codex, Pi) now get the notice at once; agents that would cancel their turn for it (ACP) still queue it.
e9394b6 to
abbb2ea
Compare
abbb2ea to
4f1d256
Compare
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
A mail notice is a plain send for every agent, as a person's own mid-turn message is. While a pending approval or question waits on the person it is held, and the turn's working edge after the answer sends it.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
a968857f-25f4-4080-992a-0c51b19f7b9d
📒 Files selected for processing: 7
src/main/runtime/orca-runtime-structured-status-sink-wiring.test.tssrc/main/runtime/orchestration/agent-facing-parity.test.tssrc/main/runtime/orchestration/structured-mailbox-pointer-delivery.test.tssrc/main/runtime/orchestration/structured-mailbox-pointer-delivery.tssrc/main/runtime/orchestration/structured-mailbox-pointer-host.test.tssrc/main/runtime/orchestration/structured-mailbox-pointer-host.tssrc/main/runtime/orchestration/terminal-pane-mail-routing.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.
# Conflicts: # src/main/runtime/structured-chat-coordinator-mail-queue.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments: 1
src/main/runtime/structured-chat-coordinator-mail.test.ts: 1
459-497: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the answer-triggered retry on the coordinator path.
acp-structured-mail-admission.test.tscovers a real ACP question and held mail, but it stops at{ kind: 'held' }.structured-chat-dispatch-worker-mail.test.tscovers provider failure and retry on a later turn. The coordinator test covers active-turn steering only.Add a focused test that answers the provider question after mail is held and asserts one coordinator retry. This can catch answer-to-retry wiring regressions that the existing tests miss.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ff2fcaef-3ae7-4bbb-9b53-9a1fc8144c26
📒 Files selected for processing: 1
src/main/runtime/orchestration/send-agent-turn.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
…28540's hold Each orchestration message still reaches a native chat as its own message, now through the merged plain send for every agent and its admission hold: a message held for a waiting prompt stops the pass so later mail cannot overtake it, and the mailbox is parked before the first read. Heartbeats are skipped in this lane only and stay unread. A mailbox with an unacknowledged check batch waits for its ack, as on main. Chat text is capped at 4,000 characters with the command that shows the whole message.
…worker A coordinator's follow-up to a chat-mode worker (Chat UI on) always reached it as a queued card that waits for the worker's current turn to end. `orca orchestration send --deliver now` stores the request on the message row, and the structured pointer lane sends the batch's nudge with delivery `now`, which joins the running turn the way the composer's Steer button does. The default stays `queue`. - New `messages.pointer_delivery` column (schema v44, defaults to `queue`). - Delivery is read off the batch's own rows, so a retry under a reused operation id always resends with the delivery it was minted with (delivery is part of the host's operation fingerprint). - Carried through group sends and both federation relay directions as an optional field written only for `now`, so plain mail is byte-identical for older peers. - Terminal (PTY) recipients are unchanged: they are still nudged at the next idle prompt, since typing into a busy agent TUI can land in a permission prompt. - CLI help, flag help, and the orchestration messaging guide document it. Ported onto upstream stablyai#28540, which already steers every mail notice into a busy chat's running turn (held only while an approval or question waits). The pointer lane keeps that upstream behavior for every row; the `--deliver` flag, the v44 column and the federation field stay so existing scripts and databases keep working, and the help text says a row's `queue` no longer holds a nudge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…worker A coordinator's follow-up to a chat-mode worker (Chat UI on) always reached it as a queued card that waits for the worker's current turn to end. `orca orchestration send --deliver now` stores the request on the message row, and the structured pointer lane sends the batch's nudge with delivery `now`, which joins the running turn the way the composer's Steer button does. The default stays `queue`. - New `messages.pointer_delivery` column (schema v44, defaults to `queue`). - Delivery is read off the batch's own rows, so a retry under a reused operation id always resends with the delivery it was minted with (delivery is part of the host's operation fingerprint). - Carried through group sends and both federation relay directions as an optional field written only for `now`, so plain mail is byte-identical for older peers. - Terminal (PTY) recipients are unchanged: they are still nudged at the next idle prompt, since typing into a busy agent TUI can land in a permission prompt. - CLI help, flag help, and the orchestration messaging guide document it. Ported onto upstream stablyai#28540, which already steers every mail notice into a busy chat's running turn (held only while an approval or question waits). The pointer lane keeps that upstream behavior for every row; the `--deliver` flag, the v44 column and the federation field stay so existing scripts and databases keep working, and the help text says a row's `queue` no longer holds a nudge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…worker A coordinator's follow-up to a chat-mode worker (Chat UI on) always reached it as a queued card that waits for the worker's current turn to end. `orca orchestration send --deliver now` stores the request on the message row, and the structured pointer lane sends the batch's nudge with delivery `now`, which joins the running turn the way the composer's Steer button does. The default stays `queue`. - New `messages.pointer_delivery` column (schema v44, defaults to `queue`). - Delivery is read off the batch's own rows, so a retry under a reused operation id always resends with the delivery it was minted with (delivery is part of the host's operation fingerprint). - Carried through group sends and both federation relay directions as an optional field written only for `now`, so plain mail is byte-identical for older peers. - Terminal (PTY) recipients are unchanged: they are still nudged at the next idle prompt, since typing into a busy agent TUI can land in a permission prompt. - CLI help, flag help, and the orchestration messaging guide document it. Ported onto upstream stablyai#28540, which already steers every mail notice into a busy chat's running turn (held only while an approval or question waits). The pointer lane keeps that upstream behavior for every row; the `--deliver` flag, the v44 column and the federation field stay so existing scripts and databases keep working, and the help text says a row's `queue` no longer holds a nudge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>















ELI5
When a worker agent reports back to a coordinator running in a native chat, Orca posts a short "You have 1 orchestration message, run
check" notice into the coordinator's chat. If the coordinator was busy, that notice waited as a queued card until the coordinator's whole turn ended. A coordinator that spends its turn in a long command (for example, a loop polling its own inbox) never saw its workers' reports until it stopped, and the cards piled up. Now the notice is simply sent to the agent right away, the same way for every agent, just as a terminal agent gets its notice typed in.What Changed
Before: every notice to a busy native chat became a queued card ("From … Steer / Delete") and was only sent after the turn ended. Several reports during one turn meant several cards.
After: every native-chat agent gets the notice sent right away, exactly as if you had typed it and pressed send. No card is created, for any agent. What the agent then does with a message that arrives mid-turn is the same thing it does with your own mid-turn message:
One hold: while the chat is waiting on you to answer an approval or a question, the notice is not sent. Sending then would land in the turn you are answering, and on ACP agents would withdraw the open prompt you were about to answer. The notice goes out the moment you answer, or when the turn ends. Held mail never shows as a card or a bubble; it simply arrives after you answer.
Mechanism:
delivery: 'queue'(queue as a card if busy). It is now always a plain send (delivery: 'now') for every agent. Delivery is part of what the host checks when a send is retried under the same operation ID, and it no longer depends on the agent or the moment, so a retry can never disagree with the first attempt.Why
The queue was chosen so agent mail would behave like a person's queued message. For a coordinator, though, "wait for the turn to end" can mean waiting until the coordinator gives up. Sending right away matches what terminal coordinators already get, through the one send path native chat already has, and behaves identically for every agent so users see one behaviour. Holding while a prompt waits on the person follows the common pattern: a notice for an agent is held while one of its prompts waits on the person, and goes out after the answer.
Alternatives considered:
Differences from the common pattern:
This is PR 1 of 2. PR 2 replaces the notice with the actual mail content for native chats, one chat message per mail message, so the coordinator no longer needs to run
check.Linked Issue
None (maintainer change).
Visual Proof
Rendered-app QA on this change (
b4cd51c5714), with real signed-in agents (Codex and Grok) in a hidden test copy on a separate Mac:1. Busy Codex coordinator: the worker's notice arrives as a sent message (this PR)


Same flow on


main: the notice waits as a queued card2. Codex waiting on an approval: mail is held, then sent right after Allow



3. Grok (an ACP agent): busy chat takes the notice next; mail held around a question and released after the answer





Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
End-to-end test on the coordinator-mail test rig: a worker report that arrives while the coordinator is mid-turn goes into that turn as a Codex
turn/steernaming the running turn. No card is created, the message keeps who it's from, and nothing is resent at the idle edge.Real host plus ACP test: with a question already open, a mail notice is refused with nothing recorded, the question stays answerable, and exactly one plain send happens after the answer.
Delivery test: the park is registered before the session read, and an answer landing during that read still leads to one send, with no duplicate and no lost unread message.
Host test: the notice is a plain send for every agent. Status wiring test: a chat going back to working retries its parked mail.
Removed
structured-chat-coordinator-mail-queue.test.ts: all four of its tests asserted the queue-until-turn-end behaviour this PR replaces. The 20-card queue limit it also covered is still tested at the host level.The 52 targeted tests pass under Node (
pnpm testruns them under Bun, wherenode:sqliteis unavailable, on main too). CI runs lint, typecheck and the full suite.Notes
mainlet the commit hook reformat three files from main (structured-agent-session-send-runtime.ts,structured-agent-session-send-settlement.ts,AiVaultScanProgressStrip.test.tsx); those changes are whitespace only.