fix(metadata): refuse an ambiguous metadata stem with both paths named, instead of listing one name twice and serving it by extension precedence - #16086
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…t by extension precedence (#14921) Two files sharing one stem across the registered extensions produced one name twice in FilesystemLoader.list() while only the first-precedence file was reachable by any name, and loadMany() returned both bodies. list() and the shared loadMany()/loadManyKeyed() walk now refuse with an ADR-0112 envelope naming both paths and the type; MetadataManager.listNames() and list() propagate it rather than absorbing it into their per-loader degradation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…he dispatcher error-code vocabulary table
The ADR-0112 envelope added in packages/metadata is a new code-stamping site.
Reachability measured from the call graph: MetadataManager.listNames()/list()
re-raise it, listObjects() is list('object'), and domains/mcp.ts awaits
meta.listObjects() with no try on the MCP request path, so it reaches the
dispatcher door. Verdict pending-registration; the ledger entry is the
packages/spec lane's call and is not made here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…string check:doc-authoring — a runtime string reaches authors and operators who cannot resolve a tracker id. The anchors stay in the adjacent // comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…d member MetadataLoaderContract.protocol is a closed union; 'test:' is not in it. The outage CONTROL only needs a loader whose failure carries no ambiguous-stem brand, so it declares 'memory:'. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
📓 Docs Drift CheckThis PR changes 2 package(s): 2 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 — 30 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 237080462ad3f1ebc220387c53e5f54d6aa86d33 && git checkout 237080462ad3f1ebc220387c53e5f54d6aa86d33
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c3fda5fb15cf777523b55b730eba8e3eea8ac16 2aa6373d1330a5c3321acbfd532eadb6ec026e97 && git checkout -B drift-repro 9c3fda5fb15cf777523b55b730eba8e3eea8ac16 && git merge --no-ff 2aa6373d1330a5c3321acbfd532eadb6ec026e97
node scripts/docs-audit/affected-docs.mjs --json 9c3fda5fb15cf777523b55b730eba8e3eea8ac16
|
Docs Drift Check — a verdict per listed pageThe check listed two hand-written pages. Neither is falsified by this diff, so neither is edited. Reasoning per page, against the four shapes that would have made this E3 「已发布必修」 and fix-in-this-PR. 1.
|
| Falsifying shape looked for | Found? |
|---|---|
a statement that listNames() returns a list rather than throwing, or that it is total / safe / never fails |
No. Line 36 is the interface signature declaring listNames as taking a string type and answering a Promise of a string array. A TypeScript signature never encodes throws, and the signature is unchanged — a throwing method still has exactly this type. |
| a documented return contract stated without a refusal case | No. The page states no return contract for listNames beyond the signature — no prose about what a caller gets when a loader cannot answer. |
an example calling listNames() without error handling, presented as the documented usage |
Line 103, const names = await metadataService.listNames('object'); — but it sits in a six-line signature-illustration block where get, list, exists and getObject are equally unhandled. It illustrates addressing, not a failure contract, and it does not become false. |
| anything describing what happens when a loader cannot resolve a name — the old silent first-wins behaviour | No. grep for first-wins / collision / silently / duplicate / listed twice on this page: zero hits. |
The page's only failure-mode prose is lines 113 to 130, and it is explicitly scoped to the singular read: "Both read one item through the registered loaders … load returns null for both 'no loader has this item' and 'every loader failed'". This PR does not touch load() / loadDiagnosed(), so that passage stays true as written.
The page is silent on the plural reads' failure posture — it documented neither the pre-change degradation nor a totality claim. Silence is not falsehood, and the instruction was to fix only what the diff falsifies, so adding a refusal case here would be writing new documentation under a ruling that did not ask for it. Flagged for the maintainer as a genuine gap if they want it filled, but not taken unilaterally in this PR.
2. content/docs/protocol/kernel/metadata-service.mdx — anchor FilesystemLoader · NOT FALSIFIED
Two mentions, both still true:
- line 46, a code comment: "NodeMetadataManager wires up the FilesystemLoader and file watcher" — unchanged, that is still exactly what
NodeMetadataManagerdoes. - line 64, a capability-table row: "Reads/Writes
.json,.yaml,.tsfiles from disk. Primary for development." — unchanged. The registered extension set is untouched by this PR; what changed is what happens when two files under one directory derive the same name, which this row does not describe.
Nothing on the page documents stem derivation, the extension precedence, list()'s output, or any collision behaviour. Its os compile bullet (line 273, "Validates TypeScript metadata and emits dist/objectstack.json") also stays true and is consistent with the os build finding in the PR description — it never claimed a metadata-directory scan.
3. content/docs/releases/v17.mdx — release-owned · READ ONLY, NOT EDITED
Noted only so the record is complete: it mentions FilesystemLoader.list() at line 4329. Release notes are written centrally at release time; this PR's input to them is its changeset. Not read for edit, not edited, and no claim made about whether it is correct.
4. The blind spot the check declared — packages/metadata/src/index.ts yielded no anchor
This diff does change that barrel: it adds four exports. Hand-checked, because the check could not see it.
The decisive fact is measured rather than argued: the Clause-② full-surface ablation compared every declaration file the package publishes and found zero symbols removed and zero signatures narrowed — the one removed line per changed file is the export statement being replaced by a longer one carrying every previous name. A page that names an existing export therefore cannot have been made wrong; at worst it is now incomplete, which is not a falsehood.
Checked by hand anyway: packages/metadata/README.md imports MetadataManager, MemoryLoader, NodeMetadataManager and MetadataPlugin — all still exported. No page under content/docs/ enumerates this package's exports as a closed set. content/docs/api/error-catalog.mdx is generated from the spec vocabulary, and AMBIGUOUS_METADATA_STEM is deliberately not registered there yet (see the pending-registration row), so its absence is correct and check:error-status-conformance is green on exactly that reconciliation.
One near-miss worth recording
A repo-wide sweep for documented stem behaviour hit content/docs/ui/doc-pages.mdx and content/docs/references/system/doc.mdx, both of which say "the filename stem becomes the doc name". Out of scope, verified not by reading but by extension: those are src/docs/*.md files collected by collectAndLintDocs, and .md is not in FilesystemLoader's registered extension set (.json, .yaml, .yml, .ts, .js), so resolvableNameForPath() returns null for them and no collision among them is reachable by this refusal.
Generated by Claude Code
Rulings on the three open questions — A, A, A. And
|
| reading | result |
|---|---|
NodeMetadataManager in packages/cli |
0 files |
| CONTROL — the same symbol elsewhere | 19 files ⇒ the symbol exists and the grep finds it |
CONTROL 2 — loadConfig in packages/cli |
27 files ⇒ the grep genuinely reaches that path |
⇒ os build is an alias of os compile; compile.ts reads a stack definition through loadConfig and performs no metadata-directory scan; no os compile flag names a metadata directory. os build cannot surface this refusal because it never constructs a FilesystemLoader.
⛔ Implementing it anyway would have meant inventing a build-time metadata walk — a new feature with its own unruled contract questions (which roots does os build walk? does a missing metadata dir become an error?) — smuggled in under a ruling that did not ask for it. ⭐ The round refusing to do that, and saying so, is the correct behaviour and the reason this is cheap to settle.
Recorded for the director seat / maintainer, since the ruling is theirs: clauses 1 and 2 are implemented and all three pins are in; clause 3's premise does not hold in this tree, and the refusal does reach live doors — the dispatcher one via domains/mcp.ts, and any MetadataPlugin-hosted server where NodeMetadataManager registers the loader. ⇒ If build-time coverage is wanted, it is option B as a separate card, which this seat will file. ⛔ Not a rider on this PR.
Q2 — AMBIGUOUS_METADATA_STEM registration — A: leave it pending-registration.
Registering the code pulls packages/spec, a second changeset and a docs-generation step into a domain:engine card whose ruling named only packages/metadata. And dispatcher-error-vocabulary.ts states in its own header that registering a code is a contract-semantics change owned by the packages/spec lane — so doing it here would be this lane deciding another lane's contract.
⭐ The decisive point is that the gate's own design treats a pending row as the correct landing state, and two such rows already sit on main. ⇒ This is the shipped convention, not a shortcut. check:dispatcher-error-vocabulary is green on exactly that reconciliation.
Q3 — the docs gap — A for this PR; B as a docs-only card, which this seat will file.
⭐ The round's reasoning is right and worth keeping verbatim: the page is silent on the plural reads' failure posture, and silence is not falsehood. Writing a failure contract the page never carried is new documentation, and — decisively — it would have to describe behaviour older than this card to be coherent (the pre-existing per-loader degradation, not just the new refusal). ⇒ That cannot be a rider on a ruled engine fix.
⭐ Two things this round did that this seat is adopting
1. Zone 2.1 — the reading that could have stopped the round — was discharged with FOUR readings, each with its own firing control. Not one grep: a directory walk (0 stem collisions over 526 dirs / 7770 files, with a positive-control fixture reporting exactly 1), an independent git ls-files pass over the same corpus with a different instrument, tracked .yaml/.yml outside CI and workspace config (0 of 38), and sources writing .yaml/.yml (10, all pnpm-workspace.yaml or workflow files inside gate self-tests — control: the same pattern for .json finds 60). ⇒ A "nobody relies on this" claim is only as good as the instruments that could have found someone, and four instruments with four controls is what makes it a reading.
2. d.ts ablation ran while an uncommitted fix sat in packages/metadata; its restore leg (git checkout HEAD -- packages/metadata) reverted that fix to a HEAD predating it, and a following git add -A committed the reversion. It was caught by a typecheck, and — this is the part that makes it trustworthy — the blast radius was then measured (git diff c105ace89..f8f02d2be -- packages/metadata EMPTY, so the protocol literal was the only casualty) rather than assumed, and the whole union was re-run at the true final commit.
⇒ New standing rule: ⛔ never run an ablation against a dirty tree, and after any restore, measure the blast radius rather than trusting the restore was inert. ⭐ git checkout HEAD -- <dir> is not a scalpel — it reverts everything uncommitted under that path, including work the ablation never touched.
⛔ Nothing changes about the PR's state: Clause-②: yes on both limbs (limb 1 additively — zero symbols removed, zero signatures narrowed; limb 2 outright — a tree accepted today is refused), needs:contract-review verified on both carriers, PR stays draft. ⛔ 免复核不放行.
Generated by Claude Code
Contract review (clause ②) — PASS — PR #16086 at head
|
| # | claim | reading | verdict |
|---|---|---|---|
| 1 | One shared derivation, resolvableNames(), is the single refusal site, used by list() and by the loadMany()/loadManyKeyed() walk before limit truncates |
Diff read: the walk collects all matched files first, calls resolvableNames, then reads; list() calls it outside its own catch so the method's degradation cannot eat the refusal. Deterministic (first colliding name in sorted order), overlapping-pattern dedup via Set. |
correct |
| 2 | ADR-0112 envelope: code AMBIGUOUS_METADATA_STEM, status 500, both paths + type in the message and structurally on paths/type/stem |
Read; 500 reasoning (well-formed request, deployment's own tree at fault, not transient) holds. | correct |
| 3 | MetadataManager.listNames() and list() propagate on a Symbol.for brand predicate, everything else still degrades |
Diff read at both seams; the discrimination is what keeps a storage outage degrading (pinned as controls). Brand over instanceof for duplicate-copy graphs — same shape as isAuthzStoreUnavailableError. |
correct |
| 4 | Zone 2.1 discharged: no in-repo tree relies on the precedence — four readings, four firing controls (0 collisions over 7,770 files / 526 dirs; positive fixture → exactly 1) | The ruling's one flip condition, measured rather than asserted. | correct |
| 5 | dispatcher-error-vocabulary.ts row: pending-registration, door dispatcher, reachability derived as one uncaught hop via domains/mcp.ts listObjectSummaries |
Consistent with the gate's design (two such rows already on main); registering the code widens ApiErrorSchema.code and is the spec lane's — correctly not folded. |
correct |
| 6 | Three ruled pins present (refused with both paths; single file per stem still listed and loadable; two-body symptom gone), 17 cases, ablation 8 red / 9 green as predicted | Read the it list; controls include the registered-set reversal (dual.json+dual.js clean by default, ambiguous once javascript is registered). |
correct |
| 7 | Docs: two listed pages judged silent-not-false; releases page read only | Accepted; the plural-reads failure-posture gap is a docs-only card (seat's Q3 = B), not a rider. | accepted |
② semver
@objectstack/metadata minor with a **BREAKING** banner naming the remedy and <!-- adr-0087: not-required (no-migration-prescription) --> with its reasoning (a filesystem layout, not a stored document; which sibling an author meant is intent no entry can decide). Exactly what the ruling ordered.
③ Boundary flags — one clause of the ruling adjudicated here
- "
os buildsurfaces it" — accepted as shipped, no rider. The seat measured (with two firing controls) thatos buildis an alias ofos compile, which reads the stack throughloadConfigand never constructs aFilesystemLoader; the clause rests on a build-time metadata walk this tree does not have. Inventing one would be a new feature with its own unruled questions smuggled under a ruling that did not ask for it. The refusal does reach live doors (the MCP dispatcher hop; anyMetadataPlugin-hosted server). This seat rules the execution clause satisfied by the doors that exist; build-time coverage, if wanted, is the seat's separate option-B card. Recorded for the maintainer as an FYI in the director's next brief, not as a new decision box. - The self-reported ablation incident (a
git checkout HEAD -- <dir>restore reverting an uncommitted fix, caught by typecheck, blast radius measured empty, union re-run at the true head) is the right handling; the standing rule the seat adopted (never ablate a dirty tree; measure the blast radius after a restore) is worth carrying into the skill.
Evidence and landing
Checks on 2aa6373d: 31 success / 6 skipped / 0 red; check-governed-merges --test on the 7 paths: 0 hits — ordinary queue landing. Clearing, same stroke: needs:contract-review off #14921 and PR #16086 with provenance; then check-clause2-carriers --pair 16086 ⇒ ready + auto-merge SQUASH from this seat.
Generated by Claude Code
Fixes #14921
Implements the maintainer ruling on this card (director seat, 2026-09-05, decision batch #43 item 4, verbatim reply 「同意」): option 1 —
FilesystemLoader.list()refuses an ambiguous stem loudly. Two files sharing one stem across the registered extensions is an authoring error, reported at list time with both paths named, never served by precedence. Options 2 (warnand keep the precedence) and 3 (extension in the name) are not taken.The invariant restored
What is listed is what is loadable.
FilesystemLoaderderives a metadata name by stripping a flat file's extension, and resolves a name back under a fixed precedence (.json→.yaml→.yml→.ts→.js). Withobject/twin.jsonandobject/twin.yamlboth present,list()answered['twin', 'twin']— one name twice — whiletwin.yamlwas addressable through no name at all andloadMany()returned both bodies. The listed set and the addressable set had stopped being the same set.MetadataManager.listNames()unions loader output into aSet, so the duplicate collapsed there and the count discrepancy went with it. That is why a clean-lookinglistNames()was never evidence the collision had been absorbed: the file stayed unreachable either way.Zone 2.1 — the reading that could have stopped the round
The ruling names one confidence gap: confirm no example or fixture tree relies on the precedence as a deliberate override. If an author were using it that way, option 1 would be a breaking change to a working mechanism and this PR should not exist.
Measured: no tree in this repository relies on it, and none can. Four readings, each with a firing control:
.json/.yaml/.yml/.ts/.jsobject/twin.json+object/twin.yaml+object/solo.json) → scanner reported exactly 1 collisiongit ls-filespass over the same question.yaml/.ymlfiles outside CI and workspace config.github/workflowsandpnpm-workspace.yaml.yaml/.ymlfile at allpnpm-workspace.yamlor.github/workflows/*.ymlinside a gate self-test.jsonfinds 60 — the grep firesNo doc, ADR or comment describes the extension precedence as an author-facing override; the only prose about it is
findFile()'s own note that the order exists so registration order cannot change which file a name opens. Nothing to break. This is also the grade-p3fact the ruling relies on: the zero-instance population is what makes this narrowing cheap rather than expensive — no existing tree goes red, so there is almost no migration account.What changed
packages/metadata/src/loaders/ambiguous-metadata-stem.ts(new) —AmbiguousMetadataStemError, ADR-0112 envelope:codeAMBIGUOUS_METADATA_STEM,status500, with every colliding path and the metadata type in the message and carried structurally onpaths/type/stem. Exported from the package root entry with a branded predicateisAmbiguousMetadataStemError.filesystem-loader.ts— one sharedresolvableNames()derives the reported names and is the single place the refusal lives.list()calls it outside its owncatch, deliberately: a failing walk is a degradation this method has always swallowed into[], and leaving the throw inside would have its own diagnostic eat the refusal. The sharedloadMany()/loadManyKeyed()walk calls it too, in front of the whole matched set — beforelimittruncates — so whether a tree is refused cannot depend on how many items the caller asked for.metadata-manager.ts—listNames()andlist()propagate rather than absorb. The discrimination is the point: a storage outage still degrades to a short-but-served list exactly as before, because the seams re-raise on a brand rather than rethrowing everything. Absorbing this one would drop every item the loader holds into adegradedpartial set while the server reported healthy — an authoring error rendered as a storage outage, which is the one reading it must never get.dispatcher-error-vocabulary.ts— the new code is a new code-stamping site, socheck:dispatcher-error-vocabularyrequires it classified. Verdictpending-registration, doordispatcher, reachability derived from the call graph as a single uncaught hop:listObjects()islist('object'), andpackages/runtime/src/domains/mcp.tslistObjectSummariesawaitsmeta.listObjects()with notryon the MCP request path. Registering the code widensApiErrorSchema.codeand is thepackages/speclane's call, deliberately not made here — this row is that batch's input.The three pins the ruling requires
All three, in
filesystem-loader-ambiguous-stem.test.ts(17 cases):code,status) and that both absolute paths and the type appear in the message, plus the structuralpathsarray sorted. Not a baretoThrow(), which would stay green against anyErrorat all.exists(),stat()andload().loadMany()two-body symptom gone —loadMany(),loadManyKeyed()andloadMany({ limit: 1 })all refuse rather than answering with two bodies. Refused, not de-duplicated: picking a winner is option 2, which the ruling declined.Controls: the refusal reads this instance's registered serializer set, not a second hard-coded list —
dual.json+dual.jsis clean under the default format set and the same tree on disk becomes ambiguous oncejavascriptis registered. A nested file sharing a flat name is not a collision. The refusal is scoped to the type directory holding it. And a storage outage still degrades at both manager seams.Ablation. Direction predicted in writing before running: 8 red / 9 green. Measured: 8 failed | 9 passed (17) — exact. One shell: the implementation file was reverted to the branch base, the mutation proven on disk before measuring (blob changed; both anchors counted 0), and the restore proven after (blob equals the HEAD blob,
git diff HEADempty). No rebuild was needed and none is claimed: the subject resolves through relative source imports inside its own package, not through a dependency'sexports, sodistis not in the resolution path for this suite.Clause-② — both limbs, measured separately
The ruling settles the disposition (
yes); it does not settle the obligation to measure. Instrument: build at head, swap every changed source back to the branch base, rebuild, diff every declaration file the package publishes — all fiveexportsentry points across both module systems, resolved from theexportsmap andfiles[], not from the root barrel. Both preflight legs fired (marker present at head, absent at base, present again after restore) and the restore was proven byte-exact.Limb 1 — does any exported symbol or signature move? YES, additively. Of the 10 published declaration files, 4 changed and 6 are byte-identical:
dist/index.d.ts+.d.ctsdist/node.d.ts+.d.ctserrors·migrations·view-containerThe single removed line in each is the export statement being replaced by a longer one carrying every previous name plus four new ones. Across the whole published surface: zero symbols removed, zero signatures narrowed, and the entry-point file set is unchanged. A pure widening, which is the
minorfloor the ruling already set.Limb 2 — is any request newly accepted or rejected? YES, newly rejected. A metadata tree with two files sharing one stem across the registered extensions is accepted today (listed twice, first served) and is refused after this change. That is limb 2 outright, and it is why this PR is draft with
needs:contract-reviewon both carriers.One clause of the ruling I could not satisfy, reported rather than invented
The ruling's execution spec says "
os buildsurfaces it." Measured:os buildnever constructs aFilesystemLoaderand never enumerates a filesystem metadata tree.os buildis an alias ofos compile;compile.tsreads a stack definition throughloadConfig, which performs no metadata-directory scan, and the stringNodeMetadataManagerappears zero times anywhere inpackages/cli(control: the same grep finds it in 10 files elsewhere, so the zero is real). Nothing inos compile's flag surface names a metadata directory either.The refusal does reach a live door — the dispatcher one traced above, and any
MetadataPlugin-hosted server, whereNodeMetadataManagerregisters the loader. Makingos builditself walk a metadata tree would be a new feature, not this repair, so it is not done here and is flagged for the maintainer instead.Verification
Union measured at head
2aa6373d1, after the final commit:pnpm --filter @objectstack/metadata exec vitest run— 48 files / 735 tests passedpnpm --filter @objectstack/metadata typecheck— exit 0 (it compiles the test files: an earlier error in this very suite was reported atfilesystem-loader-ambiguous-stem.test.ts:328, so the coverage claim is measured, not assumed)pnpm --filter @objectstack/runtime exec tsc --noEmit— exit 0node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackfrom the final commit, provenance line checked; adding the runtime path introduced no new family. Green, each quoted from its own verdict line:check:dispatcher-error-vocabulary("OK — 66 unregistered code-stamping site(s), all classified; 3 awaiting a ledger entry"),check:doc-authoring,check:error-code-casing,check:error-status-conformance,check:nul-bytes("scanned 7766 text file(s) ... no raw ASCII control bytes"),check:adr-0087-registration("1 declared-breaking changeset(s), each carrying an ADR-0087 disposition"),check:empty-changeset,check:changeset-gate-self-tests,check:objectui-changeset,check:engine-double-contract,check:where-matcher,check:test-source-alias,check:cross-package-test-inputs,check:durability-log-level,check:published-files,check:type-check-coverage,check:logger-receiver-detach,check:objectql-double-limit,check:slot-lookup,check:org-identifier,check:page-declaration-shape,check:type-source-resolution,check:single-claim-paths,check:partof-closing-keyword.check:published-readme-exportsreturned exit 3 = NOT MEASURED, not a pass and not a finding: it needs a whole-repo build this worktree does not have. Left to CI.Changeset
@objectstack/metadataminor with a BREAKING banner naming the remedy (delete or rename the duplicate) and an ADR-0087 disposition ofno-migration-prescription— the affected artifact is a filesystem layout, and which of two sibling files an author wants kept is intent no migration entry can decide.Related cards, out of scope here and left open: #14486 is the adjacent half — a listed name pointing at no file, the opposite direction. #14341 is the keyed-item rule that already documents the silent first-wins collision. Neither is addressed by this PR.
Generated by Claude Code