fix(db): journal-consistency guard for migrations (RBR-968 / RBR-927 AC3) — fork retarget (RBR-1033/RBR-1089) - #14
Merged
Merged
Conversation
… (RBR-927) An orphaned .sql file (present in packages/db/src/migrations, absent from meta/_journal.json) is invisible to drizzle's journal-driven migrator but IS visible to the folder-enumerating inspectMigrations. The database is genuinely up to date while bootstrap reports the file as permanently pending, so applyPendingMigrations threw "Failed to bootstrap migrations" with no repair path — neither reconcilePendingMigrationHistory nor applyPendingMigrationsManually can fix it, because re-running the orphan's DDL collides with the columns the journalled copy already created. Because that throw happens inside beforeAll, every test in an embedded-Postgres suite reported `skipped` rather than `failed` — a silent-green failure mode. This adds migration-journal-consistency.ts as the single source of truth for the invariant, wired into both enforcement points so they fail identically with the offending filename: - check-migration-numbering.ts (the `check:migrations` gate, which already guards build/typecheck/generate/migrate) now runs the journal/folder set comparison *first*, so an orphan is reported by name rather than as a confusing duplicate-slot or bad-prefix numbering complaint. - applyPendingMigrations() asserts consistency before inspecting state, so the runtime path fails fast and actionably instead of bottoming out in an unfixable pending-migration loop. Detects both directions: orphaned files and journal entries with no file. Verified: `pnpm check:migrations` passes clean; re-injecting 0128_force_reassign.sql makes it exit 1 naming the file; 11 unit tests cover both directions plus the exact RBR-927 renumber shape, and one test pins the real shipped migrations folder against its journal.
… AC3) An orphaned .sql in packages/db/src/migrations is invisible to drizzle's journal-driven migrate() but IS counted as pending by inspectMigrations. The result is a database that is genuinely up to date while bootstrap reports it as permanently pending -- and because that throw happens inside beforeAll, Vitest reports the whole suite as `skipped` rather than `failed`. Silent green. Checks (all name the offending filename): 1. file <-> journal bijection -- always a hard error 2. journal `idx` uniqueness -- hard error, minus an explicit ratcheting baseline for already-shipped defects 3. journal `idx` continuity -- warning (gaps usually mean a dropped migration) Enforcement is deliberately split by blast radius: - assertMigrationJournalConsistency() runs in applyPendingMigrations, i.e. every embedded-Postgres beforeAll. Bijection only -- it must never start failing on shipped journal history or it would brick production startup. - auditMigrationJournal() is the pre-build/pre-test gate and adds idx checks. Wired at three points so a defect cannot reach a test lane: - packages/db check:migrations (already gates build/typecheck/generate/migrate) - scripts/run-vitest-stable.mjs preflight, before any suite starts - PR `policy` job, which has no pnpm install -- hence the dependency-free scripts/check-migration-journal.mjs reimplementation. An anti-drift test asserts the .mjs and .ts guards agree on the real tree. Also fixes the .gitignore rule that caused this work to be lost once already: `check-*.mjs` on line 23 silently swallowed scripts/__tests__/check-migration-journal.test.mjs, so the guard's own test file was never stageable and the CI step referencing it would have failed. Known pre-existing defects on origin/master, now reported by name rather than fixed (out of scope per the issue): duplicate idx 178 shared by 0177_activity_log_responsible_user and 0178_summary_slots; missing idx 126, 130, 177.
…l (RBR-1033) Fork's migration journal diverged from upstream before upstream's known defects (duplicate idx 178, gaps at 126/130/177) existed on this line of history, so the fork's journal has neither. Update the pinned assertions and baseline to match the fork's actual (clean) journal state instead of upstream's, so the guard still exercises its real behavior (orphan/missing file detection, --strict gating) without asserting defects this fork never had.
PraeSynBH
pushed a commit
that referenced
this pull request
Aug 8, 2026
…not undefined scriptsDir runMigrationJournalGuard() in run-vitest-stable.mjs referenced an undefined 'scriptsDir' variable, throwing a ReferenceError before any suite could run. This is caught live on fork PR #14's CI: every General tests shard, every Verify serialized server suites shard, review, and verify all failed with the same trace. The guard script itself and its dedicated unit test are correct; only this call site had the wrong identifier. Use the already-defined repoRoot instead. Verified: general-server shard 1/30 (8 suites, 38 tests) now runs to completion under the fixed preflight; migration-journal-consistency.test.ts (33/33) and check-migration-journal.test.mjs (8/8) unaffected.
PraeSynBH
added a commit
that referenced
this pull request
Aug 8, 2026
…flight — fork retarget (RBR-1089) The RBR-1033 fork-retarget merge (491c36f) added a preflight call to check-migration-journal.mjs via an undefined scriptsDir variable. Every test:run invocation that reaches runMigrationJournalGuard() throws ReferenceError: scriptsDir is not defined before any suite starts — verified failing on fork/master CI for PR #14 and #16 (General tests, Verify serialized server suites, verify gate). The rest of the file already resolves script/repo paths off repoRoot (process.cwd()); use the same pattern here. Verified locally: node scripts/check-migration-journal.mjs -> OK, 128/128 node scripts/run-vitest-stable.mjs --dry-run -> no crash, full plan Co-authored-by: CTO Agent <cto-agent@paperclip.local>
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
3 participants
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.
Retarget of RBR-1033 (originally PR paperclipai#11012 against paperclipai/paperclip upstream, where we have push:false/admin:false/maintain:false and cannot merge) onto our fork PraeSynBH/paperclip, where we have admin:true/push:true. Applies the RBR-1081 repo-governance pattern (RBR-1089).
Cherry-picked cleanly from 4f18bd3 + d110393 (rbr-968-journal-consistency-guard @ d110393/7b69f702eb) onto current fork/master (which already carries the RBR-1081 admission-control merge, 8165954). Two small conflicts resolved (packages/db/package.json check:migrations script referencing a nonexistent check-migration-safety.ts on this line of history; doc/DATABASE.md section ordering) plus the journal-idx baseline and its pinning tests updated: this fork's migration journal never carried the duplicate idx-178 / gap defects that upstream had at RBR-968 time, so the baseline is empty and the tests that pinned those upstream-specific numbers were rewritten to assert a clean journal instead.
Verified on the fork merge commit:
pnpm --filter @paperclipai/db run check:migrations— OK, 128/128 files match journal entriespnpm --filter @paperclipai/db run typecheck— clean tsc --noEmitpackages/db/src/migration-journal-consistency.test.ts— 33/33 passingnode --test scripts/__tests__/check-migration-journal.test.mjs— 8/8 passing (including the anti-drift test that the .mjs and .ts guards agree)No server code touched (diff is scoped to packages/db, scripts/, docs, .github/workflows/pr.yml, .gitignore). No global-install swap or restart performed — rides the same swap+restart confirmation sequence as RBR-1081/RBR-974.