Skip to content

fix(core): repair ~25 unit-test failures from the beta.25 cold-start caches - #1018

Open
mmcintosh wants to merge 1 commit into
mainfrom
fix/beta25-test-infra
Open

fix(core): repair ~25 unit-test failures from the beta.25 cold-start caches#1018
mmcintosh wants to merge 1 commit into
mainfrom
fix/beta25-test-infra

Conversation

@mmcintosh

Copy link
Copy Markdown
Collaborator

Description

Three per-isolate caches / context assumptions introduced by the beta.25 cold-start work behave correctly in a live Workers isolate but break outside one — which is why 25 unit tests currently fail on a clean main checkout (npm test in packages/core). Each fix preserves the production optimization while making it correct across databases / runtimes. No behavior change on Workers.

Fixes #1016

Sequencing (PR 1 of 4)

This is the shared prerequisite for a 4-PR set — merge this first. The other three each re-ground onto this one; after it lands they touch disjoint files and can merge in any order.

  1. PR 1 — fix(core): beta.25 test-infra (this PR) — repairs the ~25 pre-existing unit-test failures; the shared prerequisite.
  2. PR 2 — feat: views plugin — re-grounds onto PR 1.
  3. PR 3 — fix(bootstrap): first-boot FK race + single-flight — touches only bootstrap.ts; parallel after PR 1. (Its red CI before PR 1 lands is exactly the ~25 failures this PR fixes — none of them touch bootstrap.ts.)
  4. PR 4 — feat: forms document-model port — re-grounds onto PR 1.

Changes

  • document-scalar-schema.ts — the module-level PRAGMA column/index cache leaked across databases. In tests, the 2nd in-memory DB "already had" its q_* generated columns per the cache, the ALTERs were skipped, and every generated-column read failed with no such column: q_* (16 failures across 5 real-SQLite suites). Now a WeakMap keyed by the D1 binding: one bootstrap reuses one env.DB, so the intended per-isolate dedup is fully preserved, while a different database gets its own snapshot.
  • plugin-middleware.ts — same class: isPluginActive's status cache was keyed by pluginId only, bleeding one mock DB's active into the next test's DB (7 failures). Now WeakMap-per-DB + a generation stamp for invalidatePluginStatusCache (a WeakMap can't be enumerated, so a bump lazily discards every DB's map — activate/deactivate is rare, the refill is one read per plugin).
  • api.ts + catalog.ts — Hono's c.executionCtx getter throws when there's no ExecutionContext (vitest's app.request()), 500-ing the cached-collection read path (2 failures). Added a safeExecutionCtx() try/catch helper at all 9 call sites; scheduleKvWrite now tolerates an undefined ctx (the KV catalog warm is best-effort background work, never load-bearing).

Testing

Unit Tests

  • Added/updated unit tests
  • All unit tests passing

Verified on a branch off main containing ONLY these 4 files: tsc clean; the previously-failing test files now pass (6 files / 77 tests); full suite 1715 passed / 0 failed / 247 skipped (≈25 were failing on pure main). No bootstrap.ts / first-boot-sequence changes — this is test-harness isolation + a context guard, not a bootstrap concurrency change.

E2E Tests

  • Added/updated E2E tests
  • All E2E tests passing

Screenshots/Videos

Checklist

  • Code follows project conventions
  • Tests added/updated and passing
  • Type checking passes
  • No console errors or warnings
  • Documentation updated (if needed)

…aches

Three per-isolate cache / context assumptions from the beta.25 cold-start work behave
correctly in a live Workers isolate but break outside one — which is why 25 unit tests fail
on a clean checkout (npm test in packages/core). Each fix preserves the production
optimization while making it correct across databases / runtimes. No behavior change on Workers.

- document-scalar-schema.ts: the module-level PRAGMA column/index cache leaked across
  databases. In tests the 2nd in-memory DB "already had" its q_* generated columns per the
  cache, the ALTERs were skipped, and every generated-column read failed with "no such
  column: q_*" (16 failures across 5 real-SQLite suites). Now a WeakMap keyed by the D1
  binding: one bootstrap reuses one env.DB, so the intended per-isolate dedup is fully
  preserved, while a different database gets its own snapshot.
- plugin-middleware.ts: same class — isPluginActive's status cache was keyed by pluginId
  only, bleeding one mock DB's "active" into the next test's DB (7 failures). Now WeakMap-per-DB
  + a generation stamp for invalidatePluginStatusCache (a WeakMap can't be enumerated, so a
  bump lazily discards every DB's map — activate/deactivate is rare, the refill is one read
  per plugin).
- api.ts + catalog.ts: Hono's c.executionCtx getter throws when there's no ExecutionContext
  (vitest app.request), 500-ing the cached-collection read path (2 failures). safeExecutionCtx()
  try/catch helper at all 9 call sites; scheduleKvWrite tolerates an undefined ctx (the KV
  catalog warm is best-effort background work, never load-bearing).

Does NOT touch bootstrap.ts or the first-boot bootstrap sequence — this is test-harness
isolation + a context guard, not a bootstrap concurrency change.
@mmcintosh mmcintosh self-assigned this Jul 20, 2026
@mmcintosh
mmcintosh marked this pull request as ready for review July 20, 2026 16:22
@mmcintosh
mmcintosh requested a review from lane711 as a code owner July 20, 2026 16:22
@mmcintosh

Copy link
Copy Markdown
Collaborator Author

Note on the red CI. The only failing check is the E2E against preview job, and every failure is the same one: loginAsAdmin times out because the admin signs in but cannot reach /admin. This PR does no touch any auth or bootstrap code — it only fixes per-database test caches and an executionCtx guard. The cause is a separate, pre-existing first-boot bug: on a fresh preview database, bootstrap races a foreign-key dependency and silently leaves the RBAC roles and core plugins unseeded, so the admin gets a 403. That is fixed by the bootstrap PR (#1019, Fixes #1017), not here. This PR's own unit suite — the roughly 25 failures it fixes (#1016) — is green. The two are a pair: this PR fixes the bootstrap PR's unit tests, and the bootstrap PR fixes this PR's E2E preview. Recommend reviewing on the green unit suite; the E2E clears automatically once #1019 lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

beta.25 cold-start caches break outside a Workers isolate — ~25 core unit tests fail on a clean checkout

1 participant