Repository navigation
fix(orchestration): native chats get each orchestration message whole instead of a check pointer - #28567
Conversation
…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.
abbb2ea to
4f1d256
Compare
… instead of a check pointer Each mail message is sent to a native-chat agent as its own chat message, with the sender, type, subject, body, payload and exact reply command, and is marked read once the chat accepts it. Heartbeats are marked read, never sent. Mail an unacknowledged check batch holds stays with that batch.
…, exhaustive sender switch
3ff5b50 to
c8ec903
Compare
|
Worker reports need to reach native chats as their own attributed messages, wait while the chat is busy or a question needs an answer, and follow the existing queue controls. This PR puts them through the ordinary durable message queue. Reports waiting at Stop stay paused until Send or Resume; a provably fresh report after Stop can wake the chat while the person’s paused items stay paused. Unknown saved Stop order stays held. Not ready to merge. The database-opening regression and the bounded fixture/API repairs are pushed, and the blocking main integration is complete at Targeted real Claude and Codex QA has started setup in the admitted isolated M4Air worktree. The required matching-dependency source/CLI/full-turn contracts, busy/question/Stop/Resume/restart results and shipping phone screenshot remain owed. Current GitHub checks contain housekeeping and automated reviews only; required PR Checks have not registered on this head, and no CI or runtime pass is claimed. Earlier screenshots below are historical evidence only for their recorded heads and flows. Earlier review progress and retained screenshotsWorker reports use the native chat’s ordinary durable queue, with their own content and sender attribution. Busy chats and pending questions hold reports until the current turn completes; each eligible report then gets its own turn. Stop keeps waiting reports paused, while provably fresh reports can wake the chat without releasing the person’s paused messages. Unknown saved Stop order stays held until Send or Resume. Not ready to merge. The latest completed source review is on Recorded native-queue and selected desktop/phone tests provide bounded fixture evidence. They do not prove default runtime installation or the full real-agent flows. Matching-dependency runtime/ACP execution, final CI, targeted real Claude/Codex runs, and current screenshots remain outstanding. No current-head QA pass or review-completion marker is claimed. The targeted runs cover busy delivery, answered questions with ordered separate report turns, Stop and restart, fresh reports without personal-queue release, and existing Resume including a report-only backlog. This is a continuation of the required changed flows, not a second full QA round. The screenshots and outcomes below are historical evidence from earlier implementations, including the recorded failure. They do not qualify the rebuilt delivery/Stop flows. Earlier review and historical screenshotsNative chats currently receive a notice asking the agent to check its mailbox. This PR puts each worker report into the conversation as a separate attributed message, with a 4,000-character excerpt and an exact-message command for the full text. Real Grok QA found that sending reports into a running turn can cancel the person's answer and lose a report. The approved revision holds mail while any agent is working or waiting for the person, then starts one report turn at a time, oldest first. Terminal delivery stays unchanged. Current review on The review found these remaining repairs and integration checks:
This round changed only the coordinator integration test (+84/−41); no production source changed. An independent Codex reviewer agreed with the whole-flow contract and its size. The normal commit hook passed, the non-force push is verified, and the worktree is clean. No main synchronization or QA launch occurred. Shared eligibility, consuming-turn recovery and outstanding-delivery repair remain with #32108, which is still open. Production work at that boundary waits for its handoff. The plan now records the repair contracts and an estimate of 100–150 additional production lines, with 120–180 changed test lines; the estimate will be checked against the landed interface before the shared repairs. The independent slice has updated the obsolete busy-turn test without changing recovery code. Verification limits: the named real-host coordinator test used the scripted protocol fixture; its tested/reviewed patch exactly matches this commit. The file still has the known recovery failure. This is not real-agent QA. The older green full workflow and real-agent screenshots below are historical. Only the housekeeping workflow is registered on this head; full PR Checks, final mergeability/conflict verification, the final main synchronization, and real Codex/Grok/Claude QA on the resulting head remain required. No completion marker is claimed. For Brennan's review: final-head main-versus-branch screenshots remain outstanding. The historical comparison and the Grok failure are preserved below. The three places where behavior depends on the admission and release contract are mail admission, idle release, and the person's Stop pause. Historical QA only; these images do not prove the current timing revision: QA on head The failure: Grok can't take a message while a turn runs, so Orca cancels the turn to send one. Held reports are released the moment the person answers, and each report is now its own send, so the second report cancels the first before Grok sees it. Grok's saved history has no trace of report A while Orca marks it read and delivered. On main the single notice gets both reports read. Separately, the first report also cancels the turn that carries the answer, so Grok re-asks its question; that also happens on main (main evidence). The fix is being decided; QA reruns after it. Final main sync, its CI and the completion marker also remain. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
Merge Risk:
|
…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.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments: 2
src/cli/specs/orchestration.ts: 1
133-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore
--jsonin the inbox usage text.
GLOBAL_FLAGSis probably the source of--json, so the flag stays valid. The handler still readsjsonand prints JSON output. The usage string no longer lists it, and the other specs do. Users cannot find the flag in help output.Proposed fix
- usage: 'orca orchestration inbox [--id <id>] [--limit <n>] [--terminal <handle>] [--full]', + usage: + 'orca orchestration inbox [--id <id>] [--limit <n>] [--terminal <handle>] [--full] [--json]',src/cli/handlers/orchestration/message-inbox-handlers.ts: 1
44-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFilter messages without mutating the RPC response.
The code mutates
result.resultin place. The behavior is correct today. A copy is safer if the client later caches or shares response objects.Proposed fix
- if (id) { - result.result.messages = result.result.messages.filter((message) => message.id === id) - result.result.count = result.result.messages.length - } - printResult(result, json, (value) => { + const response = id + ? { + ...result, + result: { + ...result.result, + messages: result.result.messages.filter((message) => message.id === id), + count: result.result.messages.filter((message) => message.id === id).length + } + } + : result + printResult(response, json, (value) => {
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
1337d471-c548-46ac-b759-814998ea2690
📒 Files selected for processing: 27
config/reliability-gates.jsoncconfig/scripts/vitest-sqlite-database-runtime-files.mjssrc/cli/handlers/orchestration-legacy-read-only.test.tssrc/cli/handlers/orchestration/message-inbox-handlers.tssrc/cli/specs/orchestration.tssrc/main/acp/acp-structured-launch-resolution.test.tssrc/main/runtime/orca-runtime-tests/repository-project-operations-part-02.spec.tssrc/main/runtime/orchestration/agent-facing-parity.test.tssrc/main/runtime/orchestration/db/messages/mailbox-push-query.tssrc/main/runtime/orchestration/formatter.test.tssrc/main/runtime/orchestration/formatter.tssrc/main/runtime/orchestration/mailbox-pointer-eligibility.tssrc/main/runtime/orchestration/send-agent-turn.tssrc/main/runtime/orchestration/structured-mailbox-message-delivery.tssrc/main/runtime/orchestration/structured-mailbox-pointer-backlog.test.tssrc/main/runtime/orchestration/structured-mailbox-pointer-batch.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/types.tssrc/main/runtime/rpc/methods/orchestration/messaging/check.test.tssrc/main/runtime/rpc/methods/orchestration/messaging/message-methods.tssrc/main/runtime/structured-chat-coordinator-mail-rig.test-fixture.tssrc/main/runtime/structured-chat-coordinator-mail.test.tssrc/main/runtime/structured-chat-dispatch-worker-mail.test.tssrc/shared/rpc-contract/orchestration-params.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.
…rrupted consumers
A mail send admitted into a running turn cancels that turn for agents that cannot take a message mid-turn (ACP: Grok, OpenCode, OMP, Antigravity): the second of two reports cancelled the first before the agent saw it, and the first report cut off the turn carrying the person's answer. Mail now waits while the chat is working or a prompt waits on the person, for every agent, and goes out at the idle edge, one message per turn, oldest first.
The earlier implementation is replaced: this merge takes main's tree as is, and the following commits rebuild native-chat mail delivery on top of it.
…at's own queue Each orchestration message to a chat session becomes one ordinary chat message carrying the whole message, sent with the chat's queue like a person's message: it waits behind running work or a question and the agent is handed one per turn. A per-message record keeps the send's operation id, so a restart finds the card instead of sending twice; the message is marked read once the agent is handed it, and a check that takes it first deletes the waiting card. A message too large for one chat message keeps today's pointer to check. Terminals are unchanged.
…or /clear A send the agent rejected or left unanswered stays unread for check and is handed over again only once a later turn ran or a /clear replaced its context, so edges alone never restart an agent that fails on every start. A send whose record a rewind dropped is sent again, and a send that throws is retried on the session's next journal edge. Tests follow chats getting each message whole.
A queued mail card now runs only while its message is unread in its mailbox and that mailbox still belongs to the chat: the queue asks at handover and drops a stale card without a turn (abandoned or settled Dispatch, Run taken over by another chat, mail a check or reset took). Cleanup after check, moves, run-use, abandon, reset and worker release deletes those cards visibly and never holds the action up; mail still owed goes to the mailbox's current owner, and Orca's own deletion is never read as the person's. A send that may have reached the agent is never sent again; check keeps it. A send the agent proved it never got is sent once more, under a new id, only after a later turn or /clear ordered after that failure. A send recorded but never admitted is admitted under its own id. Each outstanding send is read by key, never by scanning the chat's history. A refusal no longer loses its retry when an edge arrives mid-send, and a coordinator refused for queue room gets its mail when the queue frees room. A chat worker's follow-up now takes its short Task first and then each moved message whole, with no check needed. The guide says a working worker gets mail when its turn ends.
… person's cards pass mail Orca cannot decide on The chat's host now answers for each send by its id: the card first, then the send itself, then a gone card's handoff found by key, and only then the admission receipt, which alone can prove nothing was admitted. A receipt whose journal detail is gone is in doubt however old its id, so a renewed id can no longer send the same message twice. A rejected send is repeated once only after a different turn was actually handed over and accepted, or a /clear was made, after the rejection; the queue's admission cursor no longer stands in for that. No answer walks the conversation's history. The mailbox reserves each send and stamps the message delivered in one write before the host call, so a crash after admission never leaves it selectable again. A refusal before admission releases that exact reservation; every retirement and release is checked against the current attempt, and a record an older build saved without a stamp is repaired as it retires. A person's Delete consumes an oversized pointer's mail too. Who holds a mailbox is read from durable facts: a Run moved to a terminal, a released worker, another host or an ended chat makes a waiting card stale, so it is withdrawn without a turn and its mail released for the real owner. Only a fact that cannot be read holds a card; the person's own cards behind it go on, later mail stays behind it, and selection and the handover check use the same rule. When the owner can be read again the queue is woken. A Task or a person's edited card never opens the mail database. An explicit Send of such a card now says what happened. Parked mail is indexed by chat, wakes only that chat's mail, ignores streamed tokens, and drops the chat's journal watch with its last mailbox. Canonical session identity is reused per session store revision instead of rebuilt from every record on each check. Main's account-switch worker test now reads "unread and still owed".
…given back its mail The queue now owns one rule for what can be handed over next, installed once on each chat's queue and read by the drain, the final handover check, queue publication and scheduling alike. Mail whose owner cannot be read keeps its place; the first of the person's own cards behind it goes instead. Held cards stay held, a returned, paused or edited card behind it still stops the queue, and later mail, from any mailbox, stays behind it. The rule walks only the queue's live cards, never its history. When nothing can be handed over nothing is published as next, so the chat reads as idle and /clear works; when the owner becomes readable again the queue is published and looked at again. Choosing and publishing the next card only reads custody; settling a stale card stays with the handover. Mail a chat gives back after its Run or worker moved goes through the runtime's own delivery, so a terminal owner is pointed at it. A chat's journal watch now lasts as long as it has mail parked, through its own redrives, so a publication that changes nothing the mail waits on starts no pass. Session identity is cached until a record joins, leaves or is cleared, not rebuilt after every admission. An explicit Send of mail whose owner cannot be read says Orca couldn't read the chat's saved state rather than that it restarted.
…far as it must, and ask mail authority only once nothing else stops a send
… and that the person can try again
…, and stop saving a send fence nothing reads
…ation-mail-content # Conflicts: # src/main/runtime/orchestration/structured-mailbox-held-worker-report.test-fixture.ts
…e types they stand for
…le mail says its message wasn't sent
…answer where the type can see it, and drop the removed test from the send gate
…use the last pick until the queue moves, and look up a send by its id
…, republish a chat once per pass, clean up only the mail a check, abandon or move touched, and act on a chat's status only when it changes
…e instead of the wall clock
…d no send left, unless a pass on it is still running
…nder the id it was reserved with
…ation-mail-content # Conflicts: # src/main/runtime/agent-session/records/agent-session-store-transactions.ts
Review loop status: ready for Brennan's reviewProblem. A coordinator running in a native chat learned about a worker's report only from a "You have 1 orchestration message. Run Change. Each orchestration message now reaches a native chat whole, as its own queued chat message. An idle chat starts a turn at once. A busy chat or one waiting on a question queues it and hands it over after the turn, one per turn, oldest first. A message is marked read once its turn is accepted, so Review. Full path, six review rounds with design-panel rulings, then repair checks.
Bug on main, not fixed here: on main, pressing Send on a refused queued card says "The command didn't run." This PR keeps that wording for every case except mail whose ownership can't be read. Verified.
Not verified: a real rollback to v1.4.246 (the accepted limitation was traced in code and tests), and ACP agents in a real run (covered by tests). For Brennan's reviewWhere judgment matters:
|





















Problem
When a worker reports to a coordinator running in a native chat (Claude, Codex or another agent in Chat UI), the chat gets a card that only says "You have 1 orchestration message. Run
orca orchestration check --run …". The agent has to spend a step runningcheckto learn what the worker said, and the person watching the chat sees only the count. The same goes for direct mail and Dispatch mail to any native chat. The notice also went out as an immediate send, so it steered into a running turn (and cancelled a running ACP turn).Change
Each orchestration message to a native chat now arrives whole, as its own chat message: sender, subject, body, payload and the exact reply command, shown with the existing agent message card. It is sent the way a person's queued message is:
How a message is tracked:
structured_mail_sendstable) with a stable operation id. After a restart Orca finds the card or turn it already made instead of making another.checkstops returning it. While it is still waiting in the queue,checkreturns it, and acheckthat takes it removes its waiting card. Deleting the card yourself also counts as taking it.check. If the agent clearly refused the send, it is offered once more, but only after a later turn was accepted or a /clear replaced the context. A send that never reached the chat is retried under the same id.check, a reset, an abandoned worker or a Run moving to another owner is withdrawn without a turn, and the message stays unread for whoever owns it now. If ownership can't be read at that moment, the mail card keeps its place and the person's own queued messages can still go ahead of it. Pressing Send on such a card shows "Orca couldn't confirm this message is still meant for this chat. Your message was not sent. Try again." Every other refused Send keeps today's wording.checkreturns it whole.checkonly when a wake names it.Differences from the common pattern
Compatibility
fromattribution shape, so desktop cards, phones and older paired clients render these messages with what they already have.checkalready took that message, because the older build doesn't check ownership before sending a queued card. The agent sees the report twice; nothing is lost. To avoid it, delete the waiting card before continuing, or stay on the newer build.Performance
Mail work runs on real changes only: delivery settles earlier sends only when something they wait on moved, refreshes each chat once per pass, keeps the queue's next-card order up to date as cards change instead of re-sorting, looks up sends, tabs and sessions by key, and limits cleanup to the mailboxes a
check, abandon or move touched. Contract tests pin these counts at two sizes (10× apart).Testing
Real-agent QA on a test copy of Orca (head c3ce504, this PR's code before the error-path fix and the main merge that followed), with real Claude Code and Codex coordinator chats signed in through the team's load balancers:
check.request_user_input) holds the mail. After the answer, the report goes out as its own turn.check.Unit and integration tests (real orchestration database, real journal, fake provider) cover: idle and busy delivery; a check taking a queued message; restart through a real database close and reopen with no duplicate; unknown, rejected and never-admitted sends; a send that throws not being overtaken by newer mail; stale-card withdrawal after check, reset, abandon and owner moves; unreadable ownership letting personal cards pass; Task before moved mail across restart; /clear, rewind and direct addresses; ACP questions not interrupted; the oversized fallback; and the scaling contracts above.