fix(orchestration): accept a missing Run id from older federation coordinators

[brennanb2025]

ELI5

Orca can run a worker agent on another machine (orca orchestration worker-start --on <saved-environment>). The machine that starts it is the coordinator; the machine that runs it is the worker host.

#19542 fixed a real bug in how the worker host files mail for those workers, but it assumed this cross-machine feature had never shipped. It had (it is in v1.4.198). So after #19542, three things go wrong when the two machines are on different versions:

1. A coordinator on v1.4.198 can no longer start workers on a host that has updated to v1.4.199. The host answers Missing Run ID, because the old coordinator never sends that field.
2. Workers that were already running when the host updated go silent. The update stamps their bookkeeping rows with an empty run id, and every mail operation for them fails from then on. The agent keeps working, but the coordinator never hears it finish.
3. Rolling the host back to v1.4.198 can fail if its database was first created by v1.4.199, because the new column has no default.

This PR makes the updated host tolerate all three: it accepts the older request shape, gives pre-existing workers a working mailbox again during the update, and gives the column a safe default. Once both machines are on v1.4.199 nothing changes; the new path is byte-identical.

How the fix works, in plain terms

Every piece of orchestration mail has to be filed under a Run (a coordinator's job). #19542 requires the worker host to know which Run a remote worker belongs to, and it made the coordinator responsible for saying so. An older coordinator does not know how to say so, and rows written before the update never had it recorded.

The fix gives the worker host a way to name that Run itself when nobody told it:

1. Old coordinator starts a worker. The host sees the request has no Run id. Instead of refusing, it creates a small placeholder Run named after the dispatch (run_federated_<dispatchId>), the same way fix(orchestration): file federated worker mail under the coordinator Run #19542 already creates a Run row for a real id, and files the worker under it. The coordinator on the other machine never needs to know; it keeps its own real Run locally, as it always did.
2. Host updates with workers already running. During the one-time database migration, each existing worker row gets the same placeholder Run created and assigned, instead of an empty id. So when that worker later sends "done" or the coordinator checks on it, the lookup finds a Run and the mail flows.
3. Host rolls back. The column now has a default, so the older build's insert (which does not mention the column) succeeds. The same placeholder-Run repair from step 2 also runs every time the database is opened, so rows the rolled-back build wrote with an empty id are fixed the next time the updated build starts, even though the one-time migration has already run.

Why a placeholder Run is safe: the bug #19542 fixed was that mail filed under the legacy Run name looked like an old-style database and triggered a destructive cleanup on restart. The placeholder names here can never match that legacy name, and there is a new test that proves the cleanup does not fire on them.

What Changed

• runId is optional on orchestration.federationAttachStart. When an older coordinator omits it, the worker host mints a deterministic stub Run per attachment (run_federated_<dispatchId>) through the same INSERT OR IGNORE INTO runs path fix(orchestration): file federated worker mail under the coordinator Run #19542 already uses for real ids. (federation-start-schema.ts, remote-dispatch-attachment-create.ts, contract-constants.ts)
• migrate-v40 backfills existing attachment rows with that stub id and inserts the stub runs rows in one set-based statement, instead of leaving them at ''. In-flight workers keep a resolvable mailbox across the update.
• The two backfill statements now live in backfillFederatedStubHomeRuns (db/federation/federated-stub-home-run-backfill.ts) and run from both migrate-v40 and the OrchestrationDb constructor right after migrate(), next to the existing on-open rememberCurrentRunCoordinatorHandles repair. Both statements are idempotent (WHERE home_run_id = '', INSERT OR IGNORE), so an open with nothing to fix is a no-op.
• create-graph-tables-sql.ts gives home_run_id a DEFAULT '' so a rolled-back v1.4.198 host can still insert into a v1.4.199-created table.
• No resolver layer. check-worker.ts, federation-control-message.ts, and message-insert.ts are untouched: a stub Run satisfies their existing requireRun.

Why this does not bring back the bug #19542 fixed

#19542 removed the ?? LEGACY_RUN_ID fallthrough because a live attachment row filed under run_legacy_local made the legacy-adoption probe replay a destructive adoption after restart. Stub ids are run_federated_<dispatchId>, never run_legacy_local, so stub-Run mail never matches the probe's source-graph query. The attachment-mailbox exclusion #19542 added to hasConsistentLegacyAdoption is untouched. A new negative test (orchestration-federated-legacy-probe.test.ts, message and delivery variants) seeds a stub-Run attachment with mail and asserts the probe returns SCHEMA_VERSION and that reopening creates no legacy_adoptions row. The "still replays adoption for a genuine legacy database" cases still pass, so the guard is not over-broad.

Wire compatibility

One required field made optional on a host-side RPC params schema: Rule 1 of docs/reference/remote-wire-compatibility.md. A v1.4.199 coordinator still always sends runId. No stream opcode, no published-content change, no capability negotiation. SCHEMA_VERSION stays 40.

Why

Mixed client and host versions are the normal state for paired machines. Federation is released, so the host must keep accepting the shipped request shape and must not strand attachments it already holds.

Linked Issue

No tracking issue exists; this is a regression from #19542 found while scanning the 1.4.199 release candidate. Happy to file one if wanted.

Visual Proof

N/A: RPC schema, SQLite migration, and create-table DDL only.

Testing

• ORCA_BACKGROUND_LAUNCH=1 pnpm exec vitest run --config config/vitest.config.ts over every test file fix(orchestration): file federated worker mail under the coordinator Run #19542 touched plus the whole src/main/runtime/rpc/methods/orchestration/federation/ directory: 76 files / 638 tests passed (vitest list --filesOnly confirmed exactly 76 files matched, including all 16 federation RPC tests). Four files from fix(orchestration): file federated worker mail under the coordinator Run #19542's list no longer exist on main; they were deleted by Remove settled-worker automatic resume and hibernation fences #19544 and fix(orchestration): own a worker terminal from creation, not after the boot wait #19608, not renamed.

• The two modified test files at the final head: 2 files / 8 tests passed.

• Second commit (13a6cda25d): ORCA_BACKGROUND_LAUNCH=1 pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orchestration/db/schema/federated-home-run-migration.test.ts src/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.test.ts: 2 files / 8 tests passed (4 migration cases including the on-open repair and a whitespace-only runId refusal; 4 schema cases). Ablation: commenting out the constructor call makes the on-open repair test fail on expected '' to be 'run_federated_ctx_rolled_back', so the test depends on the new call.

• Same commit, wider run: the migration test, orchestration-federated-legacy-probe.test.ts, orchestration-all-start-versions-migration.test.ts, db/writer-run-required.test.ts, and the whole src/main/runtime/rpc/methods/orchestration/federation/ directory: 21 files / 128 tests passed.

• Same commit: pnpm run check:code-quality:changed 0 new findings across 10 changed files; pnpm exec tsc --noEmit -p config/tsconfig.node.json clean; pnpm exec oxfmt --check on the 5 changed files clean.

• pnpm run check:code-quality:changed: 0 new findings across 7 changed files.

• pnpm exec tsc --noEmit -p config/tsconfig.node.json: clean.

• pnpm exec oxfmt --check on the changed files: clean.

• Not run: tests/e2e/cross-version-wire/* (covers the terminal stream and agentSession.*, not federation RPC); no live two-host, two-version run.

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

Known gaps (same as #19542 alone, not worsened)

(a) Control mail that a v1.4.198 worker host had already imported under run_legacy_local is not re-filed under the stub; the migration deliberately does not rewrite messages/deliveries rows. That has two visible effects after the host updates. First, check-worker reads mail by the attachment's home_run_id, so mail filed under the old legacy name is invisible to it. Second, if the coordinator retries the same message id, the host now answers request_mismatch: federation-control-message.ts compares the existing row's run_id with the attachment's (now stub) Run and treats the difference as a conflicting message. Both effects exist with #19542 alone; this PR does not add to them.

(b) Rolled-back host re-attaching workers, then updating again: closed in this PR by the on-open repair. The empty-id rows now only exist while the rolled-back v1.4.198 build is running; the first start of the updated build repairs them, and the migration's user_version no longer matters.

(c) OptionalString maps '' to undefined, so a new coordinator with a bug that sends runId: '' silently gets a stub Run instead of an error. schemas.ts has no "optional but must be non-empty when present" helper (OptionalPlainString keeps '', OptionalString drops it), so this is left as-is rather than adding a one-off helper; the schema test pins the current behaviour. A whitespace-only runId still fails with Missing Run ID at the DB layer.

Release: candidate cherry-pick for the 1.4.199 release branch release/adhoc-1.4.199-daily-202609081832-cherrypicks. Owner review requested from @Jinwoo-H (author of #19542).

Review

Please challenge whether a stub Run per attachment is the right shape. Gap (b) is now closed here by the on-open repair; please check that repair is the right place for it versus a v41 migration.

Agent skill upstream boundary

• Not applicable.

Notes

The DB layer is host-agnostic (native, WSL, and SSH hosts share it). No platform-specific code.

Checklist

• This PR is small and focused
• I explained what changed and why (including ELI5)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: da473d5e-ca83-42ef-b5b1-20c4f0accc8d

📥 Commits

Reviewing files that changed from the base of the PR and between 750e6ff and 13a6cda.

📒 Files selected for processing (10)

• src/main/runtime/orchestration/db/contract-constants.ts
• src/main/runtime/orchestration/db/federation/federated-stub-home-run-backfill.ts
• src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-create.ts
• src/main/runtime/orchestration/db/orchestration-db.ts
• src/main/runtime/orchestration/db/schema/create-graph-tables-sql.ts
• src/main/runtime/orchestration/db/schema/federated-home-run-migration.test.ts
• src/main/runtime/orchestration/db/schema/migrate-v40.ts
• src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts
• src/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.test.ts
• src/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

Federation start requests can omit a Run ID. Attachment creation derives a deterministic stub home Run ID from the dispatch ID and stores the corresponding Run and attachment records. The schema permits temporary empty home Run IDs from older hosts. Migration and database-open repair backfill those rows. Tests cover request parsing, attachment creation, migration persistence, mailbox routing, and exclusion of stub Runs from legacy adoption.

Priority: ⬆️ High

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 13a6c

Mixed-version federation requests and rollback-created attachment rows are repaired without disrupting mailbox delivery or legacy-adoption safeguards.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 10 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The description is detailed and covers the change, rationale, testing, compatibility, and checklist. However, the required Linked Issue section does not provide an issue reference and explicitly state…	Add a valid linked issue reference in the required format, such as Fixes #19542`` or the appropriate tracking issue number.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes preserve the core requirements from [#19542]: federated mail uses a resolvable home Run, legacy Run fallbacks remain removed, and legacy-adoption safeguards remain active. The added compat…
Out of Scope Changes check	✅ Passed	The production and test changes are focused on mixed-version federation compatibility, stub Run creation, database backfill, rollback support, and legacy-probe protection. No unrelated code changes ar…
Title check	✅ Passed	The title clearly and concisely describes the main change: accepting missing Run IDs from older federation coordinators.

Full details: Description check

Explanation

The description is detailed and covers the change, rationale, testing, compatibility, and checklist. However, the required Linked Issue section does not provide an issue reference and explicitly states that no tracking issue exists.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[pullfrog]

ℹ️ No blocking issues — this is a sound, well-reasoned fix. One rollout-gap observation and a test-strength note below.

Reviewed changes

• Optional runId on orchestration.federationAttachStart — the worker host now accepts a request without a Run id and mints a deterministic per-attachment stub Run (run_federated_<dispatchId>) through the same INSERT OR IGNORE INTO runs path #19542 uses for real ids, instead of refusing with Missing Run ID.
• v40 backfill — migrateV40 now inserts stub runs rows and re-points every home_run_id = '' attachment in one set-based statement, so in-flight v1.4.198 workers keep a resolvable mailbox across the update.
• DDL default — remote_dispatch_attachments.home_run_id gets DEFAULT '' so a rolled-back v1.4.198 host can still insert.
• Tests — migration backfill and stub-mint tests, plus a legacy-probe negative test (message + delivery variants) asserting stub-Run mail does not replay destructive legacy adoption.

The core design checks out. I traced home_run_id through check-worker.ts:56, federation-control-message.ts:68, and message-insert.ts:33 — it is only ever consumed by requireRun (which a stub satisfies) and internal mail filing. The coordinator addresses a worker via dispatch:<dispatchId>, never the Run id, so a stub is a legitimate internal bookkeeping construct and the coordinator's real Run stays its own. The wire change also matches OptionalString semantics: missing or empty runId coerces to undefined and falls back to the stub, while whitespace-only still trips the invalid_argument guard.

ℹ️ Backfill is gated behind the v40 version bump, so already-migrated databases are never repaired

The PR acknowledges the rollback + re-upgrade variant (gap b), but the same structural limitation has a broader reach: any database already sitting at user_version = 40 with home_run_id = '' rows will never run this backfill, because migrateV40 returns early on current >= 40. That includes a daily-channel user who installed an intermediate 1.4.199 RC build containing #19542 but not this fix — their in-flight workers go silent at that upgrade, and stay silent after the final build, since the column-and-default were already applied and the stub mint never re-runs. The stub-run shape itself is right and worth keeping; this is only about recording the deferred repair so it isn't lost. I'd file a tracking issue for the v41 / out-of-band backfill rather than fold it into this cherry-pick.

Technical details

# Backfill never runs for a DB already at v40

## Affected sites
- src/main/runtime/orchestration/db/schema/migrate-v40.ts:5 — `if (current >= 40) return` short-circuits the backfill.
- src/main/runtime/orchestration/db/schema/migrate.ts:19 — `if (current >= SCHEMA_VERSION) return` means no migration at all once `user_version` is 40.

## Required outcome
- A host that reaches 1.4.199-final with `remote_dispatch_attachments.home_run_id = ''` rows (from an intermediate RC, or the acknowledged rollback + re-upgrade) has those rows repaired, or the gap is explicitly tracked as a follow-up issue.

## Suggested approach (optional)
- Track a follow-up: run the backfill idempotently outside the versioned chain, or introduce v41. A one-line idempotent `INSERT OR IGNORE … SELECT … WHERE home_run_id = ''` + `UPDATE` is safe to run on every open regardless of version.

ℹ️ Nitpicks

• The new legacy-probe test (does not treat a stub-home-Run attachment %s as pre-Runs evidence) passes via either the stub id's distinctness or the notFederatedMailbox exclusion that #19542 already added — it does not specifically pin the "stub id is never run_legacy_local" property the PR body claims it proves. It is still a valid end-state regression test (it would fail if stub-run mail triggered adoption on reopen), just not a precise unit test of the distinctness.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[Jinwoo-H]

Reviewed against the shipped tree and the release branch. This is the right fix; thanks for catching it. #19542 was mine, and its "federation has not shipped" premise was wrong: the attach RPC and worker-start --on are in v1.4.198 via #9925. I verified the host never sends home_run_id back to the coordinator, so a host-only fix is sufficient and the desktop needs no update. #19542 is on release/adhoc-1.4.199-daily-202609081832-cherrypicks and nothing past 1.4.198 has shipped, so landing this before the daily means no user hits the regression.

Approve as is. Merge when CI is green and cherry-pick to the 1.4.199 release branch alongside #19542. Neither item below blocks; both are follow-ups.

1. Move the backfill into a v41 and bump SCHEMA_VERSION. Gap (b) you named: a host that rolls back to 1.4.198, attaches workers (rows land as '' via the new DEFAULT), then upgrades again never re-runs the v40 backfill because user_version already reads 40. Nobody can be in that state before 1.4.199 ships, so it can follow. Small change: keep v40 as the column add, put the INSERT OR IGNORE + UPDATE in migrate-v41.ts, register it in migrate.ts, set SCHEMA_VERSION = 41.

2. Consider keying the stub Run per home peer rather than per attachment. Something like run_<sha256(homePeerFingerprint).slice(0,12)>, minted in both createRemoteDispatchAttachment and the backfill. A worker attached before the upgrade and one attached after, from the same desktop, then file under the same Run, and an older desktop with several remote workers gets one Run row instead of one per worker. Matches the objective text ("Coordinated from ").

Ran your branch locally: federation, messaging, schema, and legacy-probe suites, 30 files / 269 tests green.

Separate from this PR and worth a pass before the daily: the 1.4.198 desktop is orchestration schema 30 and 1.4.199 is 40 with #16904 between them, so there may be other 1.4.198 → 1.4.199 federation breaks beyond the Run id. Also separate: a coordinator's check --wait goes silent when the remote link is severed rather than slow; nothing distinguishes the two today.

[Jinwoo-H]

Host-only fix for a pre-release regression from #19542. Verified the home Run id never crosses back to the coordinator; ran the federation/messaging/schema/probe suites locally (269 green). Follow-ups in my comment are non-blocking.

[pullfrog]

✅ No new issues found.

Reviewed changes

This run re-reviewed the single new commit (13a6cda) pushed since the prior review, which closes the one open concern it raised (the v40-gated backfill). The rest of the PR is unchanged.

• On-open stub-Run backfill — extracted the backfill into backfillFederatedStubHomeRuns and now call it from the OrchestrationDb constructor right after migrate(), so a database already at user_version = 40 (a rolled-back host re-attaching workers, or an intermediate RC install) is repaired on the next open instead of never.
• migrate-v40 delegates — the v40 migration now calls the shared backfill rather than inlining the SQL, and the guard is split so the backfill also runs when the column already exists but the version is still below 40.
• New tests — a whitespace-only runId refusal test, a rolled-back-host close/reopen integration test (with ablation proving it depends on the constructor call), and a dedicated federation-start-schema test file pinning the OptionalString semantics.

I verified the on-open repair is idempotent (WHERE home_run_id = '', INSERT OR IGNORE), crash-safe, and inert where it matters: stub runs carry no coordinator_handle, so rememberCurrentRunCoordinatorHandles ignores them, and the coordinator-mail trigger is untouched. The only production caller of createRemoteDispatchAttachment (federation.ts:67) is unaffected by making runId optional.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}