Repository navigation
fix(orchestration): file federated worker mail under the coordinator Run - #19542
Merged
Merged
Conversation
Contributor
|
This run was cancelled 🛑 The workflow was cancelled before completion. Please check the link below for details. |
Contributor
📝 WalkthroughWalkthroughThe change makes orchestration tasks and messages require explicit Run identifiers. Federated attachments now store and propagate their home Run through schemas, migrations, RPC handlers, and control messages. Worker projection returns no command for unverifiable workers without pending work. The change adds regression tests for migrations, writer validation, federated attachment delivery, restart replay, and loopback mail delivery. It also updates orchestration guidance and reliability gates. Priority: ➖ Normal — Impact reflects medium issue severity. Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to Task creation now requires a Run, but callers can still omit it at compile time and encounter a runtime failure. Make the input contract required before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the problem, fix, verification, and compatibility impact, but it does not follow the required template. It omits the required ELI5, What Changed, Visual Proof, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist sections, and it does not use the required linked-issue format. Resolution Update the description to include every template section. Add an ELI5 summary, structured What Changed and Why sections, a Linked Issue entry using the required format, Visual Proof with N/A and a reason if applicable, Testing details and checkboxes, AI Disclosure, Review, Agent skill upstream boundary, Notes, and the complete Checklist. Full details: Docstring CoverageExplanation Docstring coverage is 34.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 50 files. (49 skipped: 2 unsupported, 47 over the file limit.)
✨ 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}
Contributor
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: 1973c789-803c-4254-aa81-da2708974944
📒 Files selected for processing (100)
config/reliability-gates.jsoncconfig/scripts/orchestration-skill-guidance.test.mjsskill-guides/orchestration.mdsrc/cli/bundled-skill-guides.tssrc/main/runtime/orca-runtime-subscribe-to-terminal-resize.tssrc/main/runtime/orca-runtime-tests/lineage-and-scan-cache-part-05.spec.tssrc/main/runtime/orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec.tssrc/main/runtime/orca-runtime-tests/terminal-output-and-worker-recovery-part-04.spec.tssrc/main/runtime/orchestration-messages-fake-parity.test.tssrc/main/runtime/orchestration/coordinator-decision-gates.test.tssrc/main/runtime/orchestration/coordinator-dispatch-unobserved-prompt.test.tssrc/main/runtime/orchestration/coordinator-drift-probe-coalescing.test.tssrc/main/runtime/orchestration/coordinator-escalation-triage.test.tssrc/main/runtime/orchestration/coordinator-stale-base-flag.test.tssrc/main/runtime/orchestration/coordinator.test.tssrc/main/runtime/orchestration/db-empty-dispatch-shortcircuit.benchmark.test.tssrc/main/runtime/orchestration/db-heartbeat-straggler-guard.test.tssrc/main/runtime/orchestration/db-message-timestamp.test.tssrc/main/runtime/orchestration/db-messages.test.tssrc/main/runtime/orchestration/db-task-create-readiness.test.tssrc/main/runtime/orchestration/db-task-dispatch-invariant.test.tssrc/main/runtime/orchestration/db-task-dispatch-lifecycle-guards.test.tssrc/main/runtime/orchestration/db-task-dispatch-races.test.tssrc/main/runtime/orchestration/db-undelivered-mailboxes.test.tssrc/main/runtime/orchestration/db.test.tssrc/main/runtime/orchestration/db/attempt-outcome-projection.test.tssrc/main/runtime/orchestration/db/contract-constants.tssrc/main/runtime/orchestration/db/decision-gate-lifecycle.test.tssrc/main/runtime/orchestration/db/decision-gates/decision-gate-store.tssrc/main/runtime/orchestration/db/dispatch-depth.test.tssrc/main/runtime/orchestration/db/dispatch-mailbox-consumer-fencing.test.tssrc/main/runtime/orchestration/db/dispatch-row-writer.tssrc/main/runtime/orchestration/db/federation/federated-dispatch-observation-fence.test.tssrc/main/runtime/orchestration/db/federation/remote-dispatch-attachment-create.tssrc/main/runtime/orchestration/db/federation/remote-dispatch-attachment-release.test.tssrc/main/runtime/orchestration/db/lifecycle-transition.test.tssrc/main/runtime/orchestration/db/messages/message-insert.tssrc/main/runtime/orchestration/db/schema/create-graph-tables-sql.tssrc/main/runtime/orchestration/db/schema/federated-home-run-migration.test.tssrc/main/runtime/orchestration/db/schema/migrate-v40.tssrc/main/runtime/orchestration/db/schema/migrate.tssrc/main/runtime/orchestration/db/tasks/task-store.tssrc/main/runtime/orchestration/db/writer-run-required.test.tssrc/main/runtime/orchestration/dispatch-failure-idempotency.test.tssrc/main/runtime/orchestration/failed-start-terminal-adoption.test.tssrc/main/runtime/orchestration/federation-acknowledgment-integrity.test.tssrc/main/runtime/orchestration/federation-control-message.tssrc/main/runtime/orchestration/lifecycle-caller-edges.test.tssrc/main/runtime/orchestration/lifecycle-reconciliation.test.tssrc/main/runtime/orchestration/lightweight-run-worker-exit-escalation.test.tssrc/main/runtime/orchestration/mailbox-pointer-eligibility.test.tssrc/main/runtime/orchestration/mailbox-pointer-stage.test.tssrc/main/runtime/orchestration/mailbox-pointer-submit.test.tssrc/main/runtime/orchestration/message-batch-atomicity.test.tssrc/main/runtime/orchestration/nested-worker-depth-migration.test.tssrc/main/runtime/orchestration/orchestration-adopted-run-binding.test.tssrc/main/runtime/orchestration/orchestration-all-start-versions-migration.test.tssrc/main/runtime/orchestration/orchestration-db-retention-pagination.test.tssrc/main/runtime/orchestration/orchestration-federated-legacy-probe.test.tssrc/main/runtime/orchestration/orchestration-legacy-storage-test-fixture.tssrc/main/runtime/orchestration/orchestration-mutation-question-db.test.tssrc/main/runtime/orchestration/orchestration-schema-version-skew.tssrc/main/runtime/orchestration/orchestration-settled-worker-resume-fence-db.test.tssrc/main/runtime/orchestration/orchestration-version-skew-migration.test.tssrc/main/runtime/orchestration/orchestration-worker-dispatch-db.test.tssrc/main/runtime/orchestration/r1-identity-migration.test.tssrc/main/runtime/orchestration/types.tssrc/main/runtime/orchestration/worker-start-unobserved-prompt-settlement.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federated-message-targeting.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federated-release-safety.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federated-worker-start.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-agent-launch.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-control-mail.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-folder-placement.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-lifecycle-settlement.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-setup.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-start-prompt-budget.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.tssrc/main/runtime/rpc/methods/orchestration/federation/federation.tssrc/main/runtime/rpc/methods/orchestration/messaging/check-worker-federated-attachment.test.tssrc/main/runtime/rpc/methods/orchestration/messaging/check-worker.tssrc/main/runtime/rpc/methods/orchestration/runs/migration-behavior.test.tssrc/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.test.tssrc/main/runtime/rpc/methods/orchestration/worker/legacy-dispatch-projection.test.tssrc/main/runtime/rpc/methods/orchestration/worker/manual-dispatch-observation.test.tssrc/main/runtime/rpc/methods/structured-worker-stop-receipt.test.tssrc/main/runtime/rpc/orchestration-11745-regression-verification.test.tssrc/main/runtime/rpc/orchestration-legacy-compatibility-dispatcher-test-fixture.tssrc/main/runtime/rpc/orchestration-legacy-coordinator-race.test.tssrc/main/runtime/rpc/orchestration-legacy-question-takeover.test.tssrc/main/runtime/rpc/orchestration-legacy-takeover-delivery.test.tssrc/main/runtime/rpc/orchestration-legacy-takeover-dispatcher.test.tssrc/main/runtime/rpc/orchestration-mutation-ledger.test.tssrc/main/runtime/rpc/orchestration-mutation-request-show.test.tssrc/main/runtime/rpc/orchestration-runtime-update-settlement.test.tssrc/main/runtime/runtime-legacy-worker-terminal-resume-fence.test.tssrc/main/runtime/runtime-rpc-request-authorization.test.tssrc/shared/orchestration-fleet-projection.test.tssrc/shared/orchestration-fleet-worker-projection.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
nwparker
added a commit
that referenced
this pull request
Sep 8, 2026
…e main static analysis The 7 auto.components.settings.TerminalPane.minimumContrast.* entries added by #18126 have zero call sites; the shipped component reads settings.contrast.*. Because the runtime-required catalog classifies any key with no literal-default call site as required, the orphans broke 'Verify runtime-required localization catalog' on main and red-lit every PR in the repo. Deleting them is the root-cause fix: regenerating would instead add dead strings to the boot bundle. On main+delete, --fix regenerates a byte-identical catalog and the CI step exits 0. Merged with 'test / tests node 24 3/8' red: that failure is an unrelated main break from the #19542/#19551 collision, not from this change.
This was referenced Sep 9, 2026
brennanb2025
added a commit
that referenced
this pull request
Sep 9, 2026
…rdinators (#19689) * fix(orchestration): accept v1.4.198 coordinators on federationAttachStart #19542 made runId required on the attach RPC and backfilled existing attachments with '', on the premise that federation was unreleased. It shipped in v1.4.198, so a v1.4.198 coordinator got 'Missing Run ID' from an upgraded worker host, and every pre-upgrade attachment lost its mailbox because home_run_id='' matches no Run. - runId is optional on the wire; an absent id mints a per-attachment stub Run (run_federated_<dispatch>) through the same INSERT OR IGNORE path. - migrate-v40 backfills existing attachments with the stub and inserts the stub Runs, so in-flight workers keep reporting back. - create SQL gives home_run_id a DEFAULT '' so a rolled-back v1.4.198 host can still insert into a v1.4.199-created table. Stub Runs never carry run_legacy_local, and #19542's attachment-mailbox exclusion in the skew probe is untouched, so no adoption replay path is reintroduced (probe test added). * fix(orchestration): repair empty federated home Run ids on every open A host on v1.4.199 that rolls back to v1.4.198, attaches workers (rows land with home_run_id='' via the DEFAULT), then upgrades again never re-runs the v40 backfill because user_version is already 40, so those attachments stay without a Run and their control mail is refused. Lift the two idempotent set-based statements out of migrate-v40 into backfillFederatedStubHomeRuns and run it from both the v40 migration and the OrchestrationDb constructor after migrate(), matching the existing on-open rememberCurrentRunCoordinatorHandles repair. Tests: reopen a v40 file DB holding a v1.4.198-shaped '' row through the constructor and assert the stub Run and mailbox; pin the wire schema accepting a v1.4.198 request with no runId ('' -> undefined, whitespace passes the schema and is refused at the DB layer). --------- Co-authored-by: Merge Sim <sim@local>
brennanb2025
added a commit
that referenced
this pull request
Sep 9, 2026
…rdinators (#19689) * fix(orchestration): accept v1.4.198 coordinators on federationAttachStart #19542 made runId required on the attach RPC and backfilled existing attachments with '', on the premise that federation was unreleased. It shipped in v1.4.198, so a v1.4.198 coordinator got 'Missing Run ID' from an upgraded worker host, and every pre-upgrade attachment lost its mailbox because home_run_id='' matches no Run. - runId is optional on the wire; an absent id mints a per-attachment stub Run (run_federated_<dispatch>) through the same INSERT OR IGNORE path. - migrate-v40 backfills existing attachments with the stub and inserts the stub Runs, so in-flight workers keep reporting back. - create SQL gives home_run_id a DEFAULT '' so a rolled-back v1.4.198 host can still insert into a v1.4.199-created table. Stub Runs never carry run_legacy_local, and #19542's attachment-mailbox exclusion in the skew probe is untouched, so no adoption replay path is reintroduced (probe test added). * fix(orchestration): repair empty federated home Run ids on every open A host on v1.4.199 that rolls back to v1.4.198, attaches workers (rows land with home_run_id='' via the DEFAULT), then upgrades again never re-runs the v40 backfill because user_version is already 40, so those attachments stay without a Run and their control mail is refused. Lift the two idempotent set-based statements out of migrate-v40 into backfillFederatedStubHomeRuns and run it from both the v40 migration and the OrchestrationDb constructor after migrate(), matching the existing on-open rememberCurrentRunCoordinatorHandles repair. Tests: reopen a v40 file DB holding a v1.4.198-shaped '' row through the constructor and assert the stub Run and mailbox; pin the wire schema accepting a v1.4.198 request with no runId ('' -> undefined, whitespace passes the schema and is refused at the DB layer). --------- Co-authored-by: Merge Sim <sim@local> (cherry picked from commit addd9f3)
6 tasks done
Jinwoo-H
added a commit
that referenced
this pull request
Sep 9, 2026
…nd Run #19542 deleted the fallback that filed such mail under the legacy Run, because a live row there makes the schema-skew probe read the database as pre-Runs and replay adoption on the next open. That refusal also broke the first command in the guide: `orca orchestration send --to <handle>` between two plain terminals, which worked in v1.4.198. Restore delivery by filing under `run_unbound`, a Run the probe never matches, created on first use so `run list` shows it only to a user who has such mail. Claude-Session: 1fec75fd-224b-46ab-95fe-d88e0f3d9ff9
Jinwoo-H
added a commit
that referenced
this pull request
Sep 9, 2026
…nd Run (#19696) * fix(orchestration): file mail from terminals in no Run under an unbound Run #19542 deleted the fallback that filed such mail under the legacy Run, because a live row there makes the schema-skew probe read the database as pre-Runs and replay adoption on the next open. That refusal also broke the first command in the guide: `orca orchestration send --to <handle>` between two plain terminals, which worked in v1.4.198. Restore delivery by filing under `run_unbound`, a Run the probe never matches, created on first use so `run list` shows it only to a user who has such mail. Claude-Session: 1fec75fd-224b-46ab-95fe-d88e0f3d9ff9 * fix(orchestration): create the unbound Run only for a null Run id Claude-Session: 1fec75fd-224b-46ab-95fe-d88e0f3d9ff9
brennanb2025
pushed a commit
that referenced
this pull request
Sep 9, 2026
…nd Run (#19696) * fix(orchestration): file mail from terminals in no Run under an unbound Run #19542 deleted the fallback that filed such mail under the legacy Run, because a live row there makes the schema-skew probe read the database as pre-Runs and replay adoption on the next open. That refusal also broke the first command in the guide: `orca orchestration send --to <handle>` between two plain terminals, which worked in v1.4.198. Restore delivery by filing under `run_unbound`, a Run the probe never matches, created on first use so `run list` shows it only to a user who has such mail. Claude-Session: 1fec75fd-224b-46ab-95fe-d88e0f3d9ff9 * fix(orchestration): create the unbound Run only for a null Run id Claude-Session: 1fec75fd-224b-46ab-95fe-d88e0f3d9ff9 (cherry picked from commit 0fe132e)
This was referenced Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #19339.
Problem
A federated worker host has no
dispatch_contextsrow for the remote Dispatch, so every mailbox write for it fell through to the?? LEGACY_RUN_IDfallback and landed inrun_legacy_local. The mail worked while cached, but after the host restarted the legacy-adoption probe inorchestration-schema-version-skew.tssaw a live row in the legacy Run, decided the database predated Runs, and replayed the destructive adoption. The coordinator'scheckthen found its instruction gone.Fix
orchestration.federationAttachStartnow carries the coordinator's realrunId(required). The worker host stores it asremote_dispatch_attachments.home_run_idand inserts a stubrunsrow withINSERT OR IGNOREso foreign keys hold. No synthetic Run, no resolver.?? LEGACY_RUN_IDwrite fallbacks are deleted (message-insert.ts,task-store.ts,decision-gate-store.ts,check-worker.ts). A missing Run now throws instead of misfiling.federation-control-message.tswrites the attachment's home Run and rejects a replay under a different Run.worker-showloop fix from fix(orchestration): give federated worker hosts a non-legacy mailbox Run #19339:unverifiablewith nothing else owed projectsnextAction: none; guide text updated and bundle regenerated.Verification
tc:node,tc:cli, changed-code quality (0 new findings), bundled-guide and bundle-manifest verify, reliability gates (110), max-lines and ts-nocheck ratchets,git diff --check.orchestration.checkRPC across a DB close/reopen on a federated attachment, loopback attach filing once under the local Run, missing-home-Run refusal, v40 migration placeholder rejection, probe message/delivery cases.Compatibility
None required: federation has not shipped. Genuine pre-Runs databases still receive the existing one-time adoption. No stream opcode or daemon change.