Skip to content

fix(driver-sql): retire the lookup FOREIGN KEY branch gated on the rejected alias reference_to, and refuse the key - #12287

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-11567-retire-lookup-fk-branch
Aug 25, 2026
Merged

fix(driver-sql): retire the lookup FOREIGN KEY branch gated on the rejected alias reference_to, and refuse the key#12287
os-warren merged 1 commit into
mainfrom
claude/issue-11567-retire-lookup-fk-branch

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#11567

Implements the maintainer ruling of 2026-08-25 (comment): A + D — retire the FK branch, and refuse reference_to loudly.

What changed

SqlDriver.createColumn emitted table.foreign(name).references('id').inTable(field.reference_to) for a relationship field carrying reference_to. reference is the only relationship spelling @objectstack/spec declares — reference_to is a rejected alias — so that branch could not fire for any spec-conformant lookup, and never had.

  1. Retired the FK emission, leaving col = table.string(name).
  2. Refuse loudly: a field still carrying reference_to at DDL time now throws VALIDATION_ERROR/400 naming it as a rejected alias of reference, in the same words FieldSchema uses.

⚠️The site was re-located by symbol, not by line number. The ruling cites sql-driver.ts:13994-13995; on current main the arm is at 14412–14413 (+418 lines, from the four PRs that landed on this file). Located via createColumn's case 'lookup': / case 'user': arm.

I re-verified the ground rather than inheriting it

claimhow I re-verified it here
reference_to is a rejected alias, not a normalised oneRan FieldSchema.safeParse myself: success:false, issue.code=unrecognized_keys, message "Unrecognized key(s) on this field: reference_to. Did you mean reference_toreference?". The canonical reference parses success:true.
no authored deployment has these FKsThe branch was gated on reference_to; a repo-wide sweep finds zero non-test assignments of that key (sole non-test occurrence is a comment). No platform object can reach the branch, which is the mechanism behind the measured 0-FK reading.
the FK emission had zero test coverageSwept every test touching a FOREIGN KEY (14 files). Every one builds its FK with raw knex/SQL DDL or asserts on literal error strings — none goes through createColumn. Confirmed.

Live PG/MySQL were not re-provisioned (no servers running in this container); the 0-FK figure is carried from the investigation, and what I add is the deterministic mechanism plus a permanent pin.

⭐ The transition, measured in both directions

The refusal is a new rejection door on an existing accept path, so here is exactly what now throws that did not:

shape reaching createColumnbeforeafter
{ type:'lookup', reference:'p' } (canonical)column, no FKunchanged — column, no FK
{ type:'lookup', reference_to:'p' }column + a real FOREIGN KEY, silentlythrowsVALIDATION_ERROR/400
{ type:'text', reference_to:'p' }column, key ignored silentlythrows (new)
{ type:'lookup', reference_to:'p', multiple:true }JSON column, key ignored silentlythrows (new)

A deliberate decision worth review: I placed the guard at the top of createColumn, ahead of the if (field.multiple) short-circuit, rather than inside the lookup/user arm. Reason: FieldSchema answers unrecognized_keys for reference_to on any field regardless of type, and a multi-value lookup returns as a JSON column before the type switch — so an arm-local guard would leave that shape silently accepting a key the spec refuses, reproducing the very split the ruling closes, just narrower. This is the wider of the two readings; it is flagged here so it can be narrowed deliberately if the reviewer disagrees.

Predicate is !== undefined, not key-presence: measured that FieldSchema's canonical output does not carry reference_to as an own key, so both are correct — !== undefined is the narrower one and is immune to a producer spreading { reference_to: undefined }.

Obligation 1 — the FK-emission pin, in the retiring direction

New: packages/drivers/driver-sql/src/sql-driver-11567-lookup-no-foreign-key.test.ts (7 tests). It reads SQLite's physical catalog (PRAGMA foreign_key_list), not emitted DDL, and carries a positive control that builds a real FK with raw DDL — so the zeros are measurements, not vacuous passes.

Obligation 2 — the fixture corpus. ⚠️ The count is not 9

#12252 lists 9 fixtures in 6 packages. Measured here: 12 assignments across 8 packages. Three were missing from that list:

filewhy it was missed
packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-driver-schema.test.ts:118a 7th package; SqliteWasmDriver extends SqlDriver, so it inherits createColumn and would have gone red
packages/drivers/driver-sql/src/sql-driver-string-maxlength-varchar.test.ts:74in an already-listed package, so a per-package sweep missed it
packages/drivers/driver-mongodb/src/mongodb-driver.test.ts:347left untouched, out of scope — it feeds #12251's index defect, and "correcting" it would silently delete that card's evidence

⚠️And #12252's disposition — "most of these are a one-word fixture correction" — is wrong for two of them.batch-row-driver-text-real-driver.integration.test.ts and batch-row-http-status-real-driver.integration.test.tspin the very branch being deleted: they assert SQLITE_CONSTRAINT_FOREIGNKEY on a parent delete, and that FK existed only because reference_to reached the FK branch. A rename alone would have dissolved the raw driver fault these security-withhold suites (#8502/#8570) exist to guard.

So the vehicle was rebuilt rather than renamed: a new packages/runtime/src/raw-foreign-key-fixture.ts provisions a real FK with raw DDL — which is what every other FK-touching test here already does.

⭐ A finding that fell out of that repair

Spelling the child field the canonical way ({ type:'lookup', reference:'bd_parent' }) made all three FK-dependent tests fail with "no raw fault" — even though the FK was present and enforced. The engine now recognises the relationship and applies deleteBehavior, clearing the dependent row before the parent delete reaches the database. That is types.mdx's documented contract demonstrating itself, and it is why the FK column is declared text in those two fixtures — documented at the helper. Differential, same tables and same constraint: lookup ⇒ no raw fault, textSQLITE_CONSTRAINT_FOREIGNKEY.

