Skip to content

feat(orchestration): coordinator resume-on-boot (F3, #14) - #21

Merged
zaridan merged 2 commits into
mainfrom
zaridan/feat-orch-14-resume-on-boot
Jun 24, 2026
Merged

feat(orchestration): coordinator resume-on-boot (F3, #14)#21
zaridan merged 2 commits into
mainfrom
zaridan/feat-orch-14-resume-on-boot

Conversation

@zaridan

@zaridan zaridan commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Part of epic #5. Closes #14. Depends on F1 (#12) + F2 (#13), both merged to main.

Problem

The Coordinator is an in-memory instance with no boot hook. After an app restart, a leftover coordinator_runs.status='running' row is a zombie: it trips F1's per-target active-run guard (startCoordinatorRun's BEGIN IMMEDIATE check) and refuses a fresh orchestration.run for that target — while nothing drives the old run. With a headless/recipe director there's no LLM agent to re-issue the run, so the target is blocked forever.

MUST — the floor (zombie fix) ✅

New reconcileCoordinatorRunsOnBoot (boot-resume.ts) scans every status='running' run on boot and converges each:

  • finalize — all tasks terminal (or none) → the loop already finished; mark completed/failed.
  • resume — outstanding work + resumable → restart a converging loop (SHOULD, below).
  • fail — the floor: anything not resumable is marked failed so the guard unblocks.

Hard rule honored: a run that can't be resumed always converges to failed, never left running with no loop (resume decline/throw both fall through to failed). Idempotent (a second pass finds no running rows). F1-isolation aware (run-scoped + target_key). Wired into desktop boot behind the experimentalOrchestrators flag, fire-and-forget so a reconcile error can never block startup, and runs before any window is shown (before a user could hit the guard).

SHOULD — real resume + worktree re-adoption (per F2-DESIGN §8) ✅

  • Schema v9 persists the in-memory-only coordinator options (max_concurrent, worktree_backed, worker_agent) so a restart rebuilds the same run instead of guessing. Guessing legacy-mode for a worktree-backed run would dispatch into a bare shell that never reports done — a fresh zombie. Additive/nullable; pre-v9 rows read NULL → resume falls back to defaults.
  • resumeCoordinatorRunOnBoot rebuilds the coordinator, reclaims dead in-flight dispatches (dispatched → ready via the existing breaker) so the loop is guaranteed to converge, and re-adopts existing track worktrees: buildAdoptedTrackWorktrees scans the director's lineage children (parentWorktreeId === directorWorktreeId, same run — the exact data Mission Control uses) and Coordinator.seedAdoptedTrackWorktrees pre-seeds the track map. The next same-track dispatch is a hit — the worker agent is relaunched in the existing checkout (preserving commits), no duplicate worktree/branch.
  • Resume decision (documented): resumable only if the run has a worktree target AND the director worktree still resolves; otherwise → failed. Reclaim burns one breaker strike per restart, bounding an infinite restart→resume loop on a poison run.

Cross-cutting

Cross-platform / SSH-aware: resume routes through the existing runtime (resolveOrchestrationTargetKey, listWorktreeLineage, createManagedWorktree), inheriting the SSH/relay machinery — no local-only assumptions. Provider-neutral.

Tests (fail without the fix)

  • boot-resume.test.ts: orphaned running run reconciled to failed AND a fresh run for the target then starts (proves the guard unblocked — it throws CoordinatorRunConflictError before the reconcile); idempotency; finalize-when-done (completed / failed / zero-tasks); resume decision (succeed / decline / throw → never left running); target isolation; reclaim; lineage-based track re-adoption (dedupe by track, cross-run/cross-parent children ignored).
  • coordinator.test.ts: a seeded track is re-adopted on resume — no createWorktree, agent relaunched in the existing checkout.
  • db.test.ts: v9 options round-trip (worktree_backed=false0, unset → NULL).
  • Hardened a pre-existing ~40%-flaky test (respects maxConcurrent limit) that completed tasks in fixed array order while the cap picks 2-of-3 by random id; now completes tasks as actually dispatched and always drains the loop. Verified 0/20 + combined suite 0/5.

Verification

  • vitest orchestration + Mission Control + RPC + CLI suites green (288 passed, 5× stable).
  • typecheck (node/cli/web) clean — only 4 pre-existing TuiAgent errors in orca-runtime.test.ts (present on main).
  • oxlint clean; electron-vite build green.

Deferred / follow-ups

  • Per-run stop selection on a resumed coordinator stays as today (latest-wins) — F4's domain.
  • Auto-cleanup-on-merge of track worktrees remains out of scope (F2-DESIGN §8).

Do not merge — review first.

🤖 Generated with Claude Code

The Coordinator is an in-memory instance with no boot hook, so after an app
restart a leftover `coordinator_runs.status='running'` row is a zombie: it trips
F1's per-target active-run guard and blocks a fresh run for that target while
nothing drives the old one. F3 adds a startup reconciler that, on boot, scans
every running run and converges it.

MUST (the floor — zombie fix):
- `reconcileCoordinatorRunsOnBoot` (boot-resume.ts) classifies each running run:
  finalize (work already done → completed/failed), resume, or — the floor —
  mark failed. A run that cannot be resumed always converges to failed, never
  left running with no loop. Idempotent (a second pass finds no running rows),
  F1-isolation aware (run-scoped + target_key), wired into desktop boot behind
  the experimental flag, fire-and-forget so it can never block startup.

SHOULD (real resume + worktree re-adoption, per F2-DESIGN §8):
- Persist the in-memory-only coordinator options (schema v9: max_concurrent,
  worktree_backed, worker_agent) so a restart rebuilds the SAME run instead of
  guessing — a legacy-mode guess of a worktree-backed run would dispatch into a
  shell that never completes (a fresh zombie).
- `resumeCoordinatorRunOnBoot` rebuilds the coordinator, reclaims dead in-flight
  dispatches so the loop is guaranteed to converge, re-adopts existing track
  worktrees by scanning the director's lineage children (parentWorktreeId ===
  directorWorktreeId, same run — the data Mission Control uses) via
  `buildAdoptedTrackWorktrees` + `Coordinator.seedAdoptedTrackWorktrees`, and
  relaunches the worker agent in the existing checkout rather than forking a new
  worktree/branch. Declines (→ failed) when the director worktree is gone.

