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

.changeset/tidy-pugs-shave.md

@@ -0,0 +1,11 @@
+---
+'@objectstack/metadata-protocol': patch
+---
+
+Remove the four dead `'objects'` spelling tolerances in the metadata protocol's object registry and storage seams.
+
+`applyObjectRegistryMutation`, `applyRegistryWriteThrough`, `ensureObjectStorage` and `dropObjectStorage` each admitted a plural `'objects'` type key, and the first of them *registered under it* — the spelling-tolerant-lookup shape `canonicalMetaType`'s header rejects, and the one that previously let a plural registry entry shadow an entire code-authored listing.
+
+All four are unreachable: every producer folds the type through `PLURAL_TO_SINGULAR` / `canonicalMetaType` before these seams see it. No behaviour changes for any caller that folds — which is all of them. What changes is the failure mode of a future caller that does *not* fold: it no longer silently registers an object under a plural key, so `assertObjectRegistered` fails closed with a loud, recoverable error instead.
+
+Folding at the producer remains the rule; these guards were never a second line of defence.

packages/metadata-protocol/src/protocol.object-registry-write-through-spelling.test.ts

@@ -1,30 +1,38 @@
 // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
 //
 // [#8862] Every `applyRegistryWriteThrough` route registers an object under the
-// CANONICAL SINGULAR key — the `'objects'` limb below it is dormant.
+// CANONICAL SINGULAR key — and the `'objects'` limb below it is now REMOVED.
+//
+// This file was written in two halves, and reads as one only if you know that.
+// The MEASUREMENT half proved the limb dormant (below). The REMOVAL half then
+// deleted it and added section 4, which keeps it deleted. Everything the
+// measurement half says about the limb is therefore in the PAST TENSE — the
+// code it quotes no longer exists, and is quoted because it is what the
+// verdict was about.
 //
 // ---------------------------------------------------------------------------
 // The question this file answers, and why it had to be measured
 // ---------------------------------------------------------------------------
-// `applyObjectRegistryMutation` does not merely ADMIT a plural type key, it
-// consumes one:
+// `applyObjectRegistryMutation` did not merely ADMIT a plural type key, it
+// consumed one — this is the code as it stood before the removal:
 //
 // if (request.type !== 'object' && request.type !== 'objects') return;
 // this.engine.registry.registerItem(request.type, request.item, 'name');
 //
-// The spelling that came in is the spelling the registry entry is minted
+// The spelling that came in was the spelling the registry entry got minted
 // under. `canonicalMetaType`'s header names that exact shape as having already
 // cost the repo a real bug: one plural-spelled read minted a plural registry
 // entry, `listItems('actions')` stopped being empty, and the singular fallback
 // that had been supplying the code-authored items never ran again — so one
 // overlay row shadowed an entire code-authored listing and survived the DELETE
 // meant to lift it.
 //
-// So the limb is either a live registry-shadowing defect or dead tolerance,
-// and which one it is depends entirely on whether any caller can deliver
+// So the limb was either a live registry-shadowing defect or dead tolerance,
+// and which one it was depended entirely on whether any caller could deliver
 // `'objects'`. #8862 was filed WITHOUT that measurement, deliberately: it came
 // out of #8820, the card where an unmeasured reachability claim was the whole
-// defect. This file is that measurement.
+// defect. This file is that measurement — and, in section 4, the guard that
+// the removal it licensed stays done.
 //
 // ---------------------------------------------------------------------------
 // The trace, measured on `origin/main` — all four routes fold at the producer
@@ -54,36 +62,59 @@
 // call graph.
 //
 // ---------------------------------------------------------------------------
-// What this file is FOR, given the limb is dead
+// What this file is FOR, now that the limb is gone
 // ---------------------------------------------------------------------------
-// Dormancy proven once is dormancy unenforced. Nothing in the type system says
-// a fifth caller must fold — the parameter is a bare `type: string`, exactly
-// the shape that let #8820's hazard sit unnoticed. Two guards close that:
+// Removal is not self-enforcing either. Nothing in the type system says a
+// fifth caller must fold — the parameter is a bare `type: string`, exactly
+// the shape that let #8820's hazard sit unnoticed. Three guards close that
+// (the third added by the removal half):
 //
 // • the per-route cases pin the SPELLING that reaches the registry, so a
 // route that stops folding mints `'objects'` here and fails loudly rather
 // than silently shadowing a code-authored listing;
 // • `applyRegistryWriteThrough` call-site COUNT is pinned, so a fifth caller
