diff --git a/packages/core/src/__tests__/middleware/bootstrap-fk-ordering.test.ts b/packages/core/src/__tests__/middleware/bootstrap-fk-ordering.test.ts new file mode 100644 index 000000000..436091acd --- /dev/null +++ b/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) + }) +}) diff --git a/packages/core/src/middleware/bootstrap.ts b/packages/core/src/middleware/bootstrap.ts index 4c08d6109..f4195bb8c 100644 --- a/packages/core/src/middleware/bootstrap.ts +++ b/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 | 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((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) => { + 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; } /**