fix(metadata): derive the project_id-to-environment_id migration's table list from the declarations - #13220
Conversation
📓 Docs Drift CheckThis PR changes 1 package(s): 22 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 3 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5540edaa598899958d36d2a0516547e65595a5b5 && git checkout 5540edaa598899958d36d2a0516547e65595a5b5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 96d8b20ee4f4894b4281a552dd5f7eee1c8bb0c5 74304961e37114ab5f0b848ee14bf97bf18e6c24 && git checkout -B drift-repro 96d8b20ee4f4894b4281a552dd5f7eee1c8bb0c5 && git merge --no-ff 74304961e37114ab5f0b848ee14bf97bf18e6c24
node scripts/docs-audit/affected-docs.mjs --json 96d8b20ee4f4894b4281a552dd5f7eee1c8bb0c5
|
|
PM review — seat Route (b) accepted, and the "no new edge" claim is stronger than statedThe body argues the dependency edge pre-existed. The file list proves it outright: Verified
The two design calls are both right
Leaving The ablation is real reverse verification — mutation confirmed on disk by blob hash before the re-run, restore verified by observed state (
|
Fixes #13205
The defect
migrateProjectIdToEnvironmentIdrenamesproject_idtoenvironment_idon the metadata storage tables. ItsAFFECTED_TABLESlist was written by hand, and it outlived the declarations it described:packages/metadata-core/src/objects/sys-metadata-history.object.tsdeclares neitherproject_idnorenvironment_id— the branch/project-removal amendment (M1) removed it; the only occurrence of either name in that file is prose recording the removal.sys_metadata_history, and its only guard asks whetherproject_idis present physically (_columnExists) — which says nothing about the target column being declared.So against any database whose physical
sys_metadata_historystill carried the pre-v5 column, this migration renamed it toenvironment_id: a fresh orphan column that no declaration, nosyncSchemaand no reader knows about — the exact class the metadata drift audit exists to remove.Route taken: (b), gate on the current declaration
The card offered (a) drop the table from the list, or (b) gate each entry on the target column existing in the current declaration, preferring (b) if the declaration is reachable without inventing a dependency edge.
Measured: it is.
@objectstack/metadata'spackage.jsonalready lists"@objectstack/metadata-core": "workspace:*"underdependencies(a runtime dep, not dev), andmetadata-core's root entry already exportsSysMetadataObject/SysMetadataHistoryObject. No new edge, no new import direction —metadata-coreis the lowest shared package and does not depend back.So the list is now derived rather than restated beside the declarations:
CANDIDATE_OBJECTSnames the metadata storage objects this migration considers (the universe, fixed by history: which tables ever carried the tenancy column).AFFECTED_TABLESis those candidates whose current declaration carriesenvironment_id. The derivation and the declaration are now the same fact, so they cannot drift apart again.Route (a) was rejected on the card's own reasoning: it would leave the list a hand-maintained fact whose only protection is the pinning test. Under (b) the test guards a derivation rather than a transcription.
Two deliberate design points a reviewer should check:
status: 'skipped_not_declared'(an additive member of theProjectIdToEnvironmentIdResultstatus union) so the result array still carries one entry per candidate. An operator reading the results can tell "considered and deliberately skipped" from "forgotten again" — which is the state this defect started in. No in-repo caller readsstatus(both call sites indatabase-loader.tsdiscard the results), so nothing switches exhaustively on it today.migrateEnvIdToProjectIdis deliberately NOT changed the same way. Its target (project_id) is an intermediate column that no current declaration carries by design — gating it on today's declarations would disable the chain's first step outright. "The target must be declared" is sound only for the terminal migration of a rename chain. That reasoning is recorded in the file so the next author does not "finish the job".The test, and the property it pins
packages/metadata/src/migrations/migrate-project-id-to-environment-id.test.ts(new, 9 cases). It asserts the property, never a copy of today's answer — there is notoEqual(['sys_metadata'])anywhere in it:AFFECTED_TABLESitself: every table the migration will rename has a declaration, and that declaration carries the column the rename produces. A hand-added entry is caught whatever put it there. This is the assertion that fails onsys_metadata_history.AFFECTED_TABLES.includes(table)is compared against that table's own declaration, so the pin keeps holding — and keeps meaning something — if a declaration legitimately regains or losesenvironment_id.project_idpresent on both physical tables, the migration issues exactly oneALTER TABLE, forsys_metadata, and no statement at all — not even aPRAGMAprobe — against the undeclared table. Plus idempotency and the raw-driver refusal.The test resolves table names against the declarations it imports from
@objectstack/metadata-core(packages/metadatais already registered for that unaliased dep inKNOWN_UNALIASED_TEST_IMPORTS;pnpm check:test-source-aliasstays green with the registry unchanged).Reverse verification (ablation)
Committed the fix first, then mutated
AFFECTED_TABLESback to the pre-fix literal['sys_metadata', 'sys_metadata_history'], confirmed the mutation landed on disk (anchor matched; deleted-text occurrences 0, injected-text occurrences 1; blob hash21013e44tof484a180), and re-ran the test. Predicted direction: red. Observed: red — 5 of 9 failed, including:That last one is the defect itself, reproduced: with the hand-written list the migration reports
sys_metadata_historyas renamed and issues the orphan-mintingALTER. Restore leg verified by observed state, not by an exit code:git diff HEADempty,git statusclean, and the file's hash back to its HEAD blob21013e441079cda371987597c21a5e18b94ac93e. The ablation script carried atrap ... EXIT INT TERMrestore against absolute paths. No rebuild was needed on either leg: the test imports the module under test by relative path (source), andmetadata-core— resolved fromdist— was not mutated and was built before every run.Verification
All of the following ran at final commit
74304961e, with the heavy steps serialized throughscripts/pm/os-verify-lock.sh:pnpm --filter '@objectstack/metadata^...' buildVERDICT command-exit 0pnpm --filter @objectstack/metadata buildcheck-dts-emitted: @objectstack/metadata - 4/4 declared declaration file(s) present.Test Files 1 passed (1) · Tests 9 passed (9)pnpm --filter @objectstack/metadata exec vitest run(whole package)Test Files 39 passed (39) · Tests 687 passed (687)tsc --noEmit -p packages/metadata/tsconfig.jsoncheck:type-check-debtledger count for this package, and 0 of them in either changed file, so the ratchet does not movepnpm lint(repo-wideeslint . --no-inline-config)VERDICT command-exit 0, 89s — no narrowing claimedtsc --listFilesconfirms both changed files are inside the program that produced that count (2 hits), so the typecheck reading is a measurement over this diff and not a green gate over files nothing read.Gate families derived mechanically from the diff with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(not from a hand-written list) — all 22 runnable ones exit 0:check:nul-bytes·check:test-source-alias·check:undeclared-dep-imports·check:type-source-resolution·check:cross-package-test-inputs·check:engine-double-contract·check:where-matcher·check:query-options-erasure·check:objectql-double-limit·check:durability-log-level·check:logger-receiver-detach·check:published-files·check:page-declaration-shape·check:slot-lookup·check:objectui-changeset·check:pm-half-states·check:changeset-gate-self-tests·check:type-check-coverage·check-keyed-text-bounds·check-comment-mask-adoption·check-adr-0087-registration·check-changeset-no-major·check-empty-changeset·check-plugin-teardown-shapeTwo matched gates are NOT MEASURED locally rather than green, and say so themselves:
check:dual-build-cjs-loadsandcheck-test-completeness.mjsboth exit 3 (PREREQUISITE NOT MET) — the first wants a full workspacedist, the second grades a saved CIturbo run testlog it cannot produce. Both are CI-owned runs.Changeset
.changeset/migration-affected-tables-declared-gate.md, gradedpatch: this repairs a migration that would corrupt a schema, and narrows no published contract (the status union gains a member; nothing is removed or re-shaped).Not touched:
content/docs/releases/,docs/adr/**,.claude/**,skills/**,packages/spec/src/**,packages/drivers/driver-sql/src/sql-driver.ts.Out-of-scope finding
Filed unassigned as #13219 (
finding): the raw-driver guard in this same file prints its instruction sentence twice. Different defect class, one line, deliberately left byte-identical here.Generated by Claude Code
Generated by Claude Code