-// cannot be added without a human re-reading the trace above.
+// cannot be added without a human re-reading the trace above;
+// • [removal half] section 4 pins that no tolerance limb comes BACK — the
+// guard the count pin cannot give, since a fifth caller and a re-widened
+// predicate are different regressions.
 //
 // The fold maps are pinned too: they are an external dependency of this
-// verdict, living in `@objectstack/spec`, and the dormancy dies the day either
-// stops resolving `objects`.
+// verdict, living in `@objectstack/spec`, and the removal's safety dies the
+// day either stops resolving `objects`.
+//
+// ⚠️ One thing removal did NOT buy, recorded because the obvious reading
+// overstates it: a hypothetical fifth, unfolded caller no longer registers an
+// OBJECT (so `assertObjectRegistered` fails CLOSED — a loud error replacing a
+// silent one), but on an unscoped kernel the value still falls through to
+// `hydrateOverlayIntoRegistry`, which registers under the raw type like every
+// other overlay kind. Removal took the plural OUT of the object-specific
+// shadowing path; it did not add a second line of defence, and folding at the
+// producer remains the only thing that actually prevents a plural key.
 //
 // ---------------------------------------------------------------------------
-// Ablation directions, predicted BEFORE running (results in the PR body)
+// Ablation directions, predicted BEFORE running (results in the PR bodies)
 // ---------------------------------------------------------------------------
+// Measurement half (#9008), against the tree that still HAD the limb:
 // 1. Ship state -> GREEN
 // 2. `'objects'` limb deleted from BOTH
 // `applyRegistryWriteThrough` and
 // `applyObjectRegistryMutation` -> GREEN
 // (the limb is dead once the producers fold — this is the
-// evidence that licenses its removal as dead tolerance)
+// evidence that licensed its removal as dead tolerance)
 // 3. `applyObjectRegistryMutation`'s guard inverted to accept
 // ONLY `'objects'` -> RED
 // (non-vacuity control: proves the cases really observe
 // this seam registering, rather than passing for want of
 // an assertion)
+//
+// Removal half (#8862), against the tree with the limb GONE. ⚠️ Direction 2
+// above is why the per-route cases alone could NOT have caught a bad removal —
+// they are green either way. Section 4 is what carries this half:
+// 4. Ship state (limb removed) -> GREEN
+// 5. Any one tolerance limb restored -> RED in §4
+// (the regression this half exists to prevent)
+// 6. Comment stripper neutered to return '' -> RED in §4
+// (non-vacuity: the expected count is ONE, not zero)
 
 import { describe, expect, it } from 'vitest';
 import { readFileSync } from 'node:fs';
@@ -458,3 +489,102 @@ describe('[#8862] the `applyRegistryWriteThrough` caller set stays closed', () =
  expect(callSites).toHaveLength(1);
  });
 });