Tests:
- boot-resume.test.ts: orphaned running run reconciled to failed AND a fresh run
  for the target then starts (guard unblocked); idempotency; finalize-when-done;
  resume decision (succeed/decline/throw); target isolation; reclaim; track
  re-adoption from lineage.
- coordinator.test.ts: a seeded track is re-adopted on resume (no duplicate
  worktree; agent relaunched in the existing checkout).
- Hardened a pre-existing ~40%-flaky test ("respects maxConcurrent limit") that
  completed tasks in a fixed array order while the cap picks 2 of 3 by random id
  — now completes tasks as actually dispatched and always drains the loop.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@zaridan

zaridan commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

Review panel — not merge-ready (verified against source)

3-lens panel; the load-bearing findings re-verified against the F3 source by the coordinator. No blockers, but the HARD RULE (no run left running with no loop) has real escape hatches, and the production resume path is untested.

🟠 Must-fix

  1. Catch-less detached loop → fresh zombie (HARD-RULE violation). runFromExistingRun(run.id).finally(...) (orchestration-gates.ts:324) has no .catch, and adoptUnownedTasks (coordinator.ts:335) runs outside executeLoop's try (:337). If that DB call throws, the loop rejects un-finalized, .finally swallows it, and resume() already returned true so the reconciler skips its failed-fallback → run stays running with no loop. Fix: .catch that marks the run failed, and/or move adoptUnownedTasks inside the try.
  2. Real resume path untested. Tests call reconcileCoordinatorRunsOnBoot with no resume (MUST-only mode); resumeCoordinatorRunOnBoot (target guard, reclaim, v9 rebuild, lineage→adopt→seed, loop fire) is never exercised end-to-end — which is why feat(orchestrators): Orcastrator — director sidebar, Send to Orcastrator, and Mission Control #1 ships green. Test the real callback incl. the failure→failed branches and the catch-less-loop case.

🟠 Should-fix (same pass)

  1. Pre-v9 worktree_backed = NULL resumes in legacy mode → unbounded re-zombie. Treat worktree_backed === null with a worktree: target as not-safely-resumable → failed.
  2. No cross-process claim on resume → double-drive. Reconcile runs on serve/SSH boot too (index.ts:1536, no serve guard) and resume skips F1's BEGIN IMMEDIATE per-target guard. Route resume through the guard / atomic compare-and-set, or guard the redundant boot path.
  3. Partial reconcile failure strands later rows. Per-run try/catch (log + continue) so one transient SQLITE_BUSY doesn't leave later runs running.

Nits

Distinguish selector_not_found from transient errors in the bare catch; idempotency guard for resumed rows; await-or-document the boot-window race before the RPC binds.

No-merge until #1 + #5 land; #2#4 in the same pass.

…14, round 2)

Round-2 review found a real HARD-RULE violation (a run left running with no loop)
and that the production resume path was untested. This closes both plus the
should-fixes.

Must-fix:
1. Catch-less detached loop → fresh zombie. `executeLoop` ran `adoptUnownedTasks`
   (+ the pre-read) OUTSIDE its `try`, and the resume's `runFromExistingRun(...)`
   had no `.catch`. A throw there rejected the loop un-finalized and the reconciler
   had already skipped its failed-fallback → run stuck `running` with no loop.
   Fix (belt + suspenders): moved the pre-loop work INSIDE `executeLoop`'s try, and
   added a `.catch` on the resumed loop that force-marks the run `failed`.
