fix(bootstrap): first-boot FK race + single-flight

packages/core/src/__tests__/middleware/bootstrap-fk-ordering.test.ts

@@ -0,0 +1,122 @@
+// @ts-nocheck
+// Regression: the bootstrap FK-ordering guarantee (first-boot race).
+//
+// bootstrap.ts seeds system data on a fresh DB. Two kinds of step run there:
+//   PRODUCERS — bootstrapDocumentTypes / autoRegisterCollectionDocumentTypes — create
+//               `document_types` rows.
+//   CONSUMERS — RBAC seed / core-plugin bootstrap — `INSERT INTO documents`, whose
+//               `type_id` FK-references those `document_types` rows (0002_documents.sql:30).
+// The pre-fix code ran all of them in one Promise.all, so on a cold DB a consumer insert
+// could win the race against its producer and hit `FOREIGN KEY constraint failed` (each
+// step's error was caught+swallowed, leaving RBAC roles / plugins unseeded — then a KV
+// marker latched the partial state for 24h).
+//
+// The shared d1-sqlite harness disables FK enforcement (D1 doesn't reliably enforce it and
+// services delete derived rows explicitly). Here the FK IS the subject, so we turn it back
+// ON for the DB under test — it is exactly what production D1 enforced when it threw.
+import { Hono } from 'hono'
+import { describe, it, expect, beforeEach, afterEach } from 'vitest'
+import { createTestD1 } from '../utils/d1-sqlite'
+import { DocumentsService } from '../../services/documents'
+import { bootstrapDocumentTypes } from '../../services/document-types-seed'
+import { bootstrapMiddleware, resetBootstrap } from '../../middleware/bootstrap'
+
+function fkOnDb() {
+  const db = createTestD1()
+  db.raw.pragma('foreign_keys = ON') // re-enable for this test (harness default is OFF)
+  return db
+}
+
+describe('bootstrap FK ordering (first-boot race regression)', () => {
+  let db
+  beforeEach(() => { db = fkOnDb() })
+  afterEach(() => db.close())
+
+  it('reproduces the race failure: inserting a document before its type exists throws FK', async () => {
+    // CONSUMER before PRODUCER — what the pre-fix parallel batch allowed on a cold DB.
+    const svc = new DocumentsService(db, { tenantId: 'default' })
+    await expect(
+      svc.create({ typeId: 'rbac_role', data: { name: 'admin' }, publishOnCreate: true }),
+    ).rejects.toThrow(/FOREIGN KEY constraint failed/i)
+
+    // Nothing was written — this is the "roles missing after boot" symptom.
+    const n = db.raw.prepare("SELECT COUNT(*) AS n FROM documents WHERE type_id = 'rbac_role'").get().n
+    expect(n).toBe(0)
+  })
+
+  it('the fixed ordering succeeds: register document types FIRST, then insert documents', async () => {
+    // PRODUCER first (bootstrap Phase A) …
+    await bootstrapDocumentTypes(db)
+    const typeCount = db.raw
+      .prepare("SELECT COUNT(*) AS n FROM document_types WHERE id IN ('rbac_role','plugin')").get().n
+    expect(typeCount).toBe(2)
+
+    // … then the CONSUMER insert (bootstrap Phase B) no longer violates the FK.
+    const svc = new DocumentsService(db, { tenantId: 'default' })
+    const doc = await svc.create({ typeId: 'rbac_role', data: { name: 'admin' }, publishOnCreate: true })
+    expect(doc.typeId).toBe('rbac_role')
+
+    const n = db.raw.prepare("SELECT COUNT(*) AS n FROM documents WHERE type_id = 'rbac_role'").get().n
+    expect(n).toBe(1)
+  })
+
+  it('registering all producer types up front lets many document inserts land under FK enforcement', async () => {
+    await bootstrapDocumentTypes(db)
+    const svc = new DocumentsService(db, { tenantId: 'default' })
+    // A spread of system types real consumers (RBAC seed, plugin bootstrap) write.
+    for (const [typeId, data] of [
+      ['rbac_role', { name: 'editor' }],
+      ['rbac_verb', { name: 'read' }],
+      ['plugin', { name: 'core-auth' }],
+    ]) {
+      const d = await svc.create({ typeId, data, publishOnCreate: true })
+      expect(d.typeId).toBe(typeId)
+    }
+    const total = db.raw
+      .prepare("SELECT COUNT(*) AS n FROM documents WHERE type_id IN ('rbac_role','rbac_verb','plugin')").get().n
+    expect(total).toBe(3)
+  })
+})
+
+// The three tests above prove the general DB premise (a consumer insert before its type
+// exists throws under FK enforcement, and registering types first fixes it) — they never
+// touch bootstrap.ts itself. This block calls the REAL bootstrapMiddleware end to end, so
+// it actually regresses if bootstrap.ts's Phase A/B split is ever collapsed back into one
+// Promise.all. Plugin bootstrap is disabled (config.plugins.disableAll) to keep the FK
+// dynamic isolated to RBAC seeding — the same Phase A→B wiring covers both, so this is
+// sufficient to prove the ordering without pulling in real plugin definitions.
+describe('bootstrapMiddleware (first-boot race regression, end to end)', () => {
+  let db
+  beforeEach(() => {
+    db = fkOnDb()
+    resetBootstrap()
+  })
+  afterEach(() => {
+    db.close()
+    resetBootstrap()
+  })
+
+  it('a single cold-start request seeds document types before RBAC documents that FK-reference them', async () => {
+    const app = new Hono()
+    app.use('*', bootstrapMiddleware({ plugins: { disableAll: true } }, []))
+    app.get('/', (c) => c.text('ok'))
+
+    const res = await app.request('/', {}, { DB: db })
+    expect(res.status).toBe(200)
+
+    // If Phase A (producers) didn't fully land before Phase B (RBAC seed, a consumer)
+    // ran, this would be 0 — either from a swallowed FK error, or because a partial
+    // bootstrap never got the chance to retry. A real fresh boot seeds 4 system roles
+    // (admin/editor/author/viewer) — see rbac.ts's SYSTEM_ROLES.
+    const roleCount = db.raw.prepare("SELECT COUNT(*) AS n FROM documents WHERE type_id = 'rbac_role'").get().n
+    expect(roleCount).toBeGreaterThan(0)
+
+    const verbCount = db.raw.prepare("SELECT COUNT(*) AS n FROM documents WHERE type_id = 'rbac_verb'").get().n
+    expect(verbCount).toBeGreaterThan(0)
+
+    // document_types themselves must exist too (Phase A actually ran, not skipped).
+    const typeCount = db.raw
+      .prepare("SELECT COUNT(*) AS n FROM document_types WHERE id IN ('rbac_role','rbac_verb')").get().n
+    expect(typeCount).toBe(2)
+  })
+})