+
+// ═══════════════════════════════════════════════════════════════════════════
+// 4. [#8862 removal half] The tolerance stays removed
+// ═══════════════════════════════════════════════════════════════════════════
+//
+// The four `'objects'` tolerance limbs were deleted once the dormancy above
+// was proven. This block keeps them deleted.
+//
+// It scans EXECUTABLE text — comments stripped — for a reason that is not
+// cosmetic: the removal commit deliberately kept the deleted limbs QUOTED in
+// the surrounding comments (a reader needs to see what was removed and why),
+// so a scan of the raw file would match its own documentation and could never
+// go red. Stripping is what makes this pin capable of failing.
+//
+// ⭐ NON-VACUITY IS STRUCTURAL, not a separate control case. The expected count
+// is ONE, not zero — the surviving occurrence is `listCollection('object',
+// 'objects')`, a genuine singular/plural COLLECTION PAIR and a different
+// construct entirely (it names two registry collections; it does not tolerate
+// a spelling). So a stripper that silently blanked the file would report zero
+// and fail, and a returning tolerance limb reports two and fails. Measured
+// both directions on the removal commit: `origin/main` before it scored FIVE
+// (the pair + the four limbs), the tree after it scores ONE.
+//
+// ⛔ Do not "fix" a failure here by relaxing the count. If a plural spelling
+// genuinely has to reach one of these seams again, the answer is the same one
+// #8820 and #8862 both landed on: fold at the PRODUCER. A tolerant predicate
+// one layer below a folding boundary is the shape `canonicalMetaType`'s header
+// rejects, and the shape that already cost this repo one registry-shadowing
+// bug.
+
+/**
+ * Remove comments while preserving string literals, so a scan can tell code
+ * from prose ABOUT that code. Handles `//`, block comments, and the three
+ * quote forms with escapes.
+ */
+function executableTextOf(src: string): string {
+ let out = '';
+ let i = 0;
+ const n = src.length;
+ while (i < n) {
+ const c = src[i];
+ const d = src[i + 1];
+ if (c === '/' && d === '/') {
+ while (i < n && src[i] !== '\n') i++;
+ continue;
+ }
+ if (c === '/' && d === '*') {
+ i += 2;
+ while (i < n && !(src[i] === '*' && src[i + 1] === '/')) i++;
+ i += 2;
+ continue;
+ }
+ if (c === "'" || c === '"' || c === '`') {
+ const quote = c;
+ out += c;
+ i++;
+ while (i < n) {
+ if (src[i] === '\\') {
+ out += src[i] + (src[i + 1] ?? '');
+ i += 2;
+ continue;
+ }
+ out += src[i];
+ if (src[i] === quote) { i++; break; }
+ i++;
+ }
+ continue;
+ }
+ out += c;
+ i++;
+ }
+ return out;
+}
+
+describe('[#8862] the `objects` tolerance stays removed', () => {
+ const code = executableTextOf(readFileSync(
+ fileURLToPath(new URL('./protocol.ts', import.meta.url)),
+ 'utf8',
+ ));
+
+ it('has exactly one executable `objects` literal — the collection pair, not a tolerance', () => {
+ const occurrences = code.match(/'objects'/g) ?? [];
+ // ONE, not zero: see the non-vacuity note above. Zero means the
+ // stripper broke; two or more means a tolerance limb came back.
+ expect(occurrences).toHaveLength(1);
+ const lines = code.split('\n').filter((l) => l.includes("'objects'"));
+ expect(lines).toHaveLength(1);
+ expect(lines[0]).toContain("listCollection('object', 'objects')");
+ });
+
+ it('none of the four seams admits a plural any more', () => {
+ // The narrowed forms, pinned positively so a reformat that defeats the
+ // count above still has to keep these honest.
+ expect(code.match(/request\.type !== 'object'\) return;/g) ?? []).toHaveLength(1);
+ expect(code.match(/request\.type === 'object'\) \{/g) ?? []).toHaveLength(1);
+ // `ensureObjectStorage` + `dropObjectStorage`.
+ expect(code.match(/\n\s*if \(type !== 'object'\) return;/g) ?? []).toHaveLength(2);
+ });
+});

packages/metadata-protocol/src/protocol.ts