2. Real resume path now tested. New `orchestration-resume-on-boot.test.ts` drives
   the production `resumeCoordinatorRunOnBoot` via `runOrchestrationBootReconcile`
   with a fake runtime + real DB: resumable run → loop starts + track re-adopted
   (no duplicate worktree); the catch-less-loop case (throwing `adoptUnownedTasks`
   → run ends `failed`, never `running`); and every declines→`failed` branch.
   The hard-rule + NULL tests FAIL against round-1 code (verified).

Should-fix (same pass):
3. Pre-v9 `worktree_backed = NULL` with a worktree target is now treated as
   not-safely-resumable → `failed` (guessing legacy would dispatch into a bare
   shell that never converges → unbounded re-zombie). Only an explicit
   `worktree_backed = 0` resumes in legacy mode.
4. Cross-process double-drive closed. Schema v10 adds `coordinator_runs.resumed_at`,
   a boot-time-fenced atomic claim (`tryClaimRunForResume`, BEGIN IMMEDIATE). The
   resume callback claims FIRST; a loser returns `contended` → the run is left
   running (a live owner drives it), never double-driven and never failed out from
   under its owner. A strictly-greater later fence reclaims a crashed resumer's
   stale claim, so a crash mid-resume can't strand the run. Makes the (intentionally
   redundant) serve-mode boot reconcile safe without a serve guard.
5. Per-run try/catch in the reconcile loop: one row's transient error (e.g.
   SQLITE_BUSY) no longer strands every later running row (`reconcile-error`
   disposition, logged, continues).

Nits: claim-based idempotency replaces "idempotent by construction" (a second pass
is `contended`→`skipped`); director-resolve failure documented as fail-closed
(favoring the hard rule) with transient-vs-not-found distinction noted as follow-up.

Verification: vitest orchestration + MC + RPC suites green (297 passed, 5x stable);
typecheck node/cli/web clean (only 4 pre-existing TuiAgent errors on main); oxlint
clean; electron-vite build green. New resume tests verified red against round-1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@zaridan

zaridan commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

Round 2 — hard-rule escape hatches closed (commit 31f7542)

All must-fix + should-fix landed; the previously-untested production resume path now has end-to-end coverage that fails against round-1 code.

Must-fix

  1. Catch-less detached loop → fresh zombie (HARD-RULE). Moved adoptUnownedTasks + the pre-read inside executeLoop's try (a throw now routes to the failure-finalize), and added a .catch on the resumed runFromExistingRun(...) that force-marks the run failed and clears activeCoordinator. Belt + suspenders — no rejection path can leave a run running with no loop.
  2. Real resume path tested. New orchestration-resume-on-boot.test.ts drives the production resumeCoordinatorRunOnBoot via runOrchestrationBootReconcile (fake runtime + real DB): resumable run → loop starts + track re-adopted (no duplicate worktree); the catch-less-loop case (throwing adoptUnownedTasks → run ends failed, never running); every declines→failed branch (no-target / pre-v9-NULL / director-gone); and the contention→skipped case. Verified red against round-1 (HARD-RULE timed out stuck running; NULL returned resumed).

Should-fix

  1. Pre-v9 worktree_backed = NULL → not safely resumable → failed. Guessing legacy for an unknown-mode run would dispatch into a bare shell that never converges (unbounded re-zombie). Only an explicit worktree_backed = 0 resumes legacy.
  2. Cross-process double-drive closed. Schema v10 adds coordinator_runs.resumed_at, a boot-time-fenced atomic claim (tryClaimRunForResume, BEGIN IMMEDIATE). Resume claims first; a loser returns contended → the run is left running (a live owner drives it), never double-driven and never failed out from under its owner. A strictly-greater later fence reclaims a crashed resumer's stale claim, so a crash mid-resume can't strand the run. This makes the (intentionally redundant) serve-mode boot reconcile safe without a serve guard.
  3. Partial reconcile failure no longer strands later rows. Per-run try/catch (reconcile-error disposition, logged, continues) — one transient SQLITE_BUSY can't abort the loop.

Nits

  • Idempotency is now claim-based (a second pass is contendedskipped); comment updated.
  • Director-resolve failure documented as fail-closed (favoring the hard rule); transient-vs-not-found distinction noted as a follow-up.

Verification

vitest orchestration + MC + RPC green (297 passed, 5× stable) · typecheck node/cli/web clean (only 4 pre-existing TuiAgent errors on main) · oxlint clean · electron-vite build green. New resume tests verified red against round-1.

Still do not merge — review.

@zaridan
zaridan merged commit 194b98b into main Jun 24, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bridge F3: Coordinator resume-on-boot (kill the zombie-director failure)

1 participant