From b628b490161de7302a8849fcc4a612a2df061a00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Mon, 20 Apr 2026 14:44:59 -0400 Subject: [PATCH] Hotfix B2 Phase 1: drop task_leases.run_id FK (v51 migration) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production bug on launch — user reports UI 'FOREIGN KEY constraint failed' on 启动公司 and the main-process log loops forever with '[heartbeat] CEO not ready: checkout_race_lost'. Both trace to the same root cause introduced by v50 (e2c839b..2656684 B2 Phase 1): task_leases.run_id TEXT NOT NULL REFERENCES runs(id) The orchestration sequence contradicts this FK. In both heartbeat.ts (line 165) and task-runner.ts (line 91), the caller generates a fresh runId = randomUUID() and immediately calls db.tasks.checkoutTask BEFORE the corresponding row is inserted into the runs table. The legacy tasks.checkout_run_id column has always been 'dangling allowed' — the run row saves later when task-runner.ts opens the run record. v50's FK tightened the shadow lease contract without coordinating with that insertion sequence, so every checkoutTask attempt now rolls back with a FK violation, takes the whole CAS transaction with it, returns changes=0, and the heartbeat reports checkout_race_lost. The task never advances — the exact orphan symptom B2 was supposed to eliminate. Fix: migration v51 recreates task_leases WITHOUT the run_id FK. - task_id FK + ON DELETE CASCADE stays (a lease for a non-existent task is still meaningless) - UNIQUE(task_id, run_id) stays (still a correctness constraint) - run_id loses the FK — it mirrors the legacy checkout_run_id's loose semantics, which is exactly what Phase 1 dual-write is supposed to mirror - Existing data preserved verbatim via INSERT SELECT - Uses PRAGMA defer_foreign_keys=ON inside the transaction (the pragma is valid inside a txn; regular foreign_keys=OFF would not be since runMigrations already opened BEGIN IMMEDIATE) Post-v51 the run_id column is still NOT NULL and the shadow probe (ab4afba) will still surface missing runs as divergence — the FK tightening is a Phase 2 concern coordinated with the saveRun insertion order, not a Phase 1 shadow-write concern. typecheck green, 529 unit tests green, 8/8 eval green. --- src/main/database/migrations/v51-v51.ts | 89 +++++++++++++++++++++++++ src/main/database/schema.ts | 2 + 2 files changed, 91 insertions(+) create mode 100644 src/main/database/migrations/v51-v51.ts diff --git a/src/main/database/migrations/v51-v51.ts b/src/main/database/migrations/v51-v51.ts new file mode 100644 index 00000000..66953c46 --- /dev/null +++ b/src/main/database/migrations/v51-v51.ts @@ -0,0 +1,89 @@ +import log from "electron-log/main.js"; +import type { Migration, MigrationDeps } from "./types"; +import { tableExists } from "./types"; + +/** + * B2 Phase 1 hotfix — relax `task_leases.run_id` FK to `runs(id)`. + * + * v50 created `task_leases.run_id TEXT NOT NULL REFERENCES runs(id)`. + * That FK was a well-intentioned tightening over the legacy + * `tasks.checkout_run_id` column (which had no FK at all), but it + * broke the orchestration sequence: `heartbeat.ts` and `task-runner.ts` + * generate a fresh `runId = randomUUID()` and call `checkoutTask` on + * the task *before* the corresponding row is inserted into `runs`. + * With the FK on, `writeCheckoutLease` → `INSERT INTO task_leases` + * fails with `FOREIGN KEY constraint failed`, the whole CAS rolls + * back, `checkoutTask` returns false, and the heartbeat loops on + * `checkout_race_lost` forever — the exact orphan symptom B2 was + * supposed to *eliminate*. + * + * The legacy `tasks.checkout_run_id` was always "dangling allowed" + * (the run row is saved later by the task-runner), and during B2 + * Phase 1 the lease row is the shadow mirror of that same legacy + * intent. It must mirror the legacy's loose semantics until Phase 2 + * tightens the whole contract in one coordinated step. + * + * Fix: recreate `task_leases` without the `run_id` FK. `task_id` + * keeps its FK + ON DELETE CASCADE (you cannot lease a task that + * does not exist) and the UNIQUE(task_id, run_id) pair survives + * (still meaningful, just no longer FK-checked). Existing rows are + * preserved verbatim via INSERT SELECT. + * + * PRAGMA foreign_keys cannot be toggled inside an open transaction + * (SQLite restriction), so we follow the pattern used by v40 / v41 / + * v43 / v44 — but those migrations run their own rebuilds *without* + * nesting inside the outer transaction boundary. In our case the + * outer `runMigrations` wrapping in `schema.ts` already opens + * BEGIN IMMEDIATE, so we use `PRAGMA defer_foreign_keys = ON` + * instead, which *is* valid inside a transaction and defers all FK + * checks until COMMIT. Since we are removing an FK rather than + * relying on one, the defer_foreign_keys toggle simply prevents a + * spurious check during the INSERT SELECT window. + */ + +export function createMigrationsV51V51(deps: MigrationDeps): Migration[] { + const { db } = deps; + return [ + { + version: 51, + name: "task_leases_drop_run_id_fk", + fn: () => { + if (!tableExists(db, "task_leases")) { + log.info("[migration v51] task_leases table missing; nothing to do"); + return; + } + + db.exec("PRAGMA defer_foreign_keys = ON"); + + db.exec(` + CREATE TABLE task_leases_new ( + task_id TEXT PRIMARY KEY REFERENCES tasks(id) ON DELETE CASCADE, + run_id TEXT NOT NULL, + acquired_at TEXT NOT NULL, + expires_at TEXT NOT NULL, + checkout_run_id TEXT, + UNIQUE(task_id, run_id) + ) + `); + + db.exec(` + INSERT INTO task_leases_new + (task_id, run_id, acquired_at, expires_at, checkout_run_id) + SELECT task_id, run_id, acquired_at, expires_at, checkout_run_id + FROM task_leases + `); + + db.exec("DROP INDEX IF EXISTS idx_task_leases_expires_at"); + db.exec("DROP TABLE task_leases"); + db.exec("ALTER TABLE task_leases_new RENAME TO task_leases"); + db.exec( + "CREATE INDEX IF NOT EXISTS idx_task_leases_expires_at ON task_leases(expires_at)", + ); + + log.info( + "[migration v51] task_leases recreated without run_id FK; task_id FK + UNIQUE(task_id, run_id) preserved", + ); + }, + }, + ]; +} diff --git a/src/main/database/schema.ts b/src/main/database/schema.ts index 6eacf956..4534ab4a 100644 --- a/src/main/database/schema.ts +++ b/src/main/database/schema.ts @@ -16,6 +16,7 @@ import { createMigrationsV47V47 } from "./migrations/v47-v47"; import { createMigrationsV48V48 } from "./migrations/v48-v48"; import { createMigrationsV49V49 } from "./migrations/v49-v49"; import { createMigrationsV50V50 } from "./migrations/v50-v50"; +import { createMigrationsV51V51 } from "./migrations/v51-v51"; import { columnExists, nowIso, @@ -212,6 +213,7 @@ export function runMigrations( ...createMigrationsV48V48(deps), ...createMigrationsV49V49(deps), ...createMigrationsV50V50(deps), + ...createMigrationsV51V51(deps), ]; // Pre-migration snapshot. If any migration is pending we take a full