chore(db): add schema-vs-migrations drift guard - #2674
Merged
Conversation
src/db/schema.ts is a single shared file where two independently-valid PRs can each add a column to the same table. The existing migration column-collision check (detectColumnCollisions, #2551) only catches two DIFFERENT migration files adding the same column -- it never reads schema.ts, so schema.ts's declared shape can silently drift from what migrations/ actually produces when replayed against a fresh DB, with no CI signal. Adds scripts/check-schema-drift.mjs: replays migrations/*.sql into a fresh in-memory node:sqlite DB (mirroring test/helpers/d1.ts's TestD1Database), introspects each table's real columns via PRAGMA table_info, and diffs that against schema.ts's declared columns via drizzle-orm's getTableColumns. A RAW_SQL_ONLY_TABLES allowlist covers the 20 feature/aggregate tables that intentionally exist only in migrations/ (accessed via raw SQL, per the documented house pattern). Wired into db:schema-drift:check, the test:ci chain next to db:migrations:check, and a new CI step alongside "Check migrations" (CI runs discrete steps rather than test:ci as a whole, so a step was required for this to actually run on PRs). Closes #2565
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2674 +/- ##
=======================================
Coverage 96.10% 96.10%
=======================================
Files 237 237
Lines 26538 26538
Branches 9624 9624
=======================================
Hits 25505 25505
Misses 424 424
Partials 609 609 🚀 New features to boost your workflow:
|
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
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
src/db/schema.ts(Drizzle ORMsqliteTabledeclarations) is a single shared file where two independently-valid PRs can each add a column to the same table. The existing migration column-collision check (detectColumnCollisionsinsrc/db/migration-column-extraction.ts, added under feat(gate): detect same-table/same-column collisions across differently-numbered migrations #2551 / shipped in feat(gate): detect same-table/same-column collisions across differently-numbered migrations #2607'sscripts/check-migrations.mjs) only catches two different migration files adding the same(table, column)pair — it never readssrc/db/schema.tsat all (confirmed via grep: zero references). It cannot see schema.ts's declared shape silently drifting from whatmigrations/actually produces when replayed against a fresh DB.scripts/check-schema-drift.mjs: replays everymigrations/*.sqlfile into a fresh in-memorynode:sqliteDB (mirrorstest/helpers/d1.ts'sTestD1Databaseconcatenate-sorted-files-then-exec approach), introspects each table's real columns viaPRAGMA table_info, importssrc/db/schema.ts's exported tables and extracts their declared columns viadrizzle-orm'sgetTableColumns, and diffs the two column-name sets per table.RAW_SQL_ONLY_TABLESallowlist (20 entries, e.g.review_audit,system_flags,orb_signals,tunables_overrides) covers the feature/aggregate tables that intentionally exist only inmigrations/and are accessed via raw SQL (env.DB.prepare(...)) rather than a Drizzle declaration — the documented house pattern ("core tables use Drizzle; feature/aggregate tables use raw-SQL migrations"). Each entry was confirmed by direct inspection to be actively read/written via raw SQL elsewhere insrc/.db:schema-drift:checknpm script, added to thetest:cichain immediately afterdb:migrations:check, and added as a new CI step in.github/workflows/ci.ymlright after "Check migrations" (gated by the samebackendpath filter). CI runs each check as a discrete named step rather than invokingnpm run test:cias a whole, so a workflow step was required for this to actually execute on PRs — verifiednpm run test:ciis never invoked from any.github/workflows/*.yml.src/db/schema.ts+migrations/and asserts zero mismatches — proving they are not already drifted today (they aren't; the check passes cleanly against the current 59 Drizzle tables + 20 allowlisted raw-SQL tables = 79 total migrated tables).Closes #2565
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;scripts/**is excluded from Codecov'sinclude, so this change owes no patch coverage, buttest/unit/check-schema-drift-script.test.tsstill covers every branch (column missing from migrations, column missing from schema.ts, table missing from migrations, undeclared migration-only table both flagged and allowlisted, non-table export skip, clean pass, CLI exit code + stderr text, and the real-repo regression guard).npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderate— 0 vulnerabilitiesnpm run test:ci(all steps, unsharded) — green: 354 test files / 6746 tests passed, 6 pre-existing skipsIf any required check was skipped, explain why:
Safety
UI Evidencesection below with screenshots. (N/A — this is a CI/dev-tooling-only change with no UI surface.)UI Evidence
N/A — no UI/frontend/docs-visible change.
Notes
.claude/skills/contributing-to-gittensory/reference.md's CI-checks table. That file is not inwantedPathsand is local maintainer tooling documentation, not part of this issue's described deliverables.