Regenerate latest snapshot + validator check (DoD #3 of wxyc-shared#82)#595
Merged
Conversation
…red#82) Snapshot files in meta/ ended at 0056_snapshot.json — twelve migrations between 0057 and 0067 shipped by hand-editing the SQL + journal without running drizzle:generate, so each one's snapshot was never emitted. Cumulatively, drizzle:generate against current schema produced a 30-line catch-up migration containing operations the database already had, which broke any contributor's first attempt at adding a real new migration. Three changes: 1. shared/database/src/migrations/meta/0068_snapshot.json — single catch-up snapshot reflecting current schema state (drizzle:generate output rewritten as the latest snapshot, prevId pointing to 0056's id). Verified: a second drizzle:generate against a fresh DB now reports "No schema changes, nothing to migrate". 2. scripts/validate-migrations.mjs grows Check 7: every non-allowlisted journal entry must have a matching snapshot file. The allowlist HISTORICAL_MISSING_SNAPSHOT_IDXS = {36, 41, 47-54, 57-67} captures the historical gap and explicitly must not grow — any future hand- edit pattern fails CI with a clear message pointing at `npm run drizzle:generate`. 3. CLAUDE.md "Migration workflow" gains a paragraph explaining the two parallel meta/ artifacts (journal vs per-idx snapshots), why hand-editing the journal accumulates rot, and the validator's enforcement. Two new unit tests in tests/unit/scripts/validate-migrations.test.ts: - "flags a journal entry with no matching snapshot file (post-#590 hand-edit canary)": adds an entry at idx 9000 (outside any allowlist), no snapshot, asserts Check 7 fires with the drizzle:generate hint. - "tolerates the historically-missing 0057-0067 snapshot gap": pins the allowlist tolerance against the current repo state so future allowlist edits can't silently promote the gap to an error. The pre-existing "flags a broken prevId chain" test was hard-coded to 0055_snapshot.json (then the latest); generalised to walk the meta directory and pick whichever idx is current head, so the test stays correct as more migrations land. Closes #590. Unblocks DoD #3 of WXYC/wxyc-shared#82.
Self-review on PR #595 flagged that the "allowlist must not grow" narrative was aspirational comment, not enforced behavior — a contributor hand-editing the journal could quietly add their idx to the allowlist to make CI pass. This test parses the validator source, reads the literal Set contents, and asserts the actual set is a subset of the expected post-#590 set. Future shrinkage (snapshot backfilled for one of the allowlisted idxs) is allowed; growth fails this test, forcing an explicit edit-and-justify pass through review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #590. Ticks DoD #3 of WXYC/wxyc-shared#82 (`drizzle:generate` produces a clean diff against current schema state without hand-patching `meta/`).
The `meta/` directory had snapshots for migrations through 0056 and then a 12-idx gap before today's catch-up. Twelve migrations between 0057 and 0067 (and the recovery commits earlier this session) shipped by hand-editing the SQL + `_journal.json` without running `drizzle-kit generate`, so the snapshot side of drizzle's metadata never advanced. Running `drizzle:generate` against current schema produced a 30-line catch-up file containing operations the database already had — which broke any contributor's first attempt at a real new migration.
What this PR does
`shared/database/src/migrations/meta/0068_snapshot.json` (new) — a single catch-up snapshot reflecting current schema, generated by `drizzle-kit generate` against a fresh DB with all 67 migrations applied, with the emitted SQL discarded and the snapshot renamed to pair with the existing latest migration (0068). Its `prevId` chain points at 0056_snapshot.json's id, so the validator's existing chain walker reaches genesis through the 21-idx historical gap.
`scripts/validate-migrations.mjs` Check 7 + `HISTORICAL_MISSING_SNAPSHOT_IDXS` allowlist — every non-allowlisted journal entry must have a matching snapshot file. Allowlist contains the 21 historical idxs (36, 41, 47–54 from Repair Drizzle migrations metadata (missing snapshots, broken prevId chain) #505, 57–67 from this session). Going forward the next hand-edit-without-generate pattern fails CI with:
```
ERROR: Missing snapshot for journal entry idx N (XXX): expected meta/000N_snapshot.json. Run `npm run drizzle:generate` so drizzle-kit emits the snapshot instead of hand-editing the journal.
```
`CLAUDE.md` "Migration workflow" gets a paragraph explaining the two parallel `meta/` artifacts and why hand-editing the journal accumulates rot.
Verification
New unit tests
The pre-existing `flags a broken prevId chain` test was hard-coded to `0055_snapshot.json` (the head at the time it was written); generalised to walk `meta/` and pick whichever snapshot is currently the head, so the test stays correct as more migrations land.
What's not covered (acknowledged gap)
Snapshot freshness against schema.ts. Check 7 enforces existence of a snapshot per journal entry, not staleness of the snapshot's content. A contributor who edits an existing migration's SQL or schema.ts without re-running `drizzle:generate` would not be caught by the validator — the snapshot would be silently out of date.
The runtime safety nets that catch this are (a) `drizzle:migrate` against an integration DB at apply time, since stale snapshots typically result in schema mismatches the next time anything queries an affected table, and (b) the dev-workflow `drizzle:generate` would surface the divergence as a non-empty diff.
A CI-gated freshness check would need to run `drizzle-kit generate` against schema.ts and assert no diff — that requires a live PG and isn't a unit-test concern. Filing this as a separate follow-up if the existence-only check ever proves insufficient.
Deliberately out of scope
Why a single snapshot rather than backfilling 0057–0067 individually
Drizzle-kit's `generate` picks the latest snapshot by filename sort and diffs against it; only the latest snapshot affects `generate`'s output. The intermediate snapshots only matter for `drizzle-kit drop` (rarely used) and historical archaeology. Backfilling all 12 individually would require 12 round-trips with a custom procedure (drizzle-kit doesn't expose a "regenerate snapshot N" command — `generate` always emits the latest), and the cost/value didn't pencil. The allowlist documents the gap explicitly.
Part of WXYC/wxyc-shared#82 Phase 4. Related: #505 (predecessor; closed prematurely with the same gap unfixed) and #511 (sibling — runtime hardening; this PR is metadata hygiene).