No actionable comments were generated in the recent review. 🎉

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 help to get the list of available commands.}

Review round 1 — 3 independent gpt-5.6-luna (codex) reviewers

Blast-radius review: PASS, no findings. Verified reentrancy during onPtyExit, pointer coalescing vs. in-flight checks, multi-leaf/simultaneous exits, quit and mass-retirement lifecycle, and remote wire compatibility (no new field, opcode, or frame — existing escalation shape on existing mailbox columns).

Findings triaged

Routing review — MAJOR, adopted legacy Runs: not accepted. The claim is that adopt-legacy-run.ts creates the adopted Run with legacy = 0, so a retained legacy coordinator still on the legacy mail protocol misses a run:<id>-addressed escalation. Checked it and the premise doesn't carry:

• getRunMailboxOwnerIdsForHandle (run-lookup.ts:34) deliberately maps that retained handle to the adopted Run, and orchestration-recipient-routing.ts:58 uses it to resolve handle-addressed mail to run:<adoptedRunId> — the routing layer's own canonical destination is exactly the address this PR uses.
• It is not a regression either way: adoption sets every running row in coordinator_runs to failed, so pre-fix getActiveCoordinatorRun() returned nothing and that scenario got no message at all. The reviewer also concedes a current coordinator bound to the adopted Run works.

Test review — accepted, and acted on. The suite could have gone green on real regressions. Second commit fixes that; a mutation sweep over the fix now kills all seven mutations checked:

The bare-handle mutation originally survived every test — trg_messages_route_coordinator_mail reroutes handle-addressed mail carrying a run_id, so the two addresses converge until the Run has no bound coordinator. That divergence is now pinned.

Also picked up from the routing review: the escalation payload now carries dispatchId. applyEscalationToDispatch rejects an escalation lacking an exact Dispatch binding, and a coordinator reading the message needs to know which Dispatch died.

Verification after hardening

9 tests in the new file; orchestration suite 51 files / 425 tests pass; src/main/runtime 4577 pass with only the pre-existing quarter-circle-title-send-authorization failures, which fail identically on clean main. check:code-quality:changed, typecheck, and oxfmt all clean.

Review round 2 — both reviewers who requested changes now approve

Routing reviewer — MAJOR withdrawn. After verifying the two facts, they confirmed run:<adoptedRunId> is the routing layer's own canonical destination for a retained legacy coordinator handle, and that adoption marks running coordinator_runs rows failed so the pre-fix path delivered nothing there. Their words: "This PR does not regress a previously delivered escalation in that scenario; it adds a current-delivery handoff for a legacy-compatibility consumer."

They also traced the exact read path they'd originally thought was blocked: with no --run, handleLegacyCheck surfaces a legacyCompatibility.currentDelivery hint containing orchestration check --run <adoptedRunId>; with that flag, resolveRunScope accepts the adopted Run and getOrCreateRunDelivery reads run:<adoptedRunId>. One honest residual they noted, which I'm recording rather than fixing here: a retained legacy coordinator already blocked in the no-arg legacy check waits on its terminal handle and won't be woken by this notification — it has to poll or follow the hint. That is a pre-existing property of the legacy compatibility read path, strictly better than the pre-fix silence, and out of scope for this PR.

Test reviewer — APPROVE. Both prior MAJORs resolved; they independently confirmed the seven-mutation sweep is consistent with the new assertions.

Remaining items are MINOR and explicitly non-blocking, recorded for honesty rather than fixed here:

• settle() uses a real setTimeout(0), and each notifyMessageArrived schedules the runtime's unref'd 2s mail-pointer repoint timer, which can fire after the test closes its DB. The runtime swallows it; it's test hygiene, not correctness.
• The isolation test has multiple live Runs but only one exiting worker — it pins owner selection, not concurrent-exit ordering.
• Coverage stays local-Git-worktree-shaped; no folder-workspace or confirmed-SSH exit case. The resolver is independent of process.platform, worktree kind, and path resolution, and existing runtime tests cover the unconfirmed-SSH exit guard.

Blast-radius reviewer: PASS with no findings from the first round.

Review round 3 — three fresh gpt-5.6-luna (codex) reviewers, given the full round 1–2 record

Deliberately fresh agents rather than the round-1 reviewers, so approval wasn't just self-confirmation. Angles: adversarial (attack the prior rounds' conclusions), end-to-end coordinator behavior, and concurrency/lifecycle.

Fixed in ae11056b8f

