Skip to content

fix(driver-sql,platform-objects): the builtin-column delivery table speaks the spec's field-type vocabulary, not knex's builder names (#12131) - #12686

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-12131-builtin-column-delivery-id-type
Aug 27, 2026
Merged

fix(driver-sql,platform-objects): the builtin-column delivery table speaks the spec's field-type vocabulary, not knex's builder names (#12131)#12686
os-zhuang merged 2 commits into
mainfrom
claude/issue-12131-builtin-column-delivery-id-type

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#12131

Implements the maintainer's 2026-08-27 re-ruling (decision-inbox batch 5, verbatim 「同意」) accepting option D. This supersedes the 2026-08-25 option-A ruling on the same card, which an earlier dispatch stopped on and correctly reported as unexecutable.

The defect

BUILTIN_COLUMN_DELIVERY.id.type recorded 'string' — the knex builder name from table.string('id').primary(). undeliveredStorageAttributes compares that value with === against a declaration's type, which is a spec FieldType. Two different vocabularies, and the recorded one is not a member of the vocabulary it is compared against:

  • 'string' is absent from FieldType's 49 options (measured: FieldType.options.includes('string') === false, includes('text') === true)
  • FieldSchema refuses type: 'string' outright

So no declaration could ever match it, and the #12015 diagnostic reported every correct declaration on the platform's own key as a disagreement.

varchar canonicalizes to the field type textcanonicalizeSqlType('varchar(255)') === 'text', isCompatible('varchar(255)', 'text') === true, both re-measured here and pinned in type-compat.test.ts. So id: Field.text(...) asks for exactly what the platform's column delivers. The 45 declarations were right all along; the instrument was wrong.

Measurement

Direction and exact counts were written down before measuring. The diagnostic emits one warning per colliding field, while the card counts undelivered attributes, so both are reported:

stagewarningsattributes
baseline (origin/main @ 4bd6faa)4546 (45 type + 1 maxLength)
after the one-line producer fix11 (sys_migration.id only)
after the ruled rider00

Corpus: the 45 object definitions exported by @objectstack/platform-objects, de-duplicated by object name.

Positive controls, so a zero is a reading rather than a blind probe — all four still fire at the end:

  • id: {type:'number'} produces exactly 1 attribute (before and after — the fix narrowed the comparison, it did not blind it)
  • created_at: {type:'text'} produces exactly 1
  • id: {maxLength:64} produces exactly 1 (the maxLength branch still works after the rider)
  • a non-builtin field name produces 0

The sys_migration rider — removed, not widened to 255

Ruled in scope as the one honest disagreement in the corpus. It is removed rather than set to 255, because it was measured inert in every seam that could read it:

Declaring a width that nothing enforces is what enforce-or-remove exists to prevent, and the 44 sibling system objects declare none. Widening to 255 would keep an equally inert key while implying the same declaration belongs on the other 44 — which is option A-prime, explicitly measured out. Flagging the direction explicitly since the ruling named the correction without naming which way; it is a one-line change if the maintainer meant 255.

A guard that closes the class

The classification pin now holds every entry in the delivery table to FieldType.options. A knex builder name written there fails by name, instead of surfacing as a corpus of false warnings that reads like 45 bad declarations.

Fixture triage — the pins were written against the table, not the source

Both #12015 pin files spelled sys_presence.id as type: 'string' in their silent cases. That is not how it is declared — sys-presence.object.ts declares Field.text({ label: 'Presence ID', required: true, readonly: true }). So the pins passed while the same declaration as actually written warned, which is why 45 false warnings sat behind green tests. Each fixture was re-judged individually:

Reverse verification

Predicted direction, written before running: RED. Reverting only the delivery table's value to 'string' (mutation proven on disk by anchored greps in both directions plus a git hash-object differing from the HEAD blob) fails 9 of 18 tests across both pin files, including the new FieldType guard. Restore proven by state, not by exit code: the restored blob hash equals the HEAD blob and git diff HEAD is empty. No rebuild leg is claimed or needed — both suites import the module by a relative specifier inside the package, so vitest resolves them to the mutated src/.

The #12015 changeset carried a claim this falsifies

.changeset/builtin-column-collision-warning.md is still pending (not yet compiled into any CHANGELOG) and stated that id: { type: 'text' } fires. It no longer does. Both changesets compile into the same release notes, so the one false clause is corrected in place rather than left to ship contradicting its neighbour. That is the only edit to it.

Verification

Union run at e385a121, with every exit code captured before any pipe.

Greencheck:nul-bytes, check:changeset-gate-self-tests, check:cross-package-test-inputs (both spellings), check:driver-conformance, check:objectql-double-limit, check:objectui-changeset, check:page-declaration-shape, check:pm-half-states, check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check-adr-0087-registration, check-changeset-no-major, check-ci-filter-parity, check-comment-mask-adoption, check-empty-changeset, check-plugin-teardown-shape, release-rehearsal-clone --self-test, docs-audit/check-affected-docs, docs-audit/check-drift-comment, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check:i18n-stale-fill.

Two gates initially refused to measure and were made to measure rather than reported green:

  • check:i18nPREREQUISITE NOT MET: the workspace CLI is not built. Built it; now green, platform-objects in sync (11 bundles) (labels and descriptions are untouched, so nothing was extracted differently).
  • check:type-check-debt — refused with --re-measure cannot run: @objectstack/service-knowledge has no built type entry point. Built that dependency; now green, gate's own line: --re-measure: OK — 31 ledger entr(ies) re-measured ... none above its recorded number.

NOT MEASURED (named, not counted as green)node scripts/pm/check-half-states.mjs, exit 3, PREREQUISITE NOT MET — the token in the environment is not a valid GitHub credential. Its own output states nothing was swept. It is a PM board sweep over GitHub issues and reads nothing in this diff. Its --self-test half (check:pm-half-states) is green above.

Tests@objectstack/driver-sql whole package: 139 passed / 8 skipped files, 2171 passed / 129 skipped tests. @objectstack/platform-objects: 32 files, 519 tests passed. Typecheck clean for both, and tsc --listFiles confirms all three edited TypeScript files are actually in the driver-sql program, so "typecheck is clean" covers the edited tests rather than merely not reading them.

Scope

Untouched, per the ruling: the 45 id: Field.text(...) declarations (their label / required / readonly are load-bearing), sql-driver.ts, and the driver's internal untyped 'string' default, whose rename stays deferred behind the diagnostic's wording. #12593 stays with triage and is not addressed here.

Grade patch for both packages, argued. No door moves: the platform still owns id / created_at / updated_at, the emitted DDL is byte-identical, every object that booted before still boots, and BUILTIN_COLUMN_DELIVERY is internal to the package (not re-exported from the package entry). The platform-objects half removes one metadata key measured inert in every seam. What changes is what the driver says.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 2 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 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 — 10 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 07e64656593afb7c1915f5c97ea2e1587ef3b003packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5d790bf808956f6a9c097135dd803186a6e99b05 — the merge of head e385a1212dee8655171b6b982fdfa580d815884c into base 07e64656593afb7c1915f5c97ea2e1587ef3b003, 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 5d790bf808956f6a9c097135dd803186a6e99b05 && git checkout 5d790bf808956f6a9c097135dd803186a6e99b05
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 07e64656593afb7c1915f5c97ea2e1587ef3b003 e385a1212dee8655171b6b982fdfa580d815884c && git checkout -B drift-repro 07e64656593afb7c1915f5c97ea2e1587ef3b003 && git merge --no-ff e385a1212dee8655171b6b982fdfa580d815884c
node scripts/docs-audit/affected-docs.mjs --json 07e64656593afb7c1915f5c97ea2e1587ef3b003

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 27, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 27, 2026 09:27
@os-zhuang
os-zhuang enabled auto-merge August 27, 2026 09:28
@os-zhuang
os-zhuang added this pull request to the merge queueAug 27, 2026
Merged via the queue into main with commit 3bc2e38Aug 27, 2026
34 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-12131-builtin-column-delivery-id-type branch August 27, 2026 09:51
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.

platform-objects: 45 system objects declare id as text while the platform emits varchar(255) — the whole remainder of #12015's post-narrowing corpus

1 participant

@os-zhuang