[Air #1476] Extract runGlobalMigrations(db) so migration tests drive the real production runner

[mohidmakhdoomi]

Closes#1476

What Changed

The global.db migration sequence lived inline inside the private ensureGlobalDatabase() path in db/index.ts, so migration tests could only drive hand-maintained replicas of it, kept honest by source guards. That is one source of truth per migration, times two, maintained by hand — a tax paid again at every new migration (v15 → v16 → v17 already).

• New packages/codev/src/agent-farm/db/migrations.ts — GLOBAL_CURRENT_VERSION (17) and runGlobalMigrations(db, options?). The v2 → v17 chain moved over verbatim; no SQL changed.
• db/index.ts now calls runGlobalMigrations(db) on the existing-database path and imports GLOBAL_CURRENT_VERSION for fresh-install marker stamping. Both are re-exported from db/index.ts, so no callsite has to learn a new module.
• spec-1313-migration.test.ts rewritten to drive the real runner. No replicas remain in it.

Key Decisions

Two defaulted seams, both defaulting to today's production behavior, are what make the runner callable from a test:

• options.log (default console.log) — tests collect the per-migration lines instead of spamming stdout, and can assert which steps actually ran.
• options.runDir (default ~/.codev/run) — migration v8 renames shepherd-*.sock files on disk. That is the one genuine filesystem side effect in the chain; without this seam, a test driving the real v8 would rename a developer's live sockets.

The runner is only safe on a database that reached its recorded version through migrations. A fresh GLOBAL_SCHEMA database with no markers would fail at v5, which selects terminal_sessions.project_path — which is exactly why ensureGlobalDatabase() stamps every marker on the fresh path rather than running the chain. Behavior is unchanged; the constraint is now documented at the runner.

Out of scope, deliberately left alone: the pir-832 / bugfix-826 / spec-755 migration tests. Those replicate migrations of the retired per-workspace state.db, which has no production runner to call — extracting one would be inventing a runner for a dead path.

Test Plan

• Unit tests added
• Build passes (pnpm --filter @cluesmith/codev build)
• All tests pass — 4861 passed, 48 skipped, 0 failures

The rewritten suite keeps the v15 / v16 / v17 coverage and adds what only a callable runner allows:

• the full v1 → v17 chain on a legacy database, stamping every marker;
• whole-database convergence with a fresh GLOBAL_SCHEMA install — all tables, columns and indexes, where before only the one table under test was compared;
• v9's project_path → workspace_path data carry-over plus v13's architect role_id backfill, asserted on real rows through the v7/v8/v9 table rebuilds that no replica ever reproduced;
• v8's socket rename against an injected run directory, and a missing run directory.

Review Notes

• db/migrations.ts should read as a pure move of the migration block. Worth diffing the SQL statement-by-statement against the pre-change db/index.ts: the only intended edits are console.log(...) → log(...), the v8 runDir now coming from options, and one comment that said "handled above" now saying "handled by the caller".
• Because the runner applies every outstanding step, a pre-v15 fixture now walks v15 → v16 → v17 in a single call — the same thing a real upgrading install does. That is why the v15 mailbox assertions include not_before (v17). The old test simulated that with a hand-written ALTER.
• The two remaining source guards (send-architect-identity.test.ts, bugfix-506-annotator-worktree-cwd.test.ts) were retargeted from db/index.ts to db/migrations.ts. They are now partly redundant with the behavioral tests; kept as-is to hold this PR to its scope.

🤖 Generated with Claude Code

[mohidmakhdoomi]

Architect integration review — 3-way CMAP (risk tier: High — global.db state layer, >500 lines)

Verdict: APPROVE — unanimous (gemini / codex / claude, all HIGH confidence, zero blocking issues). Parked for maintainer approval + merge; we are not maintainers.

Independently verified by the architect before consultation:

• Faithful move confirmed mechanically. Normalized diff of the old inline block (db/index.ts) vs db/migrations.ts shows only: the extraction preamble, console.log → log, the runDir hoist into options, and the dropped return db. No SQL changed. (Claude's independent mechanical diff reached the same conclusion.)
• ensureGlobalDatabase wiring: fresh path untouched, existing path delegates to runGlobalMigrations(db) with production defaults.
• Blast radius: only db/index.ts opens global.db read-write; every other opener is readonly → exactly one production caller, behavior-preserving at the system level. Internal export only (no exports map in the package); no skeleton mirror owed.

Non-blocking findings (from CMAP, for builder to address-or-rebut in this PR or spin off):

1. Precondition guard (recommended in-PR):runGlobalMigrations on a marker-less fresh-shape DB dies mid-chain at v5 with an opaque SQLite error (project_path no longer exists). Unreachable today, but the extraction's whole point is a callable runner — a ~6-line fresh-shape detect → named error at entry converts the footgun into a diagnosable failure. Pairs with codex's note that the JSDoc ("safe on any existing database from v1 onward") overclaims the actual contract.
2. Follow-up: the literal GLOBAL_CURRENT_VERSION = 17 source guard in send-architect-identity.test.ts:239 is now redundant with the behavioral full-chain test and adds per-migration edit friction.
3. Follow-up: test harness sets only WAL; production also sets foreign_keys/synchronous/busy_timeout — matching pragmas would close the last fidelity gap (v7–v9 use DROP+RENAME rebuilds, exactly the SQL sensitive to foreign_keys).
4. Follow-up (MAINTAIN-sized): stale "replica of db/index.ts" comments in the three retired-state.db migration tests; plus one arch.md line stating the forward-only chain now lives in db/migrations.ts and must converge with db/schema.ts.

[mohidmakhdoomi]

Builder response to the architect CMAP findings

Finding 1 — precondition guard: accepted, done (235b490c).

runGlobalMigrations now rejects the one shape it cannot handle, at entry, before any step runs:

terminal_sessions is already workspace_path-shaped AND no v9 marker
→ Error: "...fresh GLOBAL_SCHEMA database that never walked the chain — stamp markers instead"

That is exactly the marker-less-fresh-schema case that used to die at v5 on no such column: project_path. Production never reaches it (ensureGlobalDatabase stamps 1..GLOBAL_CURRENT_VERSION on the fresh path) — it exists because the runner is now callable from anywhere, which was the point of the extraction. The JSDoc no longer claims "safe on any existing database from v1 onward"; it states the real contract and documents the throw.

Two tests: the marker-less fresh schema throws and leaves _migrations empty (the guard fires before any step), and the same schema with markers stamped — the real production path — is a clean no-op.

Finding 3 — pragma fidelity: accepted, done (same commit). The test harness set only journal_mode = WAL; it now sets production's full configurePragmas() set (synchronous = FULL, busy_timeout, foreign_keys = ON). Cheap, and you are right that it matters specifically for v7–v9, which rebuild tables with DROP + RENAME.

Finding 4 — stale replica comments: already done in 5a70fbb2 (pushed before your review landed, so the diff you read may predate it). pir-832, spec-755 and bugfix-826 now say which numbering space they are in and note that the retired state.db has no production runner to call — the wrong turn they invited was pointing a maintainer at migrations.ts's v12, a different migration entirely. The arch.md line is architect/MAINTAIN material, not builder scope; leaving it to you.

Finding 2 — redundant GLOBAL_CURRENT_VERSION = 17 source guard: acknowledged, left in place. No disagreement that the new full-chain marker assertion covers it behaviorally and better (it catches the bump-without-migration direction too). I left it because deleting an assertion from a Spec 1313 test is a change to that test's intent rather than to this issue's, and this PR is already the one retiring the replica precedent. Happy to drop it in the follow-up, or now if the maintainer prefers it gone before merge.

Full suite after the changes: 4863 passed, 48 skipped, 0 failures; build clean.

[waleedkadous]

Thank you for the discipline on this one — claude's lane did a mechanical normalized diff of main's inline block against migrations.ts and confirmed zero SQL changed (only the runDir hoist, one comment reflow, and the dropped return db), and proved the new precondition guard can't false-positive because v9 wraps its rebuilds and marker in one transaction. Approving.

The best part isn't in the stated scope: the convergence test that walks v1→v17 and diffs it against a fresh GLOBAL_SCHEMA install on tables, columns, indexes and triggers closes a real latent hazard in both directions. I'll take the arch.md follow-up myself (add db/migrations.ts to Core Components and the repo tree, and name that test as the enforcing guard) — that's architect territory, as you correctly left it.

One optional tidy-up for a follow-up: the literal GLOBAL_CURRENT_VERSION = 17 source guard in send-architect-identity.test.ts:239 is now behaviorally covered and just adds a per-migration edit tax.