Skip to content

fix(spec): the 17 → 18 chain names the bare element:filter / element:form node it leaves standing - #17845

Merged
os-bill merged 3 commits into
mainfrom
claude/issue-17594-step18-element-node-todo
Sep 12, 2026
Merged

fix(spec): the 17 → 18 chain names the bare element:filter / element:form node it leaves standing#17845
os-bill merged 3 commits into
mainfrom
claude/issue-17594-step18-element-node-todo

Conversation

@os-bill

@os-bill os-bill commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Fixes #17594

Clause-②: no — an ADR-0087 D3 semantic TODO plus a disposition move. No key is added to any published payload, no accept set moves, and the refusal this TODO describes already exists at HEAD.

What was wrong

element:filter and element:form were retired whole at element grain (ADR-0049). The two ADR-0087 D2 conversions that carry the retirement — element-filter-removed and element-form-removed — strip every authorable key and deliberately leave the bare component node: deleting an authored page node is a layout decision a mechanical conversion must not make.

That residue was inert until both names joined RETIRED_PAGE_COMPONENT_TYPES and PageComponentSchema.type began refusing them by name. At that point deleting the node stopped being optional and became a required step of the 17 → 18 chain — and nothing in the chain said so.

The defect, reproduced before the change

A stack carrying one node of each, driven through the real CLI (packages/cli/bin/run-dev.js migrate meta --from 17 --to 18):

--json        schemaValid: false        applied: 9      todos: 115
human path    "Migrated stack does not yet pass schema validation —
               resolve the manual changes above, then run `os validate`."

step-18 todos naming the node               human output (396 lines)
  element:filter     0                        0
  element:form       0                        0
  ElementFilter      0                        0
  ElementForm        0                        0

lit control, same corpus, same probe
  element:number     3 todos                  6 lines
  element:record_picker  3 todos              —

⇒ the zero is a reading, not a probe that could not reach. The nine mechanical rewrites the same run attributed are all to element-filter-removed / element-form-removed, so the run really did visit both nodes and really did leave them standing.

After

Same input, same command:

--json        schemaValid: false        applied: 9      todos: 116
todos whose `surface` names BOTH node types: 1
  id       element-filter-and-form-node-refused
  toMajor  18

human path, line 150
  ⚠ [protocol 18] page.component.element:filter / page.component.element:form — the bare
    component node itself, left standing by the `element-filter-removed` and
    `element-form-removed` conversions after they strip its properties
    → Delete the component node. `element:filter` → … `userFilters` … `element:form` →
      the object-bound `object-form` block …

It names the thing to delete rather than describing it. And the prescription is checkable: with both nodes deleted by hand and nothing put in their place, the same command reports schemaValid: true and the human path prints ✓ Migrated stack is schema-valid.

schemaValid stays false in the after-run by design — the node is still there until a human deletes it. What changed is that the author is now told which node, and what to put in its place.

How the entry was added

Per packages/spec/src/migrations/entries/README.md: one new file, named for its id, then the generator. ⛔ Nothing was typed between the os-generated markers.

packages/spec/src/migrations/entries/semantic/18.element-filter-and-form-node-refused.ts   (new)
pnpm --filter @objectstack/spec gen:migration-registry
  → ✓ wrote src/migrations/registry.ts (203 semantic, 168 retired-key, 178 retired-def)
  → +35 lines inside the `os-generated semantic:18` region, nothing else in the file moved

pnpm --filter @objectstack/spec check:migration-registry   exit 0
  → build-migration-registry --self-test: ok
  → ✓ src/migrations/registry.ts is current (203 semantic, …)
pnpm --filter @objectstack/spec check:generated             exit 0
  → ✓ All 15 generated artifacts are up to date.

spec-changes.json and docs/protocol-upgrade-guide.md are unchanged, correctly: both project only up to PROTOCOL_MAJOR (17). Lit control — the pre-existing step-18 entry element-number-filter-rule-array has 0 hits in either file too, so the empty diff is a property of the projection range and not of this entry.