@@ -11158,7 +11158,23 @@ export class ObjectStackProtocolImplementation implements
  * re-hydration already writes for the same rows.
  */
  private applyObjectRegistryMutation(request: { type: string; name: string; item?: any; packageId?: string | null }): void {
- if (request.type !== 'object' && request.type !== 'objects') return;
+ // [#8862] The `&& request.type !== 'objects'` limb that stood here is
+ // GONE. It did not merely admit a plural, it registered under one —
+ // `registerItem` below mints the entry under the spelling that arrived
+ // — which is the shape {@link canonicalMetaType}'s header blames for a
+ // real prior bug (one plural read minted a plural entry and shadowed an
+ // entire code-authored listing). It was measured DORMANT in #8862: the
+ // sole caller {@link applyRegistryWriteThrough} has four, and every one
+ // folds at its producer. Removing it makes a hypothetical fifth,
+ // unfolded caller fail CLOSED — it does not register, and
+ // `assertObjectRegistered` raises loudly — instead of silently minting
+ // `registerItem('objects', …)`. What keeps a fifth caller from being
+ // added unexamined is the call-site count pin in
+ // `protocol.object-registry-write-through-spelling.test.ts`, not this
+ // guard; the two are a pair.
+ if (request.type !== 'object') return;
+ // `request.type` is provably `'object'` on the line above, so this
+ // mints the canonical key rather than whatever spelling arrived.
  this.engine.registry.registerItem(request.type, request.item, 'name');
  try {
  const layer = this.classifyObjectContribution(request.name, request.packageId);
@@ -11473,7 +11489,28 @@ export class ObjectStackProtocolImplementation implements
  /** The row's org scope — `null` for an env-wide row. [#6602] */
  organizationId: string | null;
  }): void {
- if (request.type === 'object' || request.type === 'objects') {
+ // [#8862] The `|| request.type === 'objects'` limb is GONE — this
+ // branch admitted the plural that {@link applyObjectRegistryMutation}
+ // then consumed. All four call sites of this method fold at their
+ // producer (traced in
+ // `protocol.object-registry-write-through-spelling.test.ts`), so the
+ // limb was dead.
+ //
+ // ⚠️ What removal does and does NOT buy, stated precisely because the
+ // obvious reading overstates it. A hypothetical fifth, UNFOLDED caller
+ // passing `'objects'` no longer takes this branch — so it no longer
+ // reaches `registerObject`, and the object is simply not registered:
+ // `assertObjectRegistered` fails CLOSED, a loud recoverable error in
+ // place of a silent one. That is the win. It does NOT mean nothing is
+ // registered at all: on an unscoped kernel the value falls through to
+ // {@link hydrateOverlayIntoRegistry}, which registers under the RAW
+ // type like every other overlay kind. So the plural is no longer an
+ // object-specific hazard minting a shadow OBJECT — it is merely the
+ // same general "producers must fold" contract every other metadata
+ // type already lives under. Folding at the producer stays the rule;
+ // this guard is not a second line of defence and must not be written
+ // as one.
+ if (request.type === 'object') {
  // NOT org-gated, deliberately: an `object` is `allowOrgOverride:
  // false` (ADR-0005) and its physical TABLE is env-wide, so the
  // registry entry backing it is env-wide too — `assertObjectRegistered`
@@ -11775,7 +11812,16 @@ export class ObjectStackProtocolImplementation implements
  * simply no-op, and a sync failure must not abort the publish.
  */
  private async ensureObjectStorage(type: string, name: string): Promise<void> {
- if (type !== 'object' && type !== 'objects') return;
+ // [#8862] The `&& type !== 'objects'` limb is GONE. Both call sites
+ // hand over a folded value — `saveMetaItem` after
+ // `canonicalizeMetaRequestType`, and `runPublishSideEffects` as
+ // `args.singularType`, which `promoteDraftForPublish` folds through
+ // `PLURAL_TO_SINGULAR` for BOTH its callers (including
+ // `publishPackageDrafts`, the one #8820 named as the structural
+ // hazard because `listDrafts` returns the draft row's stored type
+ // verbatim). Guard-only either way: `type` never reaches
+ // `syncObjectSchema`, which takes `name` alone.
+ if (type !== 'object') return;
  try {
  await this.engine.syncObjectSchema(name);
  } catch (err: any) {
@@ -11830,7 +11876,14 @@ export class ObjectStackProtocolImplementation implements
  * reclaimed by the next sync/drop rather than blocking the delete.
  */
  private async dropObjectStorage(type: string, name: string): Promise<void> {
- if (type !== 'object' && type !== 'objects') return;
+ // [#8862] The `&& type !== 'objects'` limb is GONE. Both call sites
+ // are in `deleteMetaItem` and fold at the producer: one passes
+ // `singularTypeForRepo` (`PLURAL_TO_SINGULAR[request.type] ??
+ // request.type`), the other re-folds the same expression inline.
+ // Guard-only: `type` never reaches `dropObjectSchema`, which takes
+ // `name` alone. {@link shouldDropStorage} already folded before
+ // deciding, so this guard was answering a question settled upstream.
+ if (type !== 'object') return;
  try {
  await this.engine.dropObjectSchema(name);
  } catch (err: any) {
@@ -13397,11 +13450,17 @@ export class ObjectStackProtocolImplementation implements
  // `type = 'view'` missed a publish addressed `/meta/views/…`.
  //
  // ⚠️ `ensureObjectStorage` is NOT in that list, because it answered both
- // spellings identically before this fold and after it: it opens
+ // spellings identically before this fold and after it: it used to open
  // `if (type !== 'object' && type !== 'objects')`, a spelling-tolerant
  // lookup one layer down — the shape {@link canonicalMetaType}'s header
  // rejects.
  //
+ // [#8862] That tolerant limb is now GONE — the guard reads
+ // `if (type !== 'object')`. The paragraph is kept in the past tense
+ // rather than deleted because the reasoning below is what LICENSED the
+ // removal, and a reader arriving at the fold still needs to know why
+ // the helper was never a second line of defence.
+ //
  // [#8820] This paragraph used to justify that with "both of its call
  // sites stand behind a fold". That reason was FALSE. Counting the
  // helper's own two call sites is not the reachability question: the