fix(spec): withdraw the ADR-0087 field-required-notnull-explicit conversion — required: true stops stamping storage.notNull - #16890
Conversation
…r87-required-notnull-conversion
…ta` supplies `storage.notNull` The `relax_not_null` finding prescribed "(pre-protocol-17 sources: `os migrate meta` stamps it for every previously-required field)" as the way a NOT NULL column with no declared storage constraint gets ratified, and the comment above it closed with "`os migrate meta` ratifies it whenever the source is next migrated". Withdrawing the `field-required-notnull-explicit` conversion in this same PR is what makes both false: no chain step writes `storage.notNull` for anybody, at any protocol floor. Both now say the constraint is the author's own declaration. The deliberate SILENCE for a `required: true` field whose column is already NOT NULL is untouched — this corrects the sentences, never the finding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
📓 Docs Drift CheckThis PR changes 2 package(s): 7 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 — 133 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 0bd9b60df2025665ba26a482ca6e32b0f509c8c8 && git checkout 0bd9b60df2025665ba26a482ca6e32b0f509c8c8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin afa3a2698f5f1e24294f509ab9f0afefbdd424b8 371d28c35729c8dab9d9a66506eaabcc6ac94371 && git checkout -B drift-repro afa3a2698f5f1e24294f509ab9f0afefbdd424b8 && git merge --no-ff 371d28c35729c8dab9d9a66506eaabcc6ac94371
node scripts/docs-audit/affected-docs.mjs --json afa3a2698f5f1e24294f509ab9f0afefbdd424b8
|
Landing — ready + auto-merge armed. All three pre-checks PASS, and ② is a real pass this time.① In-seat contract tier — n/a. ② Both carriers, machine-read. Pair document built with
⭐ The second column is the one that matters, and it is why this is not the same reading as #16840. There, the script returned ③ Every check green. Head Closing-keyword scan of the whole body: exactly one keyword+issue pair — Landing is verified afterwards by content on
Generated by Claude Code |
Fixes #16693
Clause-②: no
Executes the maintainer ruling recorded on the card (director seat, decision batch #85, 2026-09-08) — option A: the ADR-0087
field-required-notnull-explicitconversion drops its storage claim. Quoted rather than paraphrased:ADR-0113 is the protocol:
requiredis the write-time contract and is not a column constraint;storage.notNullalone binds the physical column. A conversion that addsstorage.notNullwherever it findsrequired: trueasserts exactly the implication that ADR abolished.The mandatory first measurement: the artifact door APPLIES the conversion, it does not merely report it
The order required this to be established before any edit, because it decides the shape of the fix. It was taken on this branch, with controls, and it refutes the reading that
retiredFromLoadPath: trueheld the transform toos migrate meta.Which path emits the boot line.
packages/metadata/src/plugin.ts, method_convertArtifactForward— it builds the[MetadataPlugin] artifact … predates this runtime's spec … converted N site(s) forward via ADR-0087 conversion …string, and its only transform isapplyArtifactForwardConversionsfrom@objectstack/metadata-core. That function's return value is not discarded:_parseAndRegisterArtifactfeeds it toEnvironmentArtifactSchema.parse/ObjectStackDefinitionSchema.parseat all three of its call sites and registers the result.Whether it applies.
applyArtifactForwardConversionscallsapplyConversions(definition, { includeRetired: true, … })and returnsconverted. TheincludeRetired: trueis what overrides the flag — the loop it overrides reads:Measured at that seam, by ablation on this branch — the withdrawn entry re-added from the merge base,
@objectstack/specand@objectstack/metadata-corerebuilt, the mutation proved live indist/byscripts/ablation-dist-preflight.mjs(marker present in 6 built files) before any verdict was read:The negative control shows the instrument can say NO; the firing control shows the retired window is open on the same floor for a different retired conversion, so the empty subject line at HEAD is a verdict, not a dead instrument; the
includeRetired=falseleg shows the flag itself is not broken — it is overridden by the caller.So the ruling's first clause is live and non-vacuous. Removing the registry entry is what stops the stamping; nothing else on the load path was holding it back.
os migrate meta's pre-17 path is untouched, exactly as the order required.The three clauses, and what each turned out to cost
1 — the conversion no longer adds
storage.notNull. The entry leavesCONVERSIONS_BY_MAJOR, protocol 17's ADR-0087 ledger step, and the generatedspec-changes.json/protocol-upgrade-guide.mdrows. A tombstone docblock stands where the entry was, so re-adding one is a mistake with a comment in its way.2 — the remedy sentence: measured, and there is no hand-written sentence to rewrite. This is the half the dispatching seat flagged as possibly missing, and it is worth stating what the tree actually holds:
git grep -n "notNull"overpackages/cli/src/commands/compile.ts,lint.tsandvalidate.tsreturns zero hits.formatConversionNoticeinpackages/cli/src/utils/format.ts. The ADR-0087 conversion-notice wording is duplicated verbatim across all three authoring commands, and no gate holds the three equal #13743 already hoisted the three verbatim copies into it, and its docblock says so in the past tense. That function carries no remedy clause at all.from/topair:packages/spec/src/conversions/apply.ts—`Update the source to '${detail.to}'.`— which is the one the boot warning echoes throughagg.message, andpackages/spec/src/stack.zod.ts, whose wording is the constant "Update the source to the canonical shape" and never names a column.The boot line said
Update the source to 'storage.notNull: true'.only because the conversion declaredto: 'storage.notNull: true'. Withdrawing the conversion is what removes the prescription; rewording any of the three commands would either be a no-op or would change every other conversion's notice. ⇒ Clause 2 is discharged by clause 1, and the ruling's parenthetical about a three-way duplication is stale against this tree rather than wrong about the intent.3 — the falsified comment in
sql-driver.ts. Anchored on its text, not on a line number (:16432in the ruling had already drifted to:16591, and drifted again under this branch's merge). The sentence that stood there:is replaced by what the block actually does: a column reaches
notNullable()there because its author wrotestorage: { notNull: true }, and for no other reason.createColumnhas onenotNullable()branch and no dialect switch — the only othernotNullable()on a managed column in that file is the SQLite table-rebuild path, which reads physical introspection plus the drift entry sets, neverrequired.One correction beyond the fence, because this change is what makes it false
packages/drivers/driver-sql/src/schema-drift.tscarried the same falsified claim as thesql-driver.tssentence, in user-facing output. Therelax_not_nullfinding — raised when a column is NOT NULL and the metadata declares nostorageconstraint — prescribed:and the comment above it closed with "
os migrate metaratifies it whenever the source is next migrated". Withdrawing the conversion in this PR is precisely what makes both untrue: after it, no chain step writesstorage.notNullfor anybody, at any protocol floor. Both now say the constraint is the author's own declaration.required: truefield whose column is already NOT NULL, and both drift arms, are byte-for-byte the same. This corrects two sentences, never a finding.@objectstack/driver-sqlis added to the changeset because one of the two is a message users read.The two files outside the order's original fence, and why each has to move
Neither is discretionary, and "it pins the conversion" is not by itself a reason — here is the measured one for each.
packages/cli/test/migrate-meta.e2e.test.ts— forced, and invisible to this PR's CI. Reverting only this file to its merge-base bytes against the fixed tree was run as its own leg: the anchor was proved on disk first (0 occurrences at HEAD, 1 after the revert), no rebuild was needed because the subject resolves throughpackages/spec/dist, which already carries the fix. Result —Tests 2 failed | 16 passed:The file states the old contract twice — in
EXPECTED_CONVERSIONS, and in a snapshot assertion thattitle.storageequals{ notNull: true }— so leaving it untouched lands a red. It is also a*.e2e.test.ts, which since #16481 runs nightly onmain, not on this PR:OS_TEST_TIERSunset selects the 212 non-tier files and this file is in neither project's include list. Left alone it would have gone red onmainafter the merge, hours later, off the PR that caused it. It was run here underOS_TEST_TIERS=nightly --project integrationin both directions.The edit keeps the conversion's input rather than deleting it:
titleis still authoredrequired: truein the protocol-16 fixture, and the new assertion is that the chain attributes nothing to the withdrawn id while still attributing rewrites to others — so it cannot pass vacuously on an emptyappliedlist.packages/metadata-core/src/artifact-forward-conversion.test.ts— the seam is the defect's blast radius, and only this package can state it. The spec-side pin proves the registry holds no such conversion in either posture. It would keep passing if the stamp came back at the door instead — and the door is where this reached users: an artifact declaring^17.0.0, the rangecreate-objectstackstamps, converted at boot because the key is the declaredengines.protocolfloor and not the artifact's age. The regression users felt is "a scaffolded app boots with NOT NULL columns nobody asked for", and that sentence is only expressible in this package's terms. The pin carries its own anti-vacuity (the window really is open on the input:verdictisconverted-forwardandauthoredFlooris17.0.0), a positive case that an author-declaredstorage.notNullstill survives the door, and a firing control asserting a real rewrite by a different retired conversion on the same floor. Under the ablation above, this file goes red —Tests 1 failed | 18 passed— so it discriminates.Verification
Run on the final head, after
git merge origin/mainthroughscripts/pm/os-regen-merge.sh(merge committed first, then regenerated;git status --porcelainempty afterwards).pnpm --filter @objectstack/spec testTest Files 465 passed (465)·Tests 12972 passed (12972)· exit 0pnpm --filter @objectstack/spec typecheckpnpm --filter @objectstack/metadata-core testTest Files 15 passed (15)·Tests 264 passed (264)· exit 0pnpm --filter @objectstack/metadata-core typecheckpnpm --filter @objectstack/driver-sql testTest Files 161 passed | 10 skipped (171)·Tests 2445 passed | 145 skipped· exit 0pnpm --filter @objectstack/driver-sql typecheckpnpm --filter @objectstack/cli exec vitest run --project unitTest Files 186 passed (186)·Tests 2555 passed | 6 expected fail· exit 0OS_TEST_TIERS=nightly … --project integration test/migrate-meta.e2e.test.tsTest Files 1 passed (1)·Tests 19 passed (19)· exit 0pnpm --filter @objectstack/cli typecheckcheck:spec-changes"spec-changes.json is up to date" ·check:upgrade-guide"protocol-upgrade-guide.md is up to date" ·check:authorable-surfaceverified against upstream 5aef6db ·check:generated"All 15 generated artifacts are up to date" — all exit 0eslint . --no-inline-config(the whole repo, not a narrowing)Gates. Derived with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackon this head and reconciled with--ran, verbatim:$?before any pipe. Nothing in the run returned3,99,254or a queue timeout.Heavy runs went through
scripts/pm/os-verify-lock.sh; every wall-clock figure above is a shared-box reading, as that script's own verdict lines say.Ablation hygiene. Both ablations (the registry entry, and the pre-edit e2e file) restored via
git checkout HEAD -- PATHunder anEXIT INT TERMtrap with absolute paths, each restore proved by blob-hash equality and an empty whole-treegit status --porcelain, and the registry one additionally byablation-dist-preflight … --absent:marker absent from all 218 built files.Reported, not edited — stale lines on paths this PR may not touch
Each still describes the withdrawn conversion as live. All are out of bounds for a code PR and are listed for whoever owns them:
docs/adr/0113-required-write-contract-vs-column-constraint.mdlines 3 and 168 — governed (docs/adr/**); the ruling assigns this to a docs-only governed PR for the maintainer.skills/objectstack-upgrade/SKILL.mdlines 431 and 461 — governed (skills/**). Line 461 is a worked example whose right-hand column is the stamp this PR removes..claude/skills/spec-property-retirement/SKILL.mdline 212 — governed (.claude/**). It teaches theretiredFromLoadPathreasoning the measurement above refutes.content/docs/releases/v17.mdxlines 892 and 894 — release notes, written centrally at release time, never a rider on a code PR.packages/spec/CHANGELOG.md,packages/objectql/CHANGELOG.md,packages/drivers/driver-sql/CHANGELOG.md— generated, not hand-edited.验收备注
retiredFromLoadPath: truedoes not keep a conversion off any load path — three runtime seams replay every retired entry withincludeRetired: true, andapply.tssays onlymigrate metadoes #16864 —retiredFromLoadPath: trueholds nothing back at three runtime seams, all of which passincludeRetired: true. That card is open and independent; this PR only removes today's destructive instance. The tombstone docblock points at it so the next author who sets that flag on a default flip reads it first.converted 40 site(s)logged in the same boot that created 0 NOT NULL columns. The measurement above settles the first half — the door really does apply, so the discrepancy lies downstream of the door, not inretiredFromLoadPath. It was not chased further: reproducing it needs an app boot this PR has no business performing, option A makes it moot for this conversion (nothing is converted, so nothing can fail to arrive), and it stays live for the other 73 retired entries. The nearest recorded work is AppPlugin registers artifact security metadata in memory from the RAW bundle, bypassing the artifact door's ADR-0087 conversion — a second unconverted reader of the same bytes #12844, whose repair routed the security bundle through the door inpackages/runtime/src/app-plugin.ts("Same bytes, same conversion policy — one funnel") and left the object half unexamined, andos devover a HOST config composes two registrars for stack-declared security metadata —AppPluginover the config module and the dev-only HMRMetadataPluginover dist/objectstack.json #14397 on the two registrarsos devcomposes. Both are closed. Whether that deserves its own measurement card is the seat's call, not this PR's.check:comment-mask-adoption,check:comment-mask-corpus,check:doc-authoringandcheck:api-surfaceare all green over it.patchfor both packages: this is afixin the ruling's own framing — a behaviour the ADR had already abolished, removed. Nothing is authored differently and nothing is refused that was accepted before.origin/mainmoved several times during verification. Everything above was measured on this branch's head; the gate derivation printed a STALE TREE notice naming files it derives from that moved afterwards. CI on the PR re-derives against the realmain, and the merge queue rebuilds on its own generation.Generated by Claude Code