The ADR-0087 disposition move

.changeset/15110-retired-element-node-refusal.md — the changeset of the PR that made the node refusable — moves from

not-required (already-registered element-filter-removed, element-form-removed)

to

registered element-filter-and-form-node-refused

The old disposition was gate-true and semantically thin: the two ids it named registered the key strips, not the node deletion that the refusal turned into a required step. The registered form the gate parses carries ids only, so the old marker's rationale sentence does not survive the move — it is superseded by the entry's own reason field, which states the same thing at the place a consumer reads it.

node scripts/check-adr-0087-registration.mjs --base origin/main → exit 0 (and --self-test → exit 0).

Ablation of the new pin

The pin is six cases in packages/spec/src/migrations/migrations.test.ts. Mutation = delete the entry file and re-run the generator; the fix was committed first, so the restore point is a commit that exists.

HEAD blobs (restore target)
  entry     48c9acbf9b068d75a29143f8a2da4e0e643400b9
  registry  b457da53ce9927765b2add7afc3e98901b552d85

on-disk proof of the mutation (not an editor exit code)
  entry file present after rm: no
  occurrences of `element-filter-and-form-node-refused` in registry.ts: pre 1 → post 0
  registry.ts blob after regeneration: 9b9914a56a19d514bbddd93e8ac0de470d542148
    — byte-identical to the pre-change registry blob, so the generator is deterministic
      and the whole diff to that file is this entry

RED    Tests  5 failed | 119 passed (124)
       × finds the entry (anti-vacuity …)
       × a 17 → 18 run emits exactly one todo naming BOTH node types (ADR-0087 D3)
       × prescribes DELETING the node, and names each element's replacement
       × ⛔ does not prescribe an automatic delete …
       × the acceptance criterion is checkable, and names `os validate` …

restore  git checkout HEAD -- BOTH_PATHS
         hashes read back EQUAL to the HEAD blobs above; `git diff HEAD` empty;
         `git status --porcelain` empty

GREEN  Tests  124 passed (124)

The sixth case — the replay really does leave the bare nodes — stays green through the ablation on purpose: it pins the conversions' behaviour, which this PR does not touch, so the block is not one undifferentiated assertion. trap RESTORE_FN on EXIT INT TERM with absolute paths throughout; the restore is proven by git hash-object, never by an exit code.

No dist preflight: the pinned subject is ./registry.js imported relatively from inside the same package, so vitest resolves it to src/. No exports hop, no dist on the resolution path.

Changeset — measured, not assumed

Build first (both passes confirmed: dist/.build-input-hash and dist/.build-input-hash-dts stamped at the same input hash, check-dts-emitted: 34/34), then npm pack --dry-run --json over packages/spec (2012 files):

probe packed files containing it
target — the entry id 4 (dist/index.js, dist/index.mjs, dist/browser/index.js, dist/browser/index.mjs)
target — the entry's surface prose 4 (same)
targetDelete the component node. 4 (same)
positive control — element-number-filter-rule-array 10
positive control — that entry's reason prose 5
negative control — test-only prose 0
negative control — the entry file's own // comment 0
negative control — changeset-only prose 0

⇒ this round's text reaches a published dist. Changeset written: .changeset/17594-step18-element-node-todo.md, @objectstack/spec: patch.

The entry file's internal comment (the measurement provenance) lands in no published byte: it sits above the import, so the generator reads it as file scaffolding and it never enters registry.ts. The packages/spec tsup-does-not-strip-comments trap therefore does not apply here — measured, not reasoned.

Verification

