Skip to content

refactor(metadata-protocol): remove the four dead 'objects' spelling tolerances (#8862) - #9110

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-8862-remove-dead-objects-tolerance
Aug 16, 2026
Merged

refactor(metadata-protocol): remove the four dead 'objects' spelling tolerances (#8862)#9110
os-zhuang merged 2 commits into
mainfrom
claude/issue-8862-remove-dead-objects-tolerance

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8862

Removes the four dead 'objects' spelling tolerances in packages/metadata-protocol/src/protocol.ts, per the ruling at 5305944673 (option A), taken with the call-site count pin PR #9008 added — never instead of it. That pin is untouched.

The four sites

Found by grepping the bare literal'objects', not === 'objects' — three of the four are spelled !==, and the narrower pattern is what produced a false "stale premise" reading on this card earlier (retracted in the ruling comment).

symbolbeforeafter
applyObjectRegistryMutationrequest.type !== 'object' && request.type !== 'objects'request.type !== 'object'
applyRegistryWriteThroughrequest.type === 'object' || request.type === 'objects'request.type === 'object'
ensureObjectStoragetype !== 'object' && type !== 'objects'type !== 'object'
dropObjectStoragetype !== 'object' && type !== 'objects'type !== 'object'

Deliberately not touched: listCollection('object', 'objects') — a genuine singular/plural collection pair, a different construct. The prose block that described ensureObjectStorage's tolerant guard is now factually stale, so it was moved to the past tense rather than left asserting a shape that no longer exists.

The trace I owed — ensureObjectStorage / dropObjectStorage

PR #9008's ablation covered only the two write-through sites. The other two were folded by #8820 at their producers, and this PR re-derives that rather than inheriting it. Every caller traced to where type is produced:

  • ensureObjectStoragesaveMetaItem (after canonicalizeMetaRequestType) and runPublishSideEffects (args.singularType, folded by promoteDraftForPublish through PLURAL_TO_SINGULAR for both its callers — including publishPackageDrafts, the one ensureObjectStorage's 'objects' limb is now dormant — a spelling-tolerant lookup one layer below the /meta fold #8820 named as the structural hazard because listDrafts returns the stored type verbatim).
  • dropObjectStorage ← both call sites in deleteMetaItem, one passing singularTypeForRepo, the other re-folding PLURAL_TO_SINGULAR[request.type] ?? request.type inline.
  • The four applyRegistryWriteThrough sites re-verified: saveMetaItem, runPublishSideEffects, revertCommit (folds at the call site), rollbackMetaItem (folds via canonicalizeMetaRequestType).

No route can deliver a plural. None of the four is reachable — no fork to report.

⚠️ One correction to the ruling's stated rationale

The ruling says removal flips a hypothetical fifth unfolded caller to "does not register". That is half right, and the PR body should say which half, because the code disagrees with the stronger reading:

  • ✅ It no longer reaches registerObject, so the object is not registered and assertObjectRegistered fails CLOSED — a loud recoverable error replacing a silent one. This is the win the ruling was made for, and it holds.
  • ⚠️ It does not mean nothing is registered. On an unscoped kernel the value now falls through to hydrateOverlayIntoRegistry, which calls registerItem(type, …) under the raw type, exactly like every other overlay kind.

So removal takes the plural out of the object-specific shadowing path; it does not add a second line of defence. Folding at the producer remains the only thing that actually prevents a plural registry key. Both the code comments and the pin header now say this explicitly, so the next reader does not inherit the overstatement.

Pin: the tolerance stays removed

The existing pins (per-route spelling, fold maps, call-site counts) are unchanged and still pass. A new section 4 guards the removal itself, which the existing pins structurally cannot: #9008's ablation 2 proved the per-route cases are green with or without the limb, so they could never catch a regression that puts it back.

Section 4 scans executable text with comments stripped — necessary because this PR deliberately keeps the deleted limbs quoted in the surrounding comments, so a raw-file scan would match its own documentation and could never fail.

Non-vacuity is structural, not a separate control: the expected count is one, not zero — the survivor being the legitimate listCollection pair. A broken stripper reports zero and fails; a returning limb reports two and fails.

Verification

All at final head d61915f2f, after merging origin/main (which brought #9067 in; the removal survived the merge unchanged — re-measured after it, and the suite plus the load-bearing ablation were re-run at this head rather than reported from the pre-merge run).

  • Package suite:pnpm --filter @objectstack/metadata-protocol test112 files / 1567 tests passed.
  • Consumer sweep — downstream direction, verified empirically rather than assumed: --filter '...@objectstack/metadata-protocol' resolves 40 packages including objectql/rest/runtime; the suffix form resolves 14 upstream deps. Ran the three registry-behaviour consumers: objectql 212 files / 3729 tests, rest 120 / 2001, runtime 165 / 2456 — all passed.
  • Ablations, predicted in the pin header before running, each run from the committed state and restored with git checkout from the branch ref, verified byte-identical via git diff --exit-code:
    • 4. ship state → GREEN 112/1567, as predicted.
    • 5. one tolerance limb restored (ensureObjectStorage), re-run at the final head against the whole packageRED, expected [ 'objects', 'objects' ] to have a length of 1 but got 2. Exactly 2 tests failed, both in section 4, with 111 of 112 files still green — the rest of the package cannot tell the two states apart, which is precisely why this section had to be added.
    • 6. stripper neutered to return ''RED, expected [] to have a length of 1 but got 0 — the count-of-one is genuinely non-vacuous.
    • Not a dogfood ablation: section 4 reads protocol.ts source and vitest runs the package from source, so no dist/ rebuild is involved.
  • Gates, re-derived against the actual changed paths with node scripts/pm/dispatch-gates.mjs — all PASS: check:nul-bytes, check:cross-package-test-inputs (+ the ci.yml node form), check:durability-log-level, check:filter-alias-parity, check:meta-type-normalized, check:stack-collection-maps, check:changeset-gate-self-tests, check:objectui-changeset, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset.
  • Ratchet:pnpm check:type-check-debt --re-measure on the fully built closure (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70 successful) → OK, 33 ledger entries re-measured, 1926 raw errors total, none above its recorded number. No ledger entry raised — metadata-protocol has no typecheck script and sits in the ledger, so this was the check that mattered.

Changeset included (patch, @objectstack/metadata-protocol) — the diff touches src/, and while every folding caller sees no behaviour change, the failure mode for a non-folding one does change.

Generated by Claude Code


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/metadata-protocol)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/metadata-protocol)
  • content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/metadata-protocol)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/v9.mdx(via @objectstack/metadata-protocol)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

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.

Third type === 'objects' tolerance in protocol.ts: applyObjectRegistryMutation CONSUMES the tolerant spelling rather than only guarding on it

2 participants

@os-zhuang@claude