Repository navigation
Conversation
Agent-name group addresses were a hand-kept literal of nine names that drifted behind the canonical agent list. Twenty-eight launchable agents -- antigravity among them -- resolved to nobody, and because an unknown group and an empty group both resolve to zero handles, a misspelling was reported exactly like a correctly addressed pane with no members. Derive the addressable set from ALL_TUI_AGENTS, which the compiler makes exhaustive over TuiAgent, so a new agent is addressable the day it is declared. Keep the legacy @mimo alias pointing at the mimo-code identity. Add isRecognisedGroupAddress so orchestration.send rejects an unknown group as invalid_argument before it reads the sender's Run binding, separating a typo from an empty group and from a binding problem. Rewrite the guides generically (@ALL, @idle, @WorkTree:<id>, @<agent>) and add a ratchet test that fails if any doc paragraph enumerates the agent groups again.
📝 WalkthroughWalkthroughAgent group addresses now derive from Priority: ⬇️ Low Severity of issue fixed: Low Merge Risk: 🔵 Low · up to The documented address forms omit the supported legacy 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 6 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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: 8190119a-991a-45dc-801a-d3cd76d06703
📒 Files selected for processing (9)
docs/site/content/docs/cli/orchestration.mdxskill-guides/orchestration/references/messaging-and-gates.mdsrc/cli/bundled-skill-guides.tssrc/cli/specs/orchestration.tssrc/main/runtime/orchestration/group-address-doc-enumeration.test.tssrc/main/runtime/orchestration/groups.test.tssrc/main/runtime/orchestration/groups.tssrc/main/runtime/rpc/methods/orchestration/messaging/send-group.test.tssrc/main/runtime/rpc/methods/orchestration/messaging/send-group.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| - Default `check` is the bound Run's oldest unacked Delivery (FIFO). Replay until `--ack`. | ||
| - `--peek` / `--all` do not consume mail. | ||
| - Group addresses: `@all`, `@idle`, `@claude`, `@codex`, `@opencode`, `@gemini`, `@droid`, `@grok`, `@cursor`, `@worktree:<id>` — never for `worker_done` / heartbeat. | ||
| - Group addresses: `@all`, `@idle`, `@worktree:<id>`, and `@<agent>` for any agent id `--agent` accepts (`@codex`, and every other launchable agent) — never for `worker_done` / heartbeat. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the legacy @mimo alias in every group-address reference. The runtime accepts @mimo as an alias for mimo-code, but these exhaustive-looking address descriptions omit it. Users can incorrectly treat a valid address as unknown.
docs/site/content/docs/cli/orchestration.mdx#L118-L118: state that legacy@mimoremains accepted; remove the@codexexample if needed to preserve the one-named-agent paragraph rule.skill-guides/orchestration/references/messaging-and-gates.md#L42-L44: state that legacy@mimoremains accepted; avoid naming it alongside@codexin the same paragraph.src/cli/specs/orchestration.ts#L73-L73: state that legacy@mimoremains accepted alongside the generic@<agent>form.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
pnpm test fails on this branch. config/scripts/orchestration-skill-guidance.test.mjs:380 still requires messaging-and-gates.md to contain @grok and @cursor, which the rewritten paragraph removed. CI shard test / tests node 24 1/8 is already red.
Reviewed changes
- Resolver derives agent groups from
ALL_TUI_AGENTS—GROUP_AGENT_IDSreplaces the hand-kept nine-name literal; everyTuiAgentplus amimo -> mimo-codelegacy alias is now addressable, andresolveGroupAddressreads the map. isRecognisedGroupAddressseparates a typo from an empty group —sendGroupMessagethrowsinvalid_argument"Unknown group address" before it reads the sender's Run binding.- Guides rewritten generically —
orchestration.mdx,messaging-and-gates.md, the CLI spec notes, and the regeneratedbundled-skill-guides.tsnow describe@all,@idle,@worktree:<id>,@<agent>. - Documentation ratchet added —
group-address-doc-enumeration.test.tswalksskill-guides/anddocs/viascanSourceTreeand fails any paragraph naming two or more agent groups. - Coverage tests added — all-agent routing,
@antigravitydelivery, the@mimoalias, and a typo from an unbound sender.
⚠️ A second test still pins the removed @grok / @cursor names
config/scripts/orchestration-skill-guidance.test.mjs:380 asserts the messaging reference contains @grok and @cursor, but this PR rewrote that paragraph to name only @codex. The unit suite therefore fails. Because the new ratchet allows at most one agent-group name per paragraph, the fix is to relax the guidance assertion to the generic forms, not to re-add the names.
Technical details
# Stale enumeration assertion in the guidance test
## Affected sites
- `config/scripts/orchestration-skill-guidance.test.mjs:380` — `for (const group of ['@all', '@grok', '@cursor', '@worktree:<id>'])` then `expect(reference).toContain(group)`.
- `skill-guides/orchestration/references/messaging-and-gates.md:42-44` — now names only `@codex`.
- `src/main/runtime/orchestration/group-address-doc-enumeration.test.ts:17` — `MAX_AGENT_GROUPS_PER_PARAGRAPH = 1` forbids putting `@grok` and `@cursor` back in one paragraph.
## Reproduction
`pnpm test config/scripts/orchestration-skill-guidance.test.mjs` → 1 failed, 28 passed, with `AssertionError: expected '# Messaging and gates...' to contain '@grok'`. CI job `test / tests node 24 1/8` in run 34802599249 is FAILURE.
## Required outcome
- `pnpm test` green on this branch.
- The guidance test stops pinning agent ids it does not own.
## Suggested approach
- Change the loop to the generic set (for example `['@all', '@idle', '@worktree:<id>', '@<agent>']`), matching the new paragraph.
- The new ratchet scans only `.md`/`.mdx` under `skill-guides/` and `docs/`; this `.mjs` assertion sits outside it, so agent names can still drift in test/prose files it never reads. If the "cannot drift again" claim is meant to cover those too, widen the scan or call out the boundary.ℹ️ Nitpicks
agentGroupsNamedingroup-address-doc-enumeration.test.tsmatches only backticked, lowercase`@name`; an unbackticked or capitalized enumeration slips past the ratchet. A one-line comment scoping that limitation would keep the test's guarantee honest.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Scope note: the user report behind this PR had three parts; this PR is part oneFrom the original Help-menu report (Orca v1.4.198, reporter
On claim 3. The cause is that What would unblock it: a captured PTY transcript per Also note #20485 (supervised Qwen Code workers) modifies |
|
I took over the canonical group-address fix in draft #21655, based on #20549 with contributor credit. ELI5: Orca already recognized Antigravity panes, but the message router's separate name list did not include them. The router now uses the agent catalog, and a typo gets an unknown-group error instead of an unrelated Run-binding error. Verification: 72 group/RPC tests pass; the Antigravity RPC regression fails against the parent with Scope: group routing and diagnostics only. This does not establish successful agent execution, mailbox consumption, |
…#21655) * fix(orchestration): address Antigravity groups from the agent catalog Adopt group routing and unknown-address diagnostics from PR #20549; retain current generated guide content and omit the documentation enumeration ratchet. Co-authored-by: OrcaWin <orcawin@users.noreply.github.com> * test(orchestration): expect canonical agent group documentation * test(orchestration): cover delayed agent identity routing * test(orchestration): satisfy checked receipt typing * test(orchestration): validate delayed group receipt --------- Co-authored-by: OrcaWin <orcawin@users.noreply.github.com>
What this fixes
A coordinator sent
--to "@antigravity"to a Run that had a live Antigravity worker and got back:Agent-name group addresses were a hand-kept literal of nine names (
claude,openclaude,codex,opencode,mimo,gemini,droid,grok,cursor). The canonical agent listALL_TUI_AGENTShad grown well past that, and the literal never followed. Twenty-eight launchable agents resolved to nobody — Antigravity among them. The host had correctly publishedagentIdentity: 'antigravity'for the pane; the resolver simply had no entry to match it against, so it returned an empty handle list.The second half of the problem is what made it hard to diagnose.
resolveGroupAddressreturned[]for an unknown group and for a known group with no current members, so the two were indistinguishable at the call site.@antigravity(valid, live worker) and@antigravty(a typo) produced the identical sentence. And from a sender bound to no Run, the Run-binding check ran first, so a misspelled group was reported as a binding failure — pointing the author at--runinstead of at the typo.The full set that was unaddressable, taken verbatim from the failing ratchet test below:
claude-agent-teams,devin,ante,trae,autohand,mimo-code,pi,omp,prime-agent,antigravity,aider,goose,amp,kilo,kiro,crush,aug,cline,codebuff,command-code,continue,kimi,mistral-vibe,qwen-code,rovo,hermes,openclaw,copilot.The fix derives the addressable set from
ALL_TUI_AGENTS— which the compiler makes exhaustive overTuiAgent— so a new agent is addressable the day it is declared, addsisRecognisedGroupAddresssoorchestration.sendcan reject an unknown group asinvalid_argumentbefore it reads the sender's binding, and rewrites the guides generically so they cannot drift again.Fix evidence
All commands run from the worktree root on branch
fix/orchestration-antigravity-first-class.Before — behavioural tests fail against the pre-fix resolver
With
groups.tsandsend-group.tsreverted to their pre-fix state and the new tests left in place:Honest caveat on that run: four of those six are
TypeError: isRecognisedGroupAddress is not a function, because reverting the fix also removes the export the test imports. Those four are not a clean behavioural before/after — they demonstrate the function is new, not that the old code misbehaved. The two that are genuine behavioural failures are the 28-agent coverage assertion quoted in full above androutes @antigravity to a live Antigravity pane. The RPC-level run below is the clean behavioural pair, becausesend-group.test.tsgoes through the RPC and imports nothing new.Same revert, at the RPC layer — these are real assertion failures, not import errors:
That second failure is the diagnosis bug stated exactly: the typo
@antigravtycame back asterminal_not_found/ "No recipients resolved", the same answer a correctly addressed empty group gives.Before — the documentation ratchet fails against the old guides
With the source fix in place but the guides reverted to their pre-fix wording:
This is a clean before/after: only prose changed between the two runs.
After — full fix, run post-squash on the final commit
Typecheck
Changed-code quality gate
Generated bundle stays in step
src/cli/bundled-skill-guides.tsis generated fromskill-guides/, so the prose edit regenerates it:ELI5
Orca can run many different AI coding assistants side by side, each in its own terminal pane. A coordinator can send one message to all the panes running a particular assistant by writing an address like
@codex— a bit like an email group alias.The list of valid aliases was typed out by hand in one file, and it listed nine assistants. But Orca had quietly grown to support many more. Nobody went back and added them to the alias list. So if you addressed a message to
@antigravity, the mail system looked up "antigravity" in its little list of nine, didn't find it, and told you "nobody is there" — even though an Antigravity assistant was sitting right there, running, waiting.Worse, the system gave the exact same answer for two completely different situations: "that alias doesn't exist" and "that alias exists but nobody's currently in the group." So if you fat-fingered
@antigravty, it told you the same thing as a correct address to an empty group, and you'd go hunting for the wrong problem. If you weren't part of a project group at the time, it got even more confusing — it would blame that instead of your typo.Two changes. First, stop typing the alias list by hand. Build it automatically from the master list of assistants Orca supports, so the moment someone adds a new assistant, its alias works — no one has to remember. Second, teach the system to tell the two situations apart: if you use an alias that doesn't exist at all, it now says "Unknown group address" and lists the valid shapes, instead of shrugging and saying nobody's home.
There's also a bookkeeping change. The instruction manuals also listed the nine names by hand, and they'd drifted too. Those now describe the rule ("any agent id") instead of listing names, and there's an automated check that fails the build if anyone writes out a list of names in the docs again — so this particular drift can't come back.
Trade-offs
Real negatives, stated plainly:
@nonsenseused to resolve to zero recipients and then surface asterminal_not_found"No recipients resolved". It now throwsinvalid_argument"Unknown group address". Any caller — script, agent prompt, integration test — that catchesterminal_not_foundand treats it as "nothing to do" will now see an unhandledinvalid_argumentinstead. This is an error-code change on an existing path, and it is user-visible.invalid_argumentcode onorchestration.sendwhere it previously gotterminal_not_found. No new field, opcode, or param is introduced, so nothing is dropped or fails to decode — but a client with code-specific error handling for group sends sees different text and a different code. Reviewers who care about mixed-version behaviour should look here first.isRecognisedGroupAddressreturns false for a bare handle (no@), so the collision surface is limited to@-prefixed strings only.continue,amp,crush,goose,pi,anteandcopilotare ordinary English words or common product names. A future paragraph that legitimately mentions, say,`continue`and`copilot`in unrelated prose will fail this test with a confusing message. The mitigation is only that the assertion message names the remedy. The threshold (MAX_AGENT_GROUPS_PER_PARAGRAPH = 1) is a tuning knob that could need raising.skill-guides/anddocs/for.md/.mdx. It uses the sharedscanSourceTreeso it skipsnode_modules,.next,.sourceandout— an earlier version did not, and failed on dependency markdown belonging to whoever had previewed the docs locally. It is still I/O in a unit test suite, and it will get slower as the docs grow.@mimois now an alias, not the name. Both@mimoand@mimo-coderesolve to themimo-codeidentity. That is two addresses for one group, which is mild clutter and something a future cleanup has to decide about rather than a clean state.src/cli/bundled-skill-guides.ts, which is generated with very long lines — any in-flight branch touchingskill-guides/will conflict there and must be resolved by regenerating (pnpm run verify:bundled-skill-guidesto check), not by hand-merging. It also rewrites the group-address paragraph in bothdocs/site/content/docs/cli/orchestration.mdxandskill-guides/orchestration/references/messaging-and-gates.md.@antigravityto a real Antigravity pane. The test asserts onagentIdentity: 'antigravity'as the host publishes it; if the host ever publishes a different identity string for that agent, these tests would still pass while the real path stayed broken.@antigravity— there's a test pinning that), and this changes nothing about how agent identity is detected. If the host never resolves a pane's identity, that pane remains unaddressable by agent group, exactly as before.Adversarial review
Three loops. What each raised and how it was resolved:
Loop 1 — the fix was half a fix. The initial change replaced the nine-name literal with a derivation from
ALL_TUI_AGENTS, which made@antigravityroute. The review's objection was that this fixed routing but left the reported symptom's diagnosis untouched: an unknown group and an empty group still both resolved to[]and produced the same message, so a user who typo'd would still be misled, and a user in no Run would still be told it was a binding problem. Resolved by addingisRecognisedGroupAddressand a check insendGroupMessagethat runs beforeresolveAudienceRunId(), so the typo is reported as a typo regardless of binding state.resolveGroupAddressstays total and still returns[]— the distinction is a separate query, not an exception thrown from the resolver.Loop 2 — the docs were still the old nine, and would drift again. The review pointed out that the resolver was now self-maintaining but every shipped description a coordinator actually reads (
orchestration.mdx,messaging-and-gates.md, the CLI spec notes, and the generatedbundled-skill-guides.ts) still enumerated the closed nine. So@antigravitywould work while every piece of documentation said it didn't — and the same drift would recur on the next agent. Resolved by rewriting all four generically (@all,@idle,@worktree:<id>,@<agent>for any agent id--agentaccepts) and addinggroup-address-doc-enumeration.test.tsas a ratchet that fails if any paragraph enumerates the set again. The generated bundle is covered transitively viaverify:bundled-skill-guides.Loop 3 — the ratchet itself was unsound. Two defects found in the new test. (a) Its recursive walk descended into
docs/site/node_modules:docs/siteis a self-contained Next.js app whose own README tells contributors topnpm --ignore-workspace installthere, so anyone who had previewed the docs had dependency markdown on disk, and the test failed on files the contributor neither owns nor can edit. Resolved by switching to the sharedscanSourceTree, which skipsnode_modules,.next,.source,outand dot-directories — and pinned with a second test asserting no such path is ever scanned. (b) Its regex matched bare@name, so npm scopes like@opencode-ai/sdkcounted as group addresses and produced false positives. Resolved by requiring the backticked form`@name`. The review also asked that the failure message carry the remedy rather than a bare array diff, which is why the assertion passes a message naming the generic form.Final loop result: CLEAN — no findings.
Verification quoted in full under Fix evidence: 75 tests passing across the three files,
pnpm tcexit 0, changed-code quality gate 0 new findings across 7 changed files.