Bounded in-place fix, declared

packages/spec/liveness/field.json — the reference entry's note read "FK DDL reads reference_to (unmapped from reference)", which this diff falsifies. Corrected in place: same defect class, mechanical, and the ledger's live verdict is untouched ($expand/cascade/seed still carry it).

⛔ Not done, deliberately

Changeset grading — argued, not assumed

@objectstack/driver-sql: patch, not declared-breaking. ADR-0087's ledger reaches upgraders about authorable metadata; reference_to is not authorable — the spec refused it at the authoring door before this change too — so no conformant object definition behaves differently and no migration is owed to any deployment objectstack migrate meta can see. node scripts/check-adr-0087-registration.mjs exits 0. ⚠️ That gate is changeset-declaration-driven: it fires on changesets that declare breaking, so its green is a check on my grading being internally consistent, not an independent ruling that the grading is right. Flagged for review.

Verification

Gate union derived, not recallednode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, 15 paths ⇒ 26 path-matched families + 6 convention-triggered, all run at 740275c36c (the final commit):

  • 30/30 static gates exit 0, incl. check:driver-conformance, check:engine-double-contract, check:where-matcher, check:published-files, check:test-source-alias, check:nul-bytes, the four @objectstack/spec liveness gates, and check-adr-0087-registration.
  • check:type-check-coverage exit 0; check:type-check-debt --re-measure"✓ check:type-check-coverage --self-test — 47 semantic case(s) + 65 observation case(s) + 29 re-measure case(s) + 28 built-closure case(s) + 19 auto-lowering case(s) hold."
  • @objectstack/driver-sql full suite: 2044 passed, 0 failed (136 files).
  • Targeted suites green: runtime (7), objectql, rest, lint, driver-sqlite-wasm.
  • check-dev-prereqs was red until the workspace was fully built, then green on the same diff — an environment precondition, not a finding.

Ablation — direction predicted in writing first, both legs

No rebuild leg is owed: the pin imports ../src/index.js, a relative source path, so no dist is in the resolution (ablation-dist-preflight does not apply). Mutations proved on disk by anchored counts before any result was read; trap … EXIT INT TERM armed before the first mutation.

legpredictedobserved
A — re-add FK emission on canonical reference3 FK-absence assertions RED, control + refusals GREENexactly that: 3 failed / 4 passed
B — delete the refusal guard3 reference_to refusals RED, non-vacuity GREENexactly that: 3 failed / 4 passed

Both legs restored byte-identical to HEAD (git hash-object == git rev-parse HEAD:path), verified per leg.

⚠️ Honesty note: leg A's on-disk "removed-anchor count" printed 38 because I passed a multi-line pattern to grep -cF, which counts matching lines — that number is meaningless. Leg A's real proof is its injected-anchor count of 1 plus the differential; leg B's counts (removed 0 / injected 1) are clean.


Generated by Claude Code

…jected alias `reference_to` (#11567)
`createColumn` emitted `table.foreign(name).references('id')` whenever a field
carried `reference_to` — a key `FieldSchema` REFUSES as a rejected alias of
`reference`. That branch could not fire for any spec-conformant lookup and never
had: measured 0 FK constraints across all 44 exported platform objects on live
PostgreSQL 16.13 and MySQL 8.0.46. It was reachable only by metadata that went
around Zod through raw `registerObject`.
Retire the emission, and refuse the key at the DDL seam rather than honouring it
in silence — `VALIDATION_ERROR`/400, in the same words `FieldSchema` uses, so one
key stops having two doors with opposite answers.
Pin the retiring direction (no test anywhere asserted whether a lookup gets a
FOREIGN KEY, in either direction) with a positive control, so a re-added emission
fails loudly. Correct the fixture corpus that spelled the refused key, and rebuild
the two batch-row suites whose raw-fault vehicle was that very FK.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/driver-sql, @objectstack/runtime, @objectstack/spec, touching 7 documentable anchor(s). ⚠️1 changed file(s) yielded no anchor (packages/spec/liveness/field.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

32 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 48c7c34c8850c11f6b4d8b1c93500fa4653e8b9f.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/spec/liveness/field.json) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 34 pages)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 177: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 107 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 134 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 48c7c34c8850c11f6b4d8b1c93500fa4653e8b9fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 78da0b1756486c7a4eefbc600d0ea3247c884252 — the merge of head 740275c36c8958eb2695fd1c8fd2e3b8c8c4fe91 into base 48c7c34c8850c11f6b4d8b1c93500fa4653e8b9f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 78da0b1756486c7a4eefbc600d0ea3247c884252 && git checkout 78da0b1756486c7a4eefbc600d0ea3247c884252
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 48c7c34c8850c11f6b4d8b1c93500fa4653e8b9f 740275c36c8958eb2695fd1c8fd2e3b8c8c4fe91 && git checkout -B drift-repro 48c7c34c8850c11f6b4d8b1c93500fa4653e8b9f && git merge --no-ff 740275c36c8958eb2695fd1c8fd2e3b8c8c4fe91
node scripts/docs-audit/affected-docs.mjs --json 48c7c34c8850c11f6b4d8b1c93500fa4653e8b9f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 48c7c34c8850c11f6b4d8b1c93500fa4653e8b9f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] docs vs driver disagree on whether lookup columns get a real FOREIGN KEY — and the driver branch keys on reference_to, not reference

2 participants

@os-warren@claude