what result
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack 83 commands derived
those 83 83 exit 0 — two needed a re-run and are counted at their real reading, see below
dispatch-gates.mjs --ran reconciliation 83 derived, 83 run, 0 NOT-MEASURED (derived from recorded exit codes), 0 UNRUN
pnpm --filter @objectstack/spec exec vitest run --project local src/migrations/migrations.test.ts 124 passed (118 before, +6)
pnpm --filter @objectstack/spec test (the local vitest project) 471 files / 13405 tests passed, VERDICT command-exit 0
pnpm --filter @objectstack/spec test:repo (the repo project — the corpus scanners) 30 files / 520 tests passed, VERDICT command-exit 0
pnpm --filter @objectstack/spec typecheck exit 0 — includes check:test-typecheck, which compiles the test layer under tsconfig.test.json, so the new pin is type-checked rather than merely executed
pnpm --filter @objectstack/spec check:migration-registry / check:generated exit 0 / exit 0
pnpm exec eslint . --no-inline-config exit 0 over 6647 files (eslint's own config decides that population), 0 errors / 0 warnings, at bca74a4601

Two of the 83 first came back NOT MEASURED, never green:

  • pnpm check:dual-build-cjs-loadsexit 3, PREREQUISITE NOT MET (twelve packages had no dist). Built the closure (turbo run build --filter='./packages/*' --filter='./packages/*/*') and re-ran → exit 0.
  • pnpm check:type-check-debtexit 143 (SIGTERM) on the first pass, then exit 3 for the same missing-closure reason. Re-ran after the build → exit 0, 5 ledger entries re-measured, none above its recorded number.

Every exit code above was captured before any pipe (cmd > file 2>&1; EXIT=$?), and every locked run was read from the wrapper's own VERDICT command-exit line, never a bare $?.

The turbo build is a shared cache across worktrees, so packages/spec/dist was re-checked for this entry by occurrence count after it (1 hit in dist/index.js, 1 in dist/index.mjs) rather than assumed intact.

Correction after the first CI run

The first head (bca74a46) went red on Test Core, and the failure was this round's:
src/shared/retired-key-migrate-sentence.test.ts refused the new entry's acceptance criterion.

That gate scans every string literal under packages/spec/src (and packages/lint/src) for a
backticked os migrate meta --from N marker and requires the house tombstone sentence at it —
Run os migrate meta --from N to list the mechanical edits for existing sources; apply them by
hand.
The acceptance criterion named the command that way to say the upgrade is finished, so the
scanner read it as a tombstone prescription. The sentence was also split across a string-concatenation
seam, which reconstruct() merges, so both halves were judged as one.

before   … Re-running `os migrate meta --from 17` then reports the migrated stack
           schema-valid instead of asking for the manual changes again

after    … Replaying the same 17 → 18 chain over the edited source then reports the
           migrated stack schema-valid — `schemaValid: true` in `--json`, and the run
           closes with the schema-valid line rather than the manual-changes warning

Derived from the gate, not from other entries by eye, and then checked against them: no file under
migrations/entries/ spells the --from N marker. Nine name the bare command or a flagged variant
(--stored); zero name --from N. The house form belongs to retiredKey() guidance an author meets in
a parse error; a D3 acceptanceCriteria is the consumer's verify loop, and it stays checkable — the
measurement above showed that deleting both nodes makes the same command report schemaValid: true.

⛔ The gate was not weakened, skipped, or given a baseline entry. The sentence moved.

Re-measured on head 95f4fb4b:

pnpm --filter @objectstack/spec test:repo   VERDICT command-exit 0   30 files / 520 tests passed
                                            (CI's red was 1 failed | 29 passed, 2 failed | 518 passed)
pnpm --filter @objectstack/spec test        VERDICT command-exit 0   471 files / 13405 tests passed
pnpm --filter @objectstack/spec check:generated            exit 0
pnpm --filter @objectstack/spec check:migration-registry   exit 0

Why the first round missed it: packages/spec declares two vitest projects — test runs
--project local, test:repo runs --project repo — and the repo-wide corpus scanners live in the
second. A green from test is not a reading about test:repo.

⚠️ One reading to discard, not to repeat: a first test run on the freshly re-created worktree reported
470 passed | 1 skipped. packages/spec/dist did not exist yet, so a dist-dependent file skipped
itself. Building spec and re-running returned the exact 471 / 13405 above — a property of the checkout,
not of the edit.

Unchanged by this correction: the six pin cases, the entry id, and the ADR-0087 disposition move.

验收备注

  • Neither conversion's behaviour changes. The card fences that question off itself — whether a mechanical conversion may delete an authored page node is ruling-grade — and this PR does not open it. One pin case exists specifically to hold that line.
  • The disposition marker in .changeset/15110-…md now reads registered element-filter-and-form-node-refused while the entry is added by this PR rather than that one. The gate never judges that row (its changeset is breaking at base as well as at head, so it is inherited stock and skipped), and both changesets ship in the same unreleased window, so the ledger is truthful at release time. Noted rather than filed: it is a property of moving a disposition across PRs, not a defect.
  • Both changesets in this diff now name the same registration id. That is deliberate — one is the PR that made the node refusable, the other is the PR that supplies its prescription — and the gate reads dispositions per file, with no cross-file uniqueness rule.

Generated by Claude Code


Generated by Claude Code

…tanding

The two ADR-0087 D2 conversions element-filter-removed and
element-form-removed strip every authorable key and deliberately leave the
bare component node, because deleting an authored page node is a layout
decision a mechanical conversion must not make. Once both names joined
RETIRED_PAGE_COMPONENT_TYPES the parse began refusing them, so deleting the
node stopped being optional and became a required upgrade step with no
structured TODO naming it: measured on a stack carrying both nodes, the
replay ended schemaValid false and none of the 115 step-18 todos named
either element.

Adds the ADR-0087 D3 semantic entry element-filter-and-form-node-refused
(one file under migrations/entries/semantic, regenerated into the marked
region by gen:migration-registry), pins it against the replay, and moves the
node-refusal changeset's ADR-0087 disposition onto it.

Neither conversion's behaviour changes.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 12, 2026
@github-actions

github-actions Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 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
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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 — 136 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 758ac40973fd106b7c61fe0ef14bdf9db641c67apackageMentionDocs.

Which tree this was computed on

This run read content/docs from 7acf81f23bd221590c3719ac021404de9910218c — the merge of head 95f4fb4b9d40c640d778fd4d0ad219c751cd2504 into base 758ac40973fd106b7c61fe0ef14bdf9db641c67a, 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 7acf81f23bd221590c3719ac021404de9910218c && git checkout 7acf81f23bd221590c3719ac021404de9910218c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 758ac40973fd106b7c61fe0ef14bdf9db641c67a 95f4fb4b9d40c640d778fd4d0ad219c751cd2504 && git checkout -B drift-repro 758ac40973fd106b7c61fe0ef14bdf9db641c67a && git merge --no-ff 95f4fb4b9d40c640d778fd4d0ad219c751cd2504

node scripts/docs-audit/affected-docs.mjs --json 758ac40973fd106b7c61fe0ef14bdf9db641c67a

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

retired-key-migrate-sentence.test.ts scans every string literal under
packages/spec/src for a backticked `os migrate meta --from N` and requires
the house tombstone sentence at that marker. The new entry's acceptance
criterion named the command that way to say the upgrade is done, so the
scanner judged it as a tombstone prescription and refused it; the sentence
was also split across a concatenation seam, which the scanner reconstructs,
so both halves were read as one.

No entry file in migrations/entries spells that marker: nine name the bare
command or a flagged variant, none names --from N. The acceptance criterion
now says the same checkable thing without the marker -- replay the same
chain over the edited source and the run reports the stack schema-valid
rather than the manual-changes warning.

Regenerated; the pin cases, the entry id and the ADR-0087 disposition are
unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants