Repository navigation
fix(orchestration): preserve active workers across updates - #11271
Conversation
|
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:
📝 WalkthroughWalkthroughThis change adds legacy orchestration adoption and compatibility routing across storage, RPC, CLI, runtime authority verification, terminal recovery, renderer state, SSH, WSL, and Windows update flows. It introduces durable delivery and receipt contracts, authority evidence and redaction, legacy lifecycle handlers, restart reconciliation, resume fencing, recovery notifications, expanded reliability gates, and E2E coverage. The Windows updater now validates distinct installer artifacts and uses stricter process and file-lock checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR implements a seamless update migration for the Orca orchestration system, atomically adopting pre-update legacy Tasks, Dispatches, messages, and questions into a new current Run under
Confidence Score: 5/5Safe to merge. The migration is fully transactional, the authority model is consistently enforced before every state write, and the previously flagged issues have both been corrected in this diff. The v18 to v19 schema migration runs inside the existing BEGIN IMMEDIATE boundary; adoptLegacyRunIfNeeded creates the adopted run before migrating foreign-key rows and validates cross-run consistency before committing. The attestCompatibilityAuthority revocation lifecycle is wired in all the necessary eviction paths. The terminal recovery queue serialises concurrent calls correctly, and PTY adoption is gated on strict identity matching with ambiguous dispatches deferred rather than silently dropped. Files Needing Attention: No files require special attention. The heaviest logic is in db.ts and orca-runtime.ts; both have accompanying test files covering the new paths.
|
| Filename | Overview |
|---|---|
| src/main/runtime/orchestration/db.ts | Core schema v18 to v19 migration: adds delivery_contract/contract_version columns, five new tables, and the adoptLegacyRunIfNeeded atomic data-migration logic. Previously flagged LIKE-wildcard and .all() existence-check issues are both corrected in this diff. |
| src/main/runtime/orca-runtime.ts | Adds reconcileLegacyWorkerTerminals promise-queue, prepareLegacyWorkerTerminalRecovery fence logic, verifyOrchestrationCompatibilityCaller authority verification, and notifySshRelayReady double-publication for post-reconcile tab visibility. |
| src/main/runtime/rpc/orchestration-legacy-authority.ts | New file: LegacyCompatibilityAuthority resolves worker/coordinator candidates from the adopted run and guards all mutation paths behind verifyAttestedLegacyCandidate. |
| src/main/agent-hooks/server.ts | Adds hydratedAuthorityCommitments/currentAuthorityObservations tracking and attestCompatibilityAuthority, with revocation tied to pane-close, alias-clear, and tab-close events. |
| src/main/runtime/rpc/orchestration-legacy-compatibility.ts | New file: thin routing layer intercepting orchestration.send/check/ask/reply before the main RPC registry. |
| src/main/runtime/rpc/orchestration-legacy-lifecycle.ts | New file: handles legacy worker_done/heartbeat/escalation sends with idempotent commitLegacyLifecycleOperation. |
| src/main/runtime/orchestration/orchestration-legacy-worker-terminal-recovery.ts | New file: planLegacyWorkerTerminalRecovery disambiguates PTY candidates - ambiguous dispatches are deferred rather than silently adopted. |
| src/main/runtime/rpc/dispatcher.ts | Inserts OrchestrationLegacyCompatibility.tryHandle before both streaming and non-streaming dispatch paths. |
| src/main/runtime/rpc/orchestration-legacy-operation.ts | New file: shared parameter types, operationIdentity, and helpers including inferLegacyWorkerOutcome. |
| src/main/startup/legacy-worker-renderer-recovery.ts | New file: startup hook that fences auto-resume before renderer initialises, then triggers reconcileLegacyWorkerTerminals. |
Reviews (4): Last reviewed commit: "fix(orchestration): close recovery revie..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/runtime/orca-runtime.ts (1)
4181-4195: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winA failing/slow legacy reconcile now blocks recovered SSH session-tab publication.
publishRecoveredSshMobileSessionTabsis only reached ifreconcileLegacyWorkerTerminalsresolves. Since the reconcile runs behind a shared serial queue (legacyWorkerTerminalRecoveryQueue) and performs adoption + renderer reveals with retries, a throw or a slow reconcile for an unrelated connection delays or entirely skips republishing this target's recovered tabs — a user-visible regression versus the previous unconditional publish.🛡️ Decouple publication from reconcile outcome
void (async () => { - await this.reconcileLegacyWorkerTerminals({ - connectionId: targetId, - materializeRenderer: this.notifier !== null - }) + try { + await this.reconcileLegacyWorkerTerminals({ + connectionId: targetId, + materializeRenderer: this.notifier !== null + }) + } catch (error) { + console.warn('[orchestration] legacy worker reconcile failed on relay ready', { + targetId, + error + }) + } await this.publishRecoveredSshMobileSessionTabs(targetId, generation) })().catch((error) => {
🧹 Nitpick comments (7)
config/scripts/orchestration-skill-guidance.test.mjs (1)
54-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the bundled guide served by the CLI.
This test reads only
skill-guides/orchestration.md, whileskills getservesORCHESTRATION_MARKDOWNfromsrc/cli/bundled-skill-guides.ts:33. Add a parity check or exercise the bundled lookup so stale shipped guidance cannot pass this test.src/main/runtime/orca-runtime.ts (1)
3712-3728: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist the PTY liveness refresh out of the candidate loop.
refreshPtyWorktreeRecordsFromControllerissues a fulllistProcessesinventory RPC (local or over the SSH relay) and, because notargetWorktreeIdis passed, already returns the global live-PTY set. Running it once per candidate makes restart recovery O(candidates) round-trips on the reconnect path. Resolve the workspace/liveness once before the loop (or reuse a single in-flight refresh) and index candidates against that set.tests/e2e/orchestration-worker-terminal-visibility.spec.ts (1)
83-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the ledger harness shared with the sibling spec.
LedgerEvent,readLedger,isProcessAlive, and most offakeCodexSourceare duplicated verbatim intests/e2e/orchestration-legacy-worker-restart-recovery.spec.ts(lines 27-121); both copies land in this PR. A helper module undertests/e2e/helpers/(concrete name, e.g.codex-stub-ledger.ts) would keep the stub's event shape from drifting between the two suites.src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts (1)
177-194: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the stale-reconnect outcome explicit.
await staleReconnectsilently accepts either resolution or a specific rejection, andmuxInstances[2]pins the winner by construction order across three attempts. Asserting the stale promise's expected settlement (and selecting the winning mux by identity, e.g. the last instance) would keep the test from passing for the wrong reason if ownership handling changes.src/main/runtime/orchestration/types.ts (1)
228-228: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake
delivery_contractrequired onMessageRow.
MessageRowrepresents SQLite rows wheremessages.delivery_contractisNOT NULL DEFAULT 'current_delivery', and the v19 migration backfills existing rows. Making it required matches the persisted shape and avoids treatingundefinedas a fourth authority state; existing constructed messages can default omitted cases tocurrent_delivery.src/main/runtime/rpc/orchestration-legacy-compatibility-dispatcher.test.ts (1)
196-242: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case with no
orchestrationCompatibilityEvidenceat all.Every scenario supplies evidence (valid or contradictory). The absent-evidence path is a distinct branch in
LegacyCompatibilityAuthority(resolveWorkerDispatchskips its retained-assignment guard,resolveProvenCoordinatorScopestill attests), and it is the case a stale/older CLI actually produces after an update.src/main/runtime/rpc/orchestration-legacy-mail.ts (1)
173-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate of
legacyCoordinatorReadOnlyinorchestration-legacy-authority.ts.Byte-identical helper (code, message, data) exists at
orchestration-legacy-authority.tsLines 305-311. Export it from one place so the wording can't drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a5f5fea-dbb0-458a-a5dc-21ead5fcd75f
📒 Files selected for processing (86)
.github/workflows/win-update-e2e.ymlconfig/nsis/daemon-host-uninstall.nshconfig/reliability-gates.jsoncconfig/scripts/orchestration-skill-guidance.test.mjsdocs/reference/plans/2026-07-28-orchestration-seamless-update-migration.mdnative/windows-cli-launcher/OrcaCliLauncher.csskill-guides/orchestration.mdsrc/cli/bundled-skill-guides.tssrc/cli/handlers/orchestration-migration.test.tssrc/cli/handlers/orchestration-timeout-cli.test.tssrc/cli/handlers/orchestration-timeout.test.tssrc/cli/handlers/orchestration-windows-ask-cli.test.tssrc/cli/handlers/orchestration.test.tssrc/cli/handlers/orchestration.tssrc/cli/runtime-client.test.tssrc/cli/runtime/client.tssrc/cli/runtime/orchestration-compatibility-envelope.tssrc/cli/runtime/runtime-request-timeout.tssrc/cli/runtime/transport.tssrc/cli/runtime/types.test.tssrc/cli/runtime/types.tssrc/main/agent-hooks/server-authority-evidence.test.tssrc/main/agent-hooks/server.tssrc/main/cli/windows-launcher-asset.test.tssrc/main/index.tssrc/main/ipc/pty.tssrc/main/pty/wsl-orca-env.test.tssrc/main/pty/wsl-orca-env.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/orchestration-compatibility-authority.test.tssrc/main/runtime/orchestration/db.test.tssrc/main/runtime/orchestration/db.tssrc/main/runtime/orchestration/formatter.test.tssrc/main/runtime/orchestration/formatter.tssrc/main/runtime/orchestration/orchestration-legacy-storage-db.test.tssrc/main/runtime/orchestration/orchestration-legacy-worker-terminal-recovery.test.tssrc/main/runtime/orchestration/orchestration-legacy-worker-terminal-recovery.tssrc/main/runtime/orchestration/orchestration-schema-version-skew.tssrc/main/runtime/orchestration/orchestration-version-skew-migration.test.tssrc/main/runtime/orchestration/types.tssrc/main/runtime/rpc/core.tssrc/main/runtime/rpc/dispatcher.tssrc/main/runtime/rpc/errors.tssrc/main/runtime/rpc/methods/orchestration-federation-control-mail.test.tssrc/main/runtime/rpc/methods/orchestration-federation.test.tssrc/main/runtime/rpc/methods/orchestration-gates.tssrc/main/runtime/rpc/methods/orchestration-runs.tssrc/main/runtime/rpc/methods/orchestration.tssrc/main/runtime/rpc/orchestration-legacy-authority.tssrc/main/runtime/rpc/orchestration-legacy-compatibility-dispatcher.test.tssrc/main/runtime/rpc/orchestration-legacy-compatibility.tssrc/main/runtime/rpc/orchestration-legacy-lifecycle.tssrc/main/runtime/rpc/orchestration-legacy-mail.tssrc/main/runtime/rpc/orchestration-legacy-operation.tssrc/main/runtime/rpc/orchestration-legacy-question.tssrc/main/ssh/ssh-relay-session-reconnect-incarnation.test.tssrc/main/ssh/ssh-relay-session.tssrc/main/ssh/ssh-remote-cli-host-passthrough.test.tssrc/main/ssh/ssh-remote-cli-host-passthrough.tssrc/main/ssh/ssh-remote-orca-cli.test.tssrc/main/ssh/ssh-remote-orca-cli.tssrc/main/window/attach-main-window-services.tssrc/preload/api-types.tssrc/preload/index.tssrc/relay/remote-cli-env.test.tssrc/relay/remote-cli-env.tssrc/renderer/src/hooks/useIpcEvents.test.tssrc/renderer/src/hooks/useIpcEvents.tssrc/renderer/src/lib/resume-sleeping-agent-session-legacy-worker.test.tssrc/renderer/src/lib/resume-sleeping-agent-session.tssrc/renderer/src/store/slices/agent-status.tssrc/renderer/src/web/web-preload-api.tssrc/shared/agent-session-resume.tssrc/shared/orchestration-compatibility-evidence.test.tssrc/shared/orchestration-compatibility-evidence.tssrc/shared/remote-runtime-client.tssrc/shared/runtime-rpc-envelope.tssrc/shared/workspace-session-sleeping-agents.tstests/e2e/orchestration-legacy-worker-restart-recovery.spec.tstests/e2e/orchestration-worker-terminal-visibility.spec.tstools/win-update-e2e/README.mdtools/win-update-e2e/cli-args.mjstools/win-update-e2e/installer-pair-proof.mjstools/win-update-e2e/installer-pair-proof.test.mjstools/win-update-e2e/run.mjs
|
Too many files changed for review. ( Bypass the limit by tagging |
…11271) * fix(orchestration): preserve active workers across updates * test(ssh): model absent legacy adoption * test(orchestration): align compatibility contracts * fix(windows): escape updater PowerShell booleans * fix(windows): restore stock uninstall process check * fix(orchestration): keep recovery off renderer startup barrier * fix(orchestration): harden legacy recovery migration * fix(orchestration): close recovery review gaps * fix(orchestration): complete legacy worker cutover recovery * fix(orchestration): preserve legacy workers across updates --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
Summary
send,check,ask,reply, and completion prompts working through a narrowly attested per-Dispatch adapter; unprovable history is clearly read-only with no reply or acknowledgment affordancecodex resumeworkers on workspace re-entrydecision_gatedeliveryDeliberate scope
This does not revive the retired scheduler, block updates, permanently fence PTY geometry or SIGWINCH, or claim exhaustive updater/topology coverage. Normal terminal resize behavior resumes after intentional viewing. The migration guarantees safe, non-destructive, agent-recoverable continuity rather than perfect preservation of every transient state.
Design and invariants are documented in
docs/reference/plans/2026-07-28-orchestration-seamless-update-migration.md.Verification
pnpm typecheckpnpm lintpnpm test: 3,799 files and 40,149 tests passedpnpm exec electron-vite build --mode e2egit diff --checkScreenshots
Not applicable; this is runtime migration and terminal lifecycle behavior.
AI review report
Prior CodeRabbit, Greptile, and internal review findings were triaged by impact. Valid authority, host-routing, question-delivery, duplicate-worker, and tab-recovery findings were fixed. Permanent geometry fencing and exhaustive topology guarantees were intentionally removed from scope.
Security audit
Legacy mutations require attested principal and Dispatch authority. Ambiguous or unprovable callers degrade to read-only inspection; no fallback executes a mutation locally.
Notes
No update blocking and no dual scheduler.