fix(bootstrap): first-boot FK race + single-flight - #1019
Conversation
…rs + single-flight guard Fixes the greenfield first-boot race (two coupled facets), observed live on a fresh D1 + empty KV on beta.25: FACET 1 — intra-bootstrap FK ordering. The system-data seed ran all five steps in one Promise.all, but two of them (RBAC seed, core-plugin bootstrap) INSERT INTO documents whose type_id FK-references document_types rows that the other two steps (bootstrapDocumentTypes, autoRegisterCollectionDocumentTypes) are still creating in parallel. A consumer insert winning the race throws FOREIGN KEY constraint failed; the per-step .catch() swallows it, so RBAC roles/verbs and core plugins are silently left unseeded (admin routes then 403, plugins inactive). FACET 2 — no single-flight guard. bootstrapComplete only flips at the END of a run, so two concurrent requests on the same cold isolate both pass the completion/KV checks and start SECOND parallel seed — re-opening the FK window across the two runs. Compounding both: the KV skip-marker was written UNCONDITIONALLY after the batch (24h TTL), latching the partial state so every later request/isolate skips D1 and the broken bootstrap persists across restarts until the marker expires or is cleared. Fix (bootstrap.ts): - Two phases. Phase A awaits the document-type PRODUCERS first; Phase B runs the CONSUMERS + independent credential repair in parallel (latency preserved). - Single-flight in-flight promise: a concurrent request on a cold isolate awaits the running bootstrap instead of starting a second one. Check-and-set is synchronous (atomic), released in finally. - runStep() records failures; the completion latch + KV marker are written ONLY when every step succeeded, so a transient failure self-heals on the next request. Regression test (bootstrap-fk-ordering.test.ts): real SQLite, foreign_keys=ON (the shared harness disables them; here the FK is the subject). Proves insert-before-type throws FK + writes nothing, and that registering types first lets consumer inserts land. Reality-tested on fresh greenfield boots (clean D1 + empty KV) against beta.25: - single first request → clean boot, 0 FK errors, 21 types / 4 roles / 6 verbs / 4 plugins. - 4 CONCURRENT cold-isolate requests → bootstrap body runs exactly ONCE, completes clean, 0 FK errors (proves the single-flight guard). Before the fix the same first boot always FK-failed and required a KV-clear + reboot. Pre-existing: full suite shows 25 failures on this branch, identical to pristine origin/main; none touch bootstrap. They are the separately-owned beta.25 unit-suite repair (per-DB cache + executionCtx). Green CI requires that repair to land first. Coordination: touches ONLY bootstrap.ts + its test — no overlap with the beta.25 repair (plugin-middleware.ts, document-scalar-schema.ts, api.ts, catalog.ts).
|
Note on the red CI. The failing check here is the unit-test step (about 25 tests), and those failures are pre-existing on main — this branch is cut straight from current main, and none of them touch bootstrap.ts. They are the known beta.25 test-harness issue (a per-database cache leaks generated columns across in-memory test DBs, producing "no such column: q_*" errors) and are fixed by the test-infra PR (#1018, Fixes #1016). Type-check passes here, and this PR's purpose is to fix the E2E against preview failure: the loginAsAdmin timeout caused by the first-boot FK race that leaves the RBAC roles and core plugins unseeded (#1017). The two are a pair: #1018 fixes this PR's unit suite, and this PR fixes #1018's E2E preview. Recommend landing #1018 first, then this one — after both, CI is green across the stack. |
fix(bootstrap): document-type seeding must precede its FK consumers on a fresh DB
Fixes #1017
TL;DR
On a fresh database's first boot,
bootstrapMiddlewareseeds system data with aflat
Promise.allthat races a foreign-key dependency: two steps createdocument_typesrows while two other steps insertdocumentsthat FK-referencethose rows. When a consumer insert wins the race, D1 throws
FOREIGN KEY constraint failed; the per-step.catch()swallows it, so RBAC roles/verbs and every coreplugin are silently left unseeded. A second facet compounds it: there is no
single-flight guard, so two concurrent requests on a cold isolate both enter bootstrap
and seed in parallel — re-opening the same FK window across the two runs. The bootstrap
then writes an unconditional KV skip-marker (24h TTL), which latches the broken
state: every subsequent request/isolate takes the KV fast-path, skips D1 entirely,
and the site runs with no roles and no plugins until the marker expires or is cleared.
This PR (1) makes the two document-type producers run and finish before the consumer
inserts, (2) adds a single-flight in-flight promise so concurrent cold-isolate requests
await one bootstrap instead of racing parallel ones, and (3) makes the "bootstrap
complete" latch + KV marker conditional on success. It's a surgical change to
middleware/bootstrap.tsplus a real-DB regression test. Two files, +166/−49.Sequencing — this is PR 3 of a coordinated set of 4
These four PRs were split from overlapping work so each is single-concern. Land PR-1
first; after that the remaining three touch disjoint files and may land in any order.
fix(core): beta.25 test-infra(per-DB caches +executionCtxguard).Prerequisite — merge first. Fixes ~25 unit failures present on current
main.feat: views plugin port. Re-grounded onto PR-1.fix(bootstrap): first-boot FK race + single-flight. Touchesonly
middleware/bootstrap.ts(+ its test); already independent of the PR-1 files.feat: forms document-model port. Re-grounds onto PR-1 + PR-3.Why this PR shows failing unit tests until PR-1 lands: this branch is cut from
current
main, sonpm testreports the same ~25 pre-existing failures as pristinemain— in 7 files, none of which importbootstrap.ts. They are fixed by PR-1.This PR's own regression test and
tscare green; the FK-ordering + single-flightbehavior is proven by the tests and reality boots below.
Relationship to PR-4 (forms): not code-coupled — a
formdocument type is aproducer in the seed order, not an FK-race victim, so forms works on an already-booted
DB regardless. But a greenfield deploy wants this PR in first, or the very first boot
still leaves RBAC roles / plugins unseeded (admin 403s).
How we hit it (real, not theoretical)
We were spinning up a clean local greenfield stack (fresh D1, empty KV) on
beta.25to validate an incoming feature branch. The very first request logged:Two things stood out:
await Promise.all([...])inbootstrap.ts(at async Promise.all (index 4)), and the failures are exactlythe steps that write
documentsrows (RBAC seed at index 3, plugin bootstrap atindex 4). The steps that create
document_typesare indices 0–1 of the samearray.
admin routes returned
403 Insufficient permissions(no RBAC roles), and coreplugins were inactive. Restarting the worker didn't help. Only clearing the KV
bootstrap marker (
rm -rf .wrangler/state/v3/kvlocally) and re-booting fixed it —because by the second boot the
document_typesrows happened to already exist, sothe race didn't reproduce.
That second-boot-heals-it behavior is the tell: the bug is a write-ordering race on
a cold DB, and the KV marker turns a transient race into a sticky outage.
Root cause
documents.type_idisNOT NULL REFERENCES document_types(id)(
migrations/0002_documents.sql:30). The seed phase inbootstrap.tsruns fivesteps in one
Promise.all:bootstrapDocumentTypesdocument_typesrowsautoRegisterCollectionDocumentTypesdocument_typesrowsrepairMissingCredentialAccountsaccounttable)rbacService.ensureSystemRbacSeedINSERT INTO documents(rbac_role/verb/grant), FK → document_typesPluginBootstrapService.bootstrapCorePluginsinstallPlugin→INSERT INTO documents, FK → document_typesBecause all five are launched concurrently, on a cold DB a consumer's
documentsinsert can execute before the producer has committed the matching
document_typesrow →
FOREIGN KEY constraint failed. D1 enforces this FK; local SQLite in the testharness disables it (which is why unit tests never caught it — see Testing).
Second facet: no single-flight guard
bootstrapComplete(module-level) only flips totrueat the end of a successfulrun. On a cold isolate, two requests that arrive close together both read
bootstrapComplete === false, both miss the KV marker (not written yet), and both runthe full seed concurrently. Even with the ordering fix applied per-run, two
concurrent runs can interleave (run A creating types while run B inserts documents) and
duplicate every seed write. There is no promise that a second caller can await.
Third defect: the marker latches failures
After the batch:
Both are written even when steps in the batch failed. The KV marker is the cold-start
fast-path (
if (kvDone === '1') { … return next() }near the top of the middleware),so once it's set, no isolate re-runs the D1 seed for 24h — the partial bootstrap
is cached as if it were complete.
Who this affects
Any environment whose first-ever request lands on a fresh, unseeded database:
.wranglerstate.Warm/existing databases are unaffected (the
document_typesalready exist, so therace can't lose). That's precisely why it's easy to miss in day-to-day dev against an
already-bootstrapped DB, and why it bites new installs specifically.
The fix
packages/core/src/middleware/bootstrap.ts— the seed phase becomes two phases:bootstrapDocumentTypesthenautoRegisterCollectionDocumentTypes— so alldocument_typesrows are committed before anydocumentsinsert references them.These are two fast, idempotent registrations; awaiting them adds negligible latency.
plus the independent credential-account repair, kept in a
Promise.allsocold-start latency is preserved now that the FK precondition holds.
Single-flight guard. A module-level
bootstrapInFlight: Promise<void> | nullholdsthe running bootstrap. A request that arrives mid-bootstrap awaits it and proceeds
rather than starting a second seed. The check-and-set is synchronous (no
awaitbetween the
if (bootstrapInFlight)test and the assignment), so exactly one requestwins the latch; it is released in a
finally. This closes the concurrency facet andalso removes duplicate cold-start work.
A small
runStep(label, fn)wrapper records whether any step threw. ThebootstrapCompletelatch and the KV skip-marker are now written only when everystep succeeded. If something still fails transiently (a D1 hiccup, say), nothing is
cached, and the next request retries the idempotent bootstrap and converges — instead
of locking a broken state for 24h.
Net behavior on the happy path is unchanged: types get registered, consumers seed,
the marker is written. The only difference is ordering (so the FK holds) and that a
failed boot no longer poisons the cache.
Why not just widen the KV TTL / bump the version key
c99a5673("bump version to beta.25 — invalidates KV bootstrap key") already showedthe marker can cache a bad state — but a version bump only busts the key once per
release; the next fresh DB re-enters the same race and re-latches. The self-host path
633023e4("entrypoint-based seed prevents concurrent SQLite corruption") applies theright idea — seed sequentially before serving — but only to the Docker/SQLite runtime.
This PR brings that ordering guarantee to the Workers/D1 cold-start path, which is
where new cloud installs boot.
Testing
packages/core/src/__tests__/middleware/bootstrap-fk-ordering.test.ts(new, realSQLite):
d1-sqliteharness disables foreign keys (D1 doesn't reliably enforcethem and the services delete derived rows explicitly). Here the FK is the
subject, so the test re-enables
foreign_keys = ONon the DB under test — matchingwhat production D1 enforced when it threw.
documentsinsert (via the realDocumentsService.create) for a type that isn't registered yet rejects withFOREIGN KEY constraint failedand writes nothing — the "roles missing after boot"symptom.
bootstrapDocumentTypesruns (Phase A), the same consumerinserts land cleanly; a spread of real system types (
rbac_role,rbac_verb,plugin) all persist under FK enforcement.Reality-tested end-to-end on fresh greenfield boots (clean D1 + empty KV) against the
beta.25runtime:document types, 4 RBAC roles, 6 verbs, 4 plugins) — no KV-clear-and-reboot needed.
(
Starting system initializationlogged once), completes cleanly, zero FK errors— proving the single-flight guard.
Before the fix, the same first boot always FK-failed on
Installing plugin: Authentication System+ RBAC seed and required a KV-clear + second boot to recover.Gates:
tsc --noEmit0 errors. The full unit suite shows 25 pre-existing failures onthis branch identical to pristine
origin/main(verified: same 7 files, none ofwhich import
bootstrap.ts); they are the separately-flagged beta.25 unit-suitebreakage (per-DB cache +
executionCtxguard) and are fixed by that repair. Green CIfor this PR requires that repair to land first.
Scope / not in scope
633023e4) is unchanged.