Uncaught mailbox failure on the PTY-exit path — found independently by the concurrency and adversarial reviewers, and by me while they worked. onPtyExit walks every leaf synchronously and then prunes pty records. Before this PR the escalation was unreachable for lightweight Runs, so insertMessage never ran there and could not throw; now it runs on every worker death, putting a mailbox write on that path. A throw would abandon the remaining leaves — concretely, dispatch A on leaf A and dispatch B on leaf B sharing one pty: if A's insert throws, A is failed durably but B is never processed.

failDispatch is the authoritative transition and has already committed by then, so the mail is now best-effort on top of it: caught and logged rather than unwinding the exit. Covered by a test that asserts onPtyExit doesn't throw, the dispatch is still failed, and the failure is logged — and that test goes red if the guard is removed.

Not fixed here, recorded deliberately

MAJOR — restart/recovery deaths (adversarial reviewer). If the runtime is down or loses its exit subscription when a worker dies, no onPtyExit ever fires. A lightweight dispatch --inject has no worker_dispatches row, and startup recovery inner-joins that table, so it can't discover the dispatch; the stale-heartbeat path only logs. The dispatch can hold its task indefinitely with the coordinator silent.

This is real, but the reviewer states plainly it is not a regression from this PR — main has the same gap. Closing it means emitting the escalation from startup reconciliation and the stale-dispatch detector, which is a materially different change. This PR's contract is the exit event that does reach the runtime. Worth a follow-up ticket rather than scope creep here.

MINOR — adopted-legacy wake-up latency. Already recorded in round 2. A retained legacy coordinator blocked in no-arg check --wait waits on its bare handle and isn't woken by a run:<adoptedRunId> notification; it polls and gets a [CURRENT RUN MAIL WAITING] handoff. Still strictly better than the pre-fix silence. Both round-2 and round-3 reviewers independently agreed the original MAJOR was correctly withdrawn.

MINOR — formatMessagePointer hardcodes orca (behavior reviewer). Verified at formatter.ts:116. Real, but pre-existing and untouched by this PR — it affects every orchestration pointer, and WSL/dev runtimes need orca-ide/orca-dev. Separate fix.

Correction to my own earlier claim

The adversarial reviewer challenged my "7 pre-existing failures, identical on clean main," and they were right to. What I actually verified was narrow and sound: I stashed the change and ran those specific 4 files on clean main, getting the same 7 failures. What I should not have done is present a full-suite failure count as a stable baseline — it isn't. A later full src/main run on this branch produced 6 failing files, the two new ones being persistence-automations and terminal-history-tombstone-retry; both pass in isolation on this branch, i.e. load-sensitive flakes under three concurrent codex agents, matching the reviewer's own read that the extra failures were native-PTY/watcher noise.

The meaningful regression surface is clean: src/main/runtime + orchestration = 4583 passing, with only the known quarter-circle-title-send-authorization and pty-pane-claim-arbitration failures that I confirmed fail on clean main.

Current state

11 tests in the new file; orchestration suite 51 files / 426 tests pass; check:code-quality:changed, typecheck, oxfmt clean.

Added a body to the escalation (5527dff124)

The message was a subject line plus raw JSON, so the banner a coordinator actually reads gave it an exit code and three ids and nothing else — which task died, and whether it was still retryable, required looking the ids up by hand.

Before

──── From: TERM_62DC5050… (term_62dc5050…) [HIGH] (escalation) ────
Subject: Agent exited unexpectedly (code 137)
[Payload: {"taskId":"task_c917b402bd49","dispatchId":"ctx_041a65ba5ee8","exitCode":137,…}]

After

──── From: TERM_62DC5050… (term_62dc5050…) [HIGH] (escalation) ────
Subject: Agent exited unexpectedly (code 137)
Worker term_62dc5050… exited with code 137 while running task "Migrate the billing
schema to v2" (task_c917b402bd49). The task is ready to be dispatched again.
[Payload: {"taskId":"task_c917b402bd49","dispatchId":"ctx_041a65ba5ee8","exitCode":137,…}]

The second sentence is taken from what failDispatch actually settled to, so it can't contradict reality: a plain failure says the task is ready to dispatch again, a tripped circuit breaker says it will not be retried automatically. Task naming reuses buildOrchestrationTaskDisplayMetadata, the same helper behind the title shown everywhere else, rather than adding a second derivation.

Both bodies are pinned by tests, and the grading oracle now compares body — without that the wording would have been unasserted and free to rot.

Verified: 10 tests in the new file, orchestration suite 51 files / 426 tests, src/main/runtime 4578 passing with only the known-pre-existing quarter-circle-title-send-authorization failure. check:code-quality:changed, typecheck, oxfmt clean.

Still open and deliberately not in this PR: the reply affordance ([Reply: orca orchestration reply …]) is offered on these even though the sender is a terminal that just died — that is generic banner behavior, not specific to worker exits.