test(migrate-ts): green the Postgres integration release gate - #280
Merged
Conversation
The `migrate-ts-pg` job in integration-tests.yml has failed on EVERY release tag back to at least v0.20.0 -- the same 6 tests on v0.21.0 and v0.21.1. It went unnoticed structurally, not carelessly: the lane runs ONLY on `v*` tag push, so the red arrives AFTER publishing, and the same tests `describe.skip` silently without MIGRATE_TS_PG_URL, so local and PR runs look green. A release gate that is permanently red provides no signal, which is how the #279 serial-PK adoption bug reached a release. Both root causes are test-side; no product defect. 1. Sourceless fixtures (4 of 6 failures). `postgres-lenient-inet` and `pg-adversarial-fixes` declared an `object.entity` with NO `source.rdb` child. Persistability derives from a declared/inherited `source.*` (#249), so those entities are non-persistable: no CREATE TABLE is emitted, the table never exists, and the assertions fail -- then cascade as `relation "endpoints"/"readings" does not exist`. Proven, not inferred: adding `source.rdb` alone takes each file from all-fail to all-pass. 2. Stale CHECK-expression spacing (2 of 6). The expectations still carried `'open', 'closed'` with spaces after the commas, but `checkExprs` runs `normalizeCheckExpr`, which deliberately collapses comma spacing outside literals. The normalizer is right; the expectations predate it. Verified against a real Postgres 16: the full integration suite goes 124 pass / 6 fail -> 130 pass / 0 fail. The no-PG path still skips cleanly (688 pass / 19 skip / 0 fail), so PR runs are unaffected. Deliberately NOT changed: the lane's tag-only trigger. Making it run on merge is a workflow policy decision for the maintainer, not a test fix -- but until it does, this lane can silently rot again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TqsuDye2SfXGf43vuoD3n
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.
Intent
Green the migrate-ts-pg release gate, which has been failing on EVERY release tag going back to at least v0.20.0 — the same 6 tests on v0.21.0 and v0.21.1, verified byte-identical between those two runs.
WHY IT WENT UNNOTICED (structural, not carelessness). Three compounding blind spots: the migrate-ts-pg job runs ONLY on
v*tag push or manual dispatch, never on PR or merge — so the red arrives AFTER publishing, when the irreversible step is already done; the same testsdescribe.skipSILENTLY when MIGRATE_TS_PG_URL is unset, so local runs and PR runs both look green; and nobody re-checks a tag run once the packages are out. A release gate that is permanently red provides zero signal, and that is precisely how the #279 legacy-Postgres-serial-PK adoption bug (which would drop a live table's PK default and break every insert) reached a release without being caught.BOTH ROOT CAUSES ARE TEST-SIDE. There is no product defect here, and no product file is touched.
(1) SOURCELESS FIXTURES — 4 of the 6 failures.
postgres-lenient-inet.test.tsandpg-adversarial-fixes.test.tseach declared anobject.entitywith NOsource.rdbchild. Persistability derives from a declared or inheritedsource.*child (#249, "derive persistability from source presence, not object subtype"), so a sourceless entity is not persistable: no CREATE TABLE is emitted, the table never exists, the first assertion fails on a missing column, and the remaining tests in the file cascade asrelation "endpoints" does not exist/relation "readings" does not exist. This was PROVEN, not inferred — addingsource.rdbalone, with no other change, took each file from all-fail to all-pass, and the change was reverted and re-applied deliberately to confirm.(2) STALE CHECK-EXPRESSION SPACING — the other 2.
lifecycle-pg.test.tsstill expectedstatus in 'open', 'closed'with spaces after the commas, but its owncheckExprshelper runs the product'snormalizeCheckExpr, which deliberately collapses comma spacing outside string literals (from the 0.20.2 CHECK-reconciliation work). The normalizer is correct; the expectations predate it and were never updated, because this lane never ran anywhere they would be seen.VERIFICATION. Against a real Postgres 16 in a throwaway container: the full migrate-ts integration suite goes from 124 pass / 6 fail to 130 pass / 0 fail. The no-PG path still skips cleanly (688 pass / 19 skip / 0 fail), so PR runs and local runs are unaffected by these changes. Each failing file was also run in ISOLATION first, which falsified an earlier hypothesis that the failures were cross-file interference from shared-database test ordering — they fail alone too, so the cause is deterministic and per-fixture.
ONE CORRECTION TO AN EARLIER CLAIM, recorded deliberately: I initially reported the lane went red at v0.20.11 and suspected the #249 persistability change caused it. Checking further back showed the lane was ALREADY red at v0.20.0, so the redness has a longer, multi-cause history and should not be pinned on a single commit. What is proven is the current six and their two causes.
DELIBERATELY NOT CHANGED: the lane's tag-only trigger. Making migrate-ts-pg run on merge (or on PRs touching migrate-ts) is a workflow policy decision for the maintainer, not something a test fix should smuggle in. But until that changes, this lane can silently rot again — the structural half of the problem remains open by design, not by oversight.
Scope: three test files under server/typescript/packages/migrate-ts/test/integration/. No product code, no metadata vocabulary, no workflow files.
What Changed
source.rdbchild (@table) to the sourcelessEndpointandReadingentity fixtures inpostgres-lenient-inet.test.tsandpg-adversarial-fixes.test.ts— without a declared/inherited source these entities are non-persistable (fix: derive persistability from source presence, not object subtype #249), so noCREATE TABLEwas emitted and every assertion in the file cascaded intorelation ... does not exist.lifecycle-pg.test.ts(status in 'open', 'closed'→status in 'open','closed') to match the product'snormalizeCheckExpr, which deliberately collapses comma spacing outside string literals; the normalizer is correct, the stale expectations predated it.No product code is touched — both root causes are test-side. The full migrate-ts suite against real Postgres goes from 124 pass / 6 fail to 130 pass / 0 fail; the no-PG path still skips cleanly (688 pass / 19 skip / 0 fail), so PR and local runs are unaffected.
Risk Assessment
✅ Low: A well-bounded, test-only fix across three integration fixtures; both root causes are verified against product source (persistability derives from source presence per expected-schema.ts:146-156; normalizeCheckExpr collapses comma spacing outside quotes per check-expr-compare.ts:46,50-82), no product/workflow/metadata code is changed, and a diligence scan found no other latent instances of either pattern in the PG lane.
Testing
Reproduced the migrate-ts-pg CI gate exactly (postgres:16 sidecar, same credentials, the gate's own
bun testcommand). With PG set the full package is 708 pass / 0 fail (was 6 fail); without PG it is 688 pass / 19 skip / 0 fail, matching the intent's claim that PR/local runs are unaffected. A controlled before/after (revert the 3 files to base → 6 failures with the two documented causes: sourcelessendpoints/readingsfixtures and stale CHECK-expr comma spacing; restore → 8 pass / 0 fail) proves the fix is deterministic and addresses both root causes. Only test files changed; no product code touched. Transient container removed and worktree left clean at the target commit.Evidence: Full migrate-ts suite vs real Postgres (gate green)
bun test v1.3.14 (0d9b296a) 708 pass 0 fail 1498 expect() calls Ran 708 tests across 119 files. [5.37s]Evidence: Full migrate-ts suite, no PG (skip-cleanly check)
bun test v1.3.14 (0d9b296a) 688 pass 19 skip 0 fail 1345 expect() calls Ran 707 tests across 119 files. [3.66s]Evidence: Before-state: 3 base-version files vs real PG (6 failures reproduced)
(base commit, 3 files vs PG) 2 pass / 6 fail — root causes reproduced: • postgres-lenient-inet: error: relation "endpoints" does not exist (×2, sourceless fixture) • pg-adversarial-fixes: error: relation "readings" does not exist (×2, sourceless fixture) • lifecycle-pg: expect(checkExprs).toEqual/toContain failed on 'status in 'open', 'closed'' vs collapsed 'status in 'open','closed'' (×2, stale CHECK-expr spacing) After restoring target versions: 8 pass / 0 fail.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
git diff 35b0250c0..396b860d5 (scope: 3 test files, 6 line-changes, no product code)MIGRATE_TS_PG_URL=postgres://migrate:migrate@localhost:5432/migrate_test bun test test/integration/lifecycle-pg.test.ts test/integration/pg-adversarial-fixes.test.ts test/integration/postgres-lenient-inet.test.ts (3 changed files in isolation vs real PG → 8 pass / 0 fail)MIGRATE_TS_PG_URL=... bun test (full migrate-ts package vs real PG, the literal gate command → 708 pass / 0 fail across 119 files)env -u MIGRATE_TS_PG_URL bun test (full package, no PG → 688 pass / 19 skip / 0 fail)git checkout 35b0250c0 -- <3 files> then bun test vs PG (before-state reproduced the 6 failures: endpoints/readings 'does not exist' + CHECK-expr spacing)git checkout HEAD -- <3 files> then bun test vs PG (after-state restored: 8 pass / 0 fail)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.