fix(objectql,spec): rule the name argument as IMetadataService's effective key (#7378) - #7511
Merged
os-zhuang merged 1 commit intoAug 11, 2026
Conversation
…fective key (#7378) Maintainer ruling of 2026-08-11 on #7378, option (a): the `name` argument is the effective storage key and `data.name` never overrides it. Written into the contract TSDoc on `register` / `get`, and `MetadataFacade` aligned to it. Two of the three measured divergence cells are settled: - the effective key — `MetadataFacade.register` derived its key from the document (`data.name ?? name`, and `data.id` for the generic store), so a document whose own name disagreed with the argument was filed under the document's spelling and `get`/`exists` missed it. It now keys on the argument, reconciling the document to it. - the dropped non-object `data` — a primitive was accepted with no throw and filed under the literal key `undefined`, readable back through no member. It is now boxed as `{ name, content }`, the shape this class's own reads already unwrap. Arrays are boxed too rather than spread into `{ 0: …, 1: … }`. The third cell (the plural `objects` alias) is NOT changed: the alias that decides it is `SchemaRegistry.getItem`/`listItems`' own read-side special-case, and both routes to aligning it are worse than the divergence — narrowing the facade's write side re-opens #6725 for the plural spelling, and removing the registry alias runs against the platform's enforced plural→singular direction (`canonicalMetaType` #4432, `RestServer.metaTypeSingular`, `check:meta-type-normalized`). Its pin survives with that measurement written out, and the question is escalated on #7378. The conformance pins for the ruled rows now assert the ruled behaviour rather than the old `absent` divergence, plus a new `array-data-roundtrips` case and a universal assertion that a document's own name never becomes a second key. Option (c), loud refusal on a disagreement, is recorded as the v18 strictness candidate and deliberately not implemented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KtffEdkV9BA8f4yQXGXP9E
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 109 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
os-zhuang
marked this pull request as ready for review
August 11, 2026 04:50
os-zhuang
deleted the
claude/issue-7378-metadata-facade-roundtrip-ruling
branch
August 11, 2026 05:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #7378
Part of, notFixes, deliberately — and this contradicts the dispatch prompt. The prompt asked forFixes #7378; the standing clause in.claude/agents/os-dev.mdsays a card whose other half is still in the decision box must not be closed by its PR. Cell 2 (the pluralobjectsalias) is returned asneeds_decisionwith the measurement below, so merging this would leave #7378 unanswered on that row. Surfacing the conflict rather than picking a side silently — flip the line toFixesif the PM rules cell 2 out of this card's scope.The ruling this implements
Maintainer ruling recorded on the card 2026-08-11 03:00Z, option (a), quoted verbatim:
Option (c) — loud refusal on
data.name !== name— is recorded as the v18 strictness candidate and is not implemented anywhere here.Premise: verified, all three cells still reproduced at
origin/mainBaseline before any edit, on
origin/main@211abdb: the full@objectstack/objectqlsuite is green (178 files, 3149 tests) with the threeDIVERGENCEpins asserting the facade's divergent answers — i.e. the divergences are exactly as filed, and the mechanism reads as described inmetadata-facade.ts({ ...data, name: data.name ?? name }handed to stores that key on the document).What changed
Contract —
packages/spec(declared on #6017 per declare-and-proceed)Exact files touched in
packages/spec:packages/spec/src/contracts/metadata-service.ts— TSDoc only, onIMetadataService.registerand.get: the argument is the effective key,data.namenever overrides it, including for adatathat is not an object (the member declaresdata: unknown). The parked option (c) is recorded onregisteras the v18 candidate.packages/spec/src/contracts/metadata-service-roundtrip-conformance.ts— the shared case table: the header's "three cases are answered differently, pinned as measured" paragraph now records which rows the ruling settled and which one it did not; thewhyprose on the ruled rows is relabelled from "shipped implementations disagree" to the ruling; and one new case,array-data-roundtrips.No type or export signature changed, so no consumer sweep was implicated;
pnpm --filter @objectstack/spec check:generatedreports all 13 generated artifacts already up to date (nothing to regenerate, nothing hand-edited).Cell 1 — the effective key (RULED, aligned)
MetadataFacade.registerderived its key from the document twice over:data.name ?? name, anddefinition.id ? 'id' : 'name'as the generic store's key field. A document whose ownname(orid) disagreed with the argument was filed under the document's spelling —get/existsmissed it andlistNamesreported the other name. A newtoKeyedDefinitionhelper reconciles the document to the argument before either store sees it, and the generic store is now always keyed onname.Reconciling rather than key-only is forced by the stores, and is stated in the helper's TSDoc:
SchemaRegistry.registerItemreadsitem[keyField]andregisterObjectkeysobjectContributorson the schema's ownname. It is also whatObjectQL.registerMetadataCollectionsalready does on the plugin ingest path (item.name === itemName ? item : { ...item, name: itemName }), so a facade write and a plugin write now agree about identity instead of keying on different fields.Cell 3 — non-object
data(RULED, aligned)A primitive
datawas accepted with no throw and then registered under the literal keyundefined(the registry loggedRegistered setting: undefined), readable back through no member. It is now boxed as{ name, content }— the shape this class's own reads already unwrap (get/listreturnitem?.content ?? item,listNamesreadsitem?.name), so the write half now produces what the read half was already prepared to consume.SchemaRegistrywas never the obstacle: it stores any value, only the key derivation was document-based.Arrays are boxed for the same reason, which is the new
array-data-roundtripscase: an array passes atypeof data === 'object'guard, so the old spread turned[a, b]into{ 0: a, 1: b }— corruption where the primitive row measured loss.Cell 2 — the plural
objectsalias: STOPPED, not aligned (needs_decision)Measured before touching it, per the dispatch's stop clause. Both routes to aligning it are worse than the divergence:
SchemaRegistry's, not the facade's.registry.getItemandregistry.listItemsspecial-case BOTH spellings straight togetObject/getAllObjects(packages/objectql/src/registry.ts, "Special handling for 'object' and 'objects' types"). The facade's ownisObjectTypegoverns only the WRITE side. So narrowingisObjectTypeto the singular — the only facade-local move — would makeregister('objects', n, d)write intometadata['objects']whileget('objects', n)still resolves throughregistry.getItemtogetObject(n)and answers undefined. That isMetadataFacade.register('object', …)writes where neither of its own object reads look #6725 exactly, re-opened for the plural spelling: the pinned row would go green while the silent loss it exists to prevent got worse.canonicalMetaType/PLURAL_TO_SINGULARcanonicalizes every/metarequest type at the protocol boundary (meta overlays: unnormalized type segment creates phantom rows that shadow the code-authored listing and cannot be deleted #4432),RestServer.metaTypeSingulardoes it at the REST boundary, andcheck:meta-type-normalizedis a CI gate that refuses any decision made on the un-normalized:type— three authorization bypasses (/meta 的每个按类型闸门只在单数拼写下生效 —— 复数(PD #3 的规范拼写)整条绕过,含 book audience、app RBAC、dashboard 能力门 #3984, ADR-0057 D10 的 dashboardrequiresService组件门禁在GET /meta/:type/:name的缓存分支(默认路径)被完全跳过 #5881, GET /meta/books/:name(复数拼写)绕过 ADR-0046 §6.7 audience 门禁 —— 缓存分支的 doc/book 排除写的是字面量比较 #6241) came from exactly that.registry.test.tspinslistItems("objects")as a deliberate alias.So the open question is a ruling of its own: does
IMetadataServicekey its type stores on the raw string (the reference semantics) or on the canonical type (what the rest of the platform does)? TheDIVERGENCE_2pin survives, now carrying this measurement and the escalation.To be explicit about the gate the dispatch asked about:
check:meta-type-normalizedpasses on this branch, and it scanspackages/rest/srconly — it does not judge the service contract's stores. Its enforced direction is what conflicts, not its scan surface.The conformance pins now bear the ruled load
The two ruled rows are not "divergence assertion deleted".
key-is-the-name-argument-*moved fromabsentto a newreadable-keyed-by-argumentanswer, so the driver now asserts the document comes back under the argument carrying every other authored key,existsis true, andlistNamesreports the argument exactly once.primitive-data-roundtripsandarray-data-roundtripshave no per-subject entry at all — the facade is held to the table's own reference answer like every other subject. And one new assertion applies to every subject and every readable row: when the written document carries anamethat is not the key being read, that stale spelling must not appear inlistNames— which is what stops a "stored under both keys" implementation from passing the ruled rows for the wrong reason.Strictness-ledger parking for option (c): reported, not forced
The dispatch named
docs/audits/2026-07-unknown-key-strictness-ledger.md. Measured, its scope does not admit this entry: that ledger is the Zod unknown-key posture triage overz.objectsites inpackages/spec(its classification rule is "who writes this schema's input?", its verdicts areauthorable/wire/open/…, andcheck:strictness-ledgerrequires every row to name a spec file whose sites the row judges). Option (c) is a runtime-contract argument-disagreement rule on an interface member — a different axis, with noz.objectsite to be a verdict about, in a machine-checked file that is a known merge-conflict magnet. Per the dispatch's own instruction I did not force it.Parked instead where the next implementer will actually be standing: in the
IMetadataService.registerTSDoc itself, as an explicit "PARKED for v18 consideration, do not implement ahead of that ruling" paragraph next to the rule it would tighten, plus the ruling comment already on #7378. Naming a better home is the PM's call if this one is wrong.Behaviour-affecting consumers (blast radius)
Swept for in-tree constructors and callers of
MetadataFacade.register: there are none. The only in-tree references are the two re-exports (packages/objectql/src/index.ts,.../core.ts), documentation prose inpackages/spec/packages/mcp/plugin-security, and the facade's own tests. Nothing in-tree relies on document-name keying or on the plural alias read-through, so the in-repo blast radius of this change is the conformance driver and the facade's own suite.Out of tree, the class reaches hosts through those two exports. A downstream host that relied on
registerkeying bydata.nameordata.idrather than by the argument it passed will see items move to the argument's key; that is the ruled change and it is stated in the changeset.Changeset:
patchfor both packages, arguedpatch, notminor: the ruling classifies the old facade behaviour as a defect against the contract this repo already declares — 4 of the 5 measured implementations conformed before this PR, and the TSDoc change writes down what the parameter names already implied rather than adding a capability.packages/specgains no new authorable surface (TSDoc plus one conformance case). The counter-argument forminoris real and is why the changeset spells the behaviour change out: an out-of-tree host CAN observe items moving keys. It is not a new feature and it removes silent loss, sopatchwith a loud changeset body is the honest label.Verification
pnpm --workspace-concurrency=2 --filter '@objectstack/objectql^...' build— green.pnpm --filter @objectstack/objectql test— 178 files, 3153 tests passed (baseline onorigin/mainwas 178 / 3149; the +4 are the new array case across the driver's four subjects).pnpm --filter @objectstack/spec test— 374 files, 9803 tests passed (this includes the reference double's own driver replaying the new case).pnpm --filter @objectstack/objectql --filter @objectstack/spec typecheck— green both.data.name ?? nameplus theidkey field) was predicted to turn exactly 4 rows red, all on theMetadataFacadesubject. Measured:4 failed | 62 passed, red onkey-is-the-name-argument-object,key-is-the-name-argument-nonobject,primitive-data-roundtrips,array-data-roundtrips, all under['MetadataFacade'], every other subject untouched. Fix restored from a patch file (nogit stash).check:adr-anchors,check:changeset-gate-self-tests,check:doc-authoring,check:doc-formula-expressions(inpackages/lint, where it lives — there is no root script of that name; its spec-TSDoc half judged 9 examples clean),check:docs-audit-scope,check:durability-log-level,check:engine-double-contract,check:i18n(after building@objectstack/cli, which it requires),check:merge-driver,check:meta-type-normalized,check:nul-bytes,check:release-body,check:spec-parsed-alias, andpnpm --filter @objectstack/spec check:generated(all 13 artifacts already current).Generated by Claude Code