packages/core/src/middleware/bootstrap.ts

@@ -21,6 +21,13 @@ type Bindings = {
 
 // Track if bootstrap has been run in this worker instance
 let bootstrapComplete = false;
+// Single-flight latch: the in-progress bootstrap promise for THIS isolate, or null.
+// `bootstrapComplete` only flips at the END of a successful run, so without this a
+// second request arriving on a cold isolate mid-bootstrap passes the completion/KV
+// checks and starts a SECOND concurrent seed — re-opening the document_types-vs-
+// documents FK window across the two runs and duplicating writes. Concurrent callers
+// await this instead.
+let bootstrapInFlight: Promise<void> | null = null;
 
 // KV key for cross-isolate bootstrap state. Version-keyed so a code deployment
 // (SONICJS_VERSION bump) automatically invalidates the cached flag and forces
@@ -211,6 +218,17 @@ export function bootstrapMiddleware(config: SonicJSConfig = {}, allPlugins?: Arr
     const gitBranch = (c.env as any).GIT_BRANCH as string | undefined;
     setBranchLabel(isLocalhost && gitBranch ? gitBranch : undefined);
 
+    // Single-flight: if another request on this cold isolate is already running the
+    // full bootstrap, await THAT run and proceed — never start a second concurrent
+    // seed. The check-and-set below is synchronous (no await between), so exactly one
+    // request wins the latch. Cleared in the `finally` at the end of the run.
+    if (bootstrapInFlight) {
+      await bootstrapInFlight;
+      return next();
+    }
+    let releaseInFlight: () => void = () => {};
+    bootstrapInFlight = new Promise<void>((resolve) => { releaseInFlight = resolve; });
+
     try {
       console.log("[Bootstrap] Starting system initialization...");
 
@@ -252,65 +270,87 @@ export function bootstrapMiddleware(config: SonicJSConfig = {}, allPlugins?: Arr
         console.error("[Bootstrap] Error populating collection registry:", error);
       }
 
-      // 3–4. Independent D1 operations — run in parallel to minimise cold-start latency.
-      // Each step has its own error handling so one failure doesn't block the others.
+      // 3–4. System-data seeding. Ordered in two phases because of a foreign-key
+      // dependency that a flat parallel batch violates on a fresh DB:
+      //   - PRODUCERS create `document_types` rows.
+      //   - CONSUMERS `INSERT INTO documents`, whose `type_id` FK-references those rows.
+      // Running all five in one Promise.all let a consumer insert win the race against
+      // its producer on a cold DB → `FOREIGN KEY constraint failed`, caught+swallowed
+      // per step, leaving RBAC roles / core plugins unseeded (see the header note).
       console.log("[Bootstrap] Registering document types and seeding system data...");
       const { RbacService } = await import("../services/rbac");
       const rbacService = new RbacService(c.env.DB, (c.env as any).CACHE_KV);
 
+      // Track step failures so we never cache a PARTIAL bootstrap as "done" (below).
+      let bootstrapOk = true;
+      const runStep = async (label: string, fn: () => Promise<unknown>) => {
+        try {
+          await fn();
+        } catch (e) {
+          bootstrapOk = false;
+          console.error(`[Bootstrap] Error ${label}:`, e);
+        }
+      };
+
+      // Phase A — PRODUCERS: register every document type FIRST and await them, so the
+      // `document_types` rows are committed before any `documents` insert references them.
+      await runStep("registering document types", () => bootstrapDocumentTypes(c.env.DB));
+      await runStep("auto-registering collection document types", async () => {
+        const auto = await autoRegisterCollectionDocumentTypes(c.env.DB);
+        if (auto.length) console.log(`[Bootstrap] Document-backed collections registered: ${auto.join(", ")}`);
+      });
+
+      // Phase B — CONSUMERS (+ the independent credential-account repair): the types now
+      // exist, so these can safely run in parallel to keep cold-start latency low.
       await Promise.all([
-        // 3. Register document types (idempotent)
-        bootstrapDocumentTypes(c.env.DB).catch((e) =>
-          console.error("[Bootstrap] Error registering document types:", e)
-        ),
-
-        // 3b. Make every content collection document-backed.
-        autoRegisterCollectionDocumentTypes(c.env.DB)
-          .then((auto) => {
-            if (auto.length) console.log(`[Bootstrap] Document-backed collections registered: ${auto.join(", ")}`)
-          })
-          .catch((e) => console.error("[Bootstrap] Error auto-registering collection document types:", e)),
-
-        // 2c. Repair legacy credential accounts.
-        repairMissingCredentialAccounts(c.env.DB).catch((e) =>
-          console.error("[Bootstrap] Error repairing credential accounts:", e)
-        ),
-
-        // 3a. Seed system RBAC roles/verbs/grants.
-        rbacService.ensureSystemRbacSeed().catch((e) =>
-          console.error("[Bootstrap] Error seeding RBAC documents:", e)
-        ),
-
-        // 4. Bootstrap core plugins.
-        config.plugins?.disableAll
-          ? Promise.resolve()
-          : (async () => {
-              const bootstrapService = new PluginBootstrapService(c.env.DB)
-              const needsBootstrap = await bootstrapService.isBootstrapNeeded()
-              if (needsBootstrap) {
-                console.log("[Bootstrap] Bootstrapping core plugins...")
-                await bootstrapService.bootstrapCorePlugins()
-              }
-            })().catch((e) => console.error("[Bootstrap] Error bootstrapping plugins:", e)),
-      ])
+        // Independent of document types (operates on auth `account` rows).
+        runStep("repairing credential accounts", () => repairMissingCredentialAccounts(c.env.DB)),
+
+        // Seeds system RBAC roles/verbs/grants as `documents` (FK → document_types).
+        runStep("seeding RBAC documents", () => rbacService.ensureSystemRbacSeed()),
+
+        // Bootstraps core plugins; installPlugin writes plugin `documents` (FK → document_types).
+        runStep("bootstrapping plugins", async () => {
+          if (config.plugins?.disableAll) return;
+          const bootstrapService = new PluginBootstrapService(c.env.DB);
+          if (await bootstrapService.isBootstrapNeeded()) {
+            console.log("[Bootstrap] Bootstrapping core plugins...");
+            await bootstrapService.bootstrapCorePlugins();
+          }
+        }),
+      ]);
 
       // Seed starter content after types are registered (idempotent no-op when present).
+      // Deliberately NOT run through runStep/bootstrapOk: this is a decorative demo post,
+      // not a security-critical step like RBAC or plugin seeding. Gating the completion
+      // latch on it would mean a persistent failure here (unlike a missing FK dependency,
+      // which can't happen — its type is registered unconditionally above) forces every
+      // subsequent request to redo the full ~10s bootstrap forever just to retry seeding
+      // one blog post. Logged on failure like before, but never blocks the latch.
       await bootstrapDefaultContent(c.env.DB).catch((e) =>
         console.error("[Bootstrap] Error seeding default content:", e)
-      )
+      );
 
-      // Mark bootstrap as complete for this worker instance and persist to KV
-      // so subsequent cold-start isolates can skip the D1 work entirely.
-      bootstrapComplete = true;
-      console.log("[Bootstrap] System initialization completed");
-      try {
-        const cacheKv = (c.env as any).CACHE_KV as KVNamespace | undefined
-        if (cacheKv) {
-          // 24h TTL — long enough that hot instances never re-bootstrap, short
-          // enough that a DB reset auto-heals within a day.
-          await cacheKv.put(BOOTSTRAP_KV_KEY(), '1', { expirationTtl: 86400 })
-        }
-      } catch { /* KV write failure is non-fatal */ }
+      // Mark bootstrap complete ONLY when every step succeeded. A partial bootstrap must
+      // not latch the in-memory flag or persist the KV skip marker: doing so would cache
+      // the broken state for the isolate's lifetime AND for the KV TTL (24h), so every
+      // later request/isolate skips the D1 work while roles/plugins stay missing. Leaving
+      // both unset lets the next request retry the (idempotent) bootstrap and converge
+      // once the transient condition clears.
+      if (bootstrapOk) {
+        bootstrapComplete = true;
+        console.log("[Bootstrap] System initialization completed");
+        try {
+          const cacheKv = (c.env as any).CACHE_KV as KVNamespace | undefined
+          if (cacheKv) {
+            // 24h TTL — long enough that hot instances never re-bootstrap, short
+            // enough that a DB reset auto-heals within a day.
+            await cacheKv.put(BOOTSTRAP_KV_KEY(), '1', { expirationTtl: 86400 })
+          }
+        } catch { /* KV write failure is non-fatal */ }
+      } else {
+        console.warn("[Bootstrap] System initialization completed WITH ERRORS — not caching the skip marker; the next request will retry.");
+      }
 
       // Fire project snapshot telemetry (fire-and-forget, never blocks boot)
       try {
@@ -378,6 +418,11 @@ export function bootstrapMiddleware(config: SonicJSConfig = {}, allPlugins?: Arr
     } catch (error) {
       console.error("[Bootstrap] Error during system initialization:", error);
       // Don't prevent the app from starting, but log the error
+    } finally {
+      // Release the single-flight latch so a later request can retry if this run
+      // did not mark bootstrap complete (transient failure self-heal).
+      bootstrapInFlight = null;
+      releaseInFlight();
     }
 
     // 4. Verify security configuration (outside try/catch so critical
@@ -393,6 +438,7 @@ export function bootstrapMiddleware(config: SonicJSConfig = {}, allPlugins?: Arr
  */
 export function resetBootstrap() {
   bootstrapComplete = false;
+  bootstrapInFlight = null;
 }
 
 /**