fix(driver-sql): a shadow-carried UNIQUE is not index drift, and its remedy dropped the constraint - #13055
Conversation
…remedy dropped the constraint The index differ compared declared columns against the columns an index physically KEYS. A #11627 hash-shadow-carried UNIQUE keys exactly one driver-owned VARBINARY(32) generated column, so the comparison could never match and a clean boot reported the index it had just created as destructive `recreate_index` drift. Following that remedy removed the constraint: the drop-by-name leaves the generated column behind, the re-sync's shadow `ADD COLUMN` fails on the survivor with a duplicate-COLUMN error that neither the "already exists" absorb nor the unique-violation branch matches, and the apply ends with the UNIQUE dropped and not re-created. Both passes now read one vocabulary instead of special-casing the differ: the shadow name derivation moves next to `isHashShadowColumn`, introspection resolves what the shadow HASHES from the stored GENERATION_EXPRESSION, the differ compares the ENFORCED key, and the sync inspects a surviving shadow column (re-key / re-generate / refuse) rather than assuming it absent. A real key comparison, not a blanket skip: a pre-#12998 shadow hashes the RAW columns and leaves every NULL-organization row unconstrained, and is indistinguishable by name from a healthy one. It stays reported, as the ADR-0120 D4 tightening it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
📓 Docs Drift CheckThis PR changes 1 package(s): 6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 9 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 397c11c2150236505ffd0825ef1ab43074de649b && git checkout 397c11c2150236505ffd0825ef1ab43074de649b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df59de0d69130fee44602f4cb1368b07639a886c 2b1593e104579bc4c0dbeb826796bcb2ad8863d5 && git checkout -B drift-repro df59de0d69130fee44602f4cb1368b07639a886c && git merge --no-ff 2b1593e104579bc4c0dbeb826796bcb2ad8863d5
node scripts/docs-audit/affected-docs.mjs --json df59de0d69130fee44602f4cb1368b07639a886c
|
Fixes #13015
A healthy #11627 hash-shadow-carried UNIQUE was reported as destructive index
drift, and following the remedy the differ printed would have dropped the
constraint. Both halves are fixed here.
The defect, reproduced
diffManagedIndexescompared the declared key against the columns an indexphysically KEYS. A shadow-carried UNIQUE keys exactly one driver-owned
VARBINARY(32) generated column, so the comparison could never match. Neither
escape hatch covered it:
isRuntimeManagedIndexsees no expressions (the shadowis a plain column key) and the
runtimeCreatedIndexesledger only records DDLissued through the public
execute()seam.Why the remedy was worse than the defect
recreate_indexdrops the UNIQUE by name and re-runs the additive sync. Thesync retakes the shadow route, and its
ALTER TABLE ... ADD COLUMNthen failson the generated column that SURVIVED the index drop. That is a
duplicate-COLUMN error, matched by neither the "already exists" absorb (which
spells index names) nor the unique-violation branch — so the apply ends with the
constraint dropped and not re-created. An operator following the finding's own
os migrate apply --allow-destructiveadvice removed a live uniquenessguarantee.
The direction taken, and why not the weaker one
The card offered a weaker alternative: record shadow indexes as runtime-managed
so the differ leaves them alone. Rejected, and the rejection is load-bearing.
A shadow written before #12998 hashes the RAW columns, so
CONCATyields NULLfor every NULL-organization row and the rows the
COALESCE(organization_id, '__global__')bucket exists to constrain are constrained by nothing (#5030'sshape). It is indistinguishable BY NAME from a healthy shadow. A blind skip
would have traded one false destructive finding for a true silent one — a class
of unenforced-constraint drift made permanently invisible.
Since #12998 the stored
GENERATION_EXPRESSIONcarries the NULL-safe COALESCEparts, so a real key-form comparison is available, and that is what this does.
The card also notes that half the vocabulary was already taught: the
orphan-COLUMN pass guards the shadow via
isHashShadowColumnwhile the index itcarries was proposed for destructive rebuild. The fix makes both passes read one
vocabulary rather than special-casing the differ:
hashShadowColumnFormoves fromSqlDriverintoschema-drift.tsbesideisHashShadowColumn; the static now delegates, so the name the sync CREATESand the name the differ LOOKS FOR have one definition.
parseHashShadowKeyPartsis the one parser for the generation expression;hashShadowSourceColumnsnow calls it instead of carrying a second copy ofthe regex.
introspectIndexesresolves what each carrier hashes and records it on thePhysicalIndex; the differ compares the key the index ENFORCES(
enforcedIndexKey) rather than the digest column it stores. Drift reportsand plan messages name that key too.
createHashShadowUniqueIndexinspects a surviving shadow column instead ofassuming it absent:
reusableis re-keyed in place,staleis re-generated,foreign(a non-generated column of that name) is refused rather thandropped.
hashShadowColumnStatedecides "same key" throughcanonicalIndexKey, the differ's own identity, so the sync and the differcannot answer the question differently.
A carrier whose expression cannot be read reports nothing rather than proposing
a drop it cannot reason about; the introspection enrichment swallows its own
failure so the read's other consumers (upsert conflict targets, the boot's
index-presence probe) cannot start failing because a second query did.
The pin, in the destructive direction
packages/drivers/driver-sql/src/sql-driver-13015-shadow-carried-index-drift.test.ts.Every "no finding" assertion carries a colocated positive control in the same
diffManagedIndexescall — a genuinely drifted index that must still bereported — so silence can never be read as the differ having gone quiet. The
control shares no key column and no name fragment with the shadow fixtures.
The live-MySQL cell reads the PHYSICAL catalog (
information_schema), never thediffer's report about itself, and asserts the declared UNIQUE is present and
carried by its shadow BEFORE concluding anything from an empty drift list. It
covers: a fresh sync reporting no index drift; applying every reported entry
with
allowDestructiveand the constraint still standing and still enforcing; amanually dropped index whose surviving shadow column is re-keyed; and a
raw-column shadow that is reported, then repaired to the COALESCE form.
Ablation, measured. Two anchored mutations restore the pre-fix differ
(
enforcedIndexKeyalways returns the physical key; the unresolved-carrierguard disabled). Mutation proven on disk in both directions — injected marker
OS13015_ABLATIONcount 2, deleted anchor count 0, blob moved39f7154227to11a19f78f9. Ablated run: 5 of 6 tests RED. The entry thepre-fix differ produces for a healthy shadow-carried unique reproduces the
card's measured output exactly:
Restored byte-identically: worktree blob back to
39f7154227, marker count 0,git diff HEADempty, tests green again. No rebuild was needed for theablation and none is claimed: the suite imports
../src/index.jsand./schema-drift.js, both source paths inside the package, so vitest resolvesthe mutated source directly.
The stale-shadow pin earned its place immediately: it caught a real defect in
the first cut of this fix, where the fail-safe guard was written unconditionally
and silently demoted the whole change to the blind skip argued against above —
green, quiet, and the exact trade this PR exists to refuse.
Verification
All at
2b1593e10, the final commit.pnpm --filter @objectstack/driver-sql test— 145 files passed, 9 skipped;2219 tests passed, 131 skipped.
pnpm --filter @objectstack/driver-sql typecheck(tsc --noEmit) — exit 0.Not assumed to cover the new tests:
tsc --listFilesreports the new testfile in the program (1 hit, with
schema-drift.tsas the positive control).pnpm lint(repo-wideeslint . --no-inline-config) — exit 0 in 63s. Nonarrowing was needed, so none is claimed.
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(change set: 4 paths, all committed) and the union run there: 27 gates green,
including
check:driver-conformance,check:test-source-alias,check:engine-double-contract,check:cross-package-test-inputs,check:dispatcher-error-vocabulary,check:nul-bytes, and the fourchangeset families.
check:type-check-coveragegreen;check:type-check-debt --re-measuregreenafter building the workspace closure — 31 ledger entries re-measured, 1570 raw
tsc errors, none above its recorded number.
scripts/pm/check-half-states.mjsexited 3 =PREREQUISITE NOT MET(noGitHub credential in this container). NOT MEASURED, not a red, and unrelated
to this diff.
Scope notes
Clause-②, asked out loud and answered against the actual diff: NO.
packages/drivers/**is not the contract face and this diff does not widen apublic surface — an earlier cut exported the shadow vocabulary from
src/index.tsand that was reverted, because nothing outside this packageconsumes a hash shadow.
src/index.tsis byte-identical tomain. What thediff DOES change is what
os migrate planreports and whatos migrate applydoes to a production constraint, which is why the question is asked rather than
assumed away; but the accept/reject behaviour of any authored contract is
untouched, no metadata key changes meaning, and no expectation was re-baselined
to match new output. The one existing behaviour that shifts is the
actualstring and message on shadow-carried findings, which now name the enforced key
instead of the digest column.
Changeset:
patch, argued. No public surface change (verified:index.tsuntouched), nothing removed or renamed, so neither minor nor breaking applies.
Not "no changeset" either — AGENTS.md exempts pure bug fixes, but this one
changes operator-visible behaviour on a destructive migration path, which
belongs in the release notes.
Serial queue: this fix is NOT confineable to
schema-drift.ts. The differhalf alone would be, but it would be the blind skip. The real key-form
comparison needs the
GENERATION_EXPRESSIONread inintrospectIndexes, andthe surviving-generated-column handling is in
createHashShadowUniqueIndex—both in
sql-driver.ts. #12999 stays serial behind this card.Generated by Claude Code
Generated by Claude Code