From fd6a750bc5a14ec27d6104e6e62f1e287e5fe17e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 04:16:09 +0000 Subject: [PATCH] fix(objectql,spec): rule the `name` argument as IMetadataService's effective key (#7378) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01KtffEdkV9BA8f4yQXGXP9E --- .../metadata-facade-roundtrip-ruling.md | 45 ++++ packages/objectql/src/metadata-facade.ts | 63 +++++- ...data-service-roundtrip-conformance.test.ts | 214 +++++++++++++----- .../metadata-service-roundtrip-conformance.ts | 48 +++- .../spec/src/contracts/metadata-service.ts | 42 +++- 5 files changed, 340 insertions(+), 72 deletions(-) create mode 100644 .changeset/metadata-facade-roundtrip-ruling.md diff --git a/.changeset/metadata-facade-roundtrip-ruling.md b/.changeset/metadata-facade-roundtrip-ruling.md new file mode 100644 index 0000000000..0543ff3db5 --- /dev/null +++ b/.changeset/metadata-facade-roundtrip-ruling.md @@ -0,0 +1,45 @@ +--- +"@objectstack/objectql": patch +"@objectstack/spec": patch +--- + +fix(objectql,spec): the `name` argument is `IMetadataService`'s effective key (#7378) + +`register(type, name, data)` → `get(type, name)` now holds on `MetadataFacade` +for every `data`, which is what the other four measured implementations +(`MetadataManager` with and without a writable loader, `createMemoryMetadata`, +and the contract's own reference double) already did. Maintainer ruling of +2026-08-11 on #7378, option (a): **the argument is the effective key and +`data.name` never overrides it.** + +**Contract (`@objectstack/spec`).** `IMetadataService.register` and `.get` now +state the rule instead of leaving it to the `@param` names — including for a +`data` that is not an object and so has no `name` to derive, since `data` is +declared `unknown`. `METADATA_ROUNDTRIP_CASES` gains an `array-data-roundtrips` +row: an array passes a `typeof data === 'object'` guard, so a store that keys by +spreading the document corrupts `[a, b]` into `{ 0: a, 1: b }` — the sibling of +the primitive row's silent loss. + +**Behaviour change (`@objectstack/objectql`).** Two `MetadataFacade.register` +paths that derived the storage key from the document now derive it from the +argument: + +- a document whose own `name` (or `id`) disagreed with the `name` argument was + filed under the document's spelling, so `get`/`exists` missed it and + `listNames` reported the other name. It is now stored, read, listed and + unregistered under the argument, with the stored `name` reconciled to it. +- a non-object `data` — a string, number, boolean, array or `null` — was + accepted with no throw and then filed under the literal key `undefined`, + readable back through no member of the class. It is now boxed as + `{ name, content }`, the shape this class's own reads already unwrap, and + round-trips unchanged. + +A host that relied on `MetadataFacade.register` keying by `data.name` or +`data.id` rather than by the argument it passed will see items move to the +argument's key. No in-tree caller does; the class reaches hosts only through the +package's root and `core` exports. + +Not ruled and deliberately unchanged: the plural `objects` type alias (that +alias is `SchemaRegistry`'s own read-side special-case, and its conformance pin +stays a measured divergence with the escalation recorded on #7378), and the +loud-refusal option (c), parked as a v18 strictness candidate. diff --git a/packages/objectql/src/metadata-facade.ts b/packages/objectql/src/metadata-facade.ts index f1f0746a03..10d796c232 100644 --- a/packages/objectql/src/metadata-facade.ts +++ b/packages/objectql/src/metadata-facade.ts @@ -21,6 +21,51 @@ function isObjectType(type: string): boolean { */ const RUNTIME_AUTHORED_PACKAGE_ID = 'sys_metadata'; +/** + * [#7378] Reconcile a `register(type, name, data)` payload to the key the + * contract says it is stored under: **the `name` ARGUMENT**. + * + * `IMetadataService.register` (`@objectstack/spec/contracts`) rules that the + * argument is the effective key and `data.name` never overrides it (maintainer + * ruling 2026-08-11 on #7378, option (a)). Every store this class writes into + * derives its key from the DOCUMENT — `SchemaRegistry.registerItem` reads + * `item[keyField]`, `registerObject` keys `objectContributors` on the schema's + * own `name` — so the only way to honour the argument here is to reconcile the + * document to it before either store sees it. Two shapes, one rule: + * + * - **An object document** keeps every key it was authored with, with `name` + * set to the argument. This is the same normalization + * `ObjectQL.registerMetadataCollections` already applies on the plugin + * ingest path (`item.name === itemName ? item : { ...item, name: itemName }`, + * engine.ts), so a facade write and a plugin write agree about identity + * rather than each keying on a different field. It is also the only + * coherent answer for the `object` type specifically: an object's `name` IS + * its identity to `getObject`, to the data plane and to every driver, so a + * body whose `name` disagreed with its registry key would be dispatched on + * under one spelling and addressable under the other. + * - **Anything else** — a string, a number, a boolean, an array, `null` — is + * boxed as `{ name, content }`. `data` is declared `unknown`, not `object`, + * and a primitive has no `name` to key on: the previous code passed it + * through untouched, `registerItem` read `item['name']` off a string, and + * the value was filed under the literal key `undefined` — accepted with no + * throw and readable back through no member of this class (#7378 cell 3). + * The box is not a new convention: this class's own reads already unwrap it + * (`get`/`list` return `item?.content ?? item`, `listNames` reads + * `item?.name`), so the write half now produces exactly what the read half + * was already prepared to consume. Arrays are boxed rather than spread for + * the same reason — `{ ...[1, 2] }` is `{ 0: 1, 1: 2 }`, which is the same + * silent corruption one shape over. + * + * ⛔ Not a ruling on the DISAGREEMENT itself. Refusing `data.name !== name` + * loudly (option (c)) is recorded on #7378 as the v18 strictness candidate and + * is deliberately NOT implemented here. + */ +function toKeyedDefinition(name: string, data: unknown): any { + return typeof data === 'object' && data !== null && !Array.isArray(data) + ? { ...(data as Record), name } + : { name, content: data }; +} + /** * MetadataFacade * @@ -39,12 +84,15 @@ export class MetadataFacade { constructor(private registry: SchemaRegistry) {} /** - * Register a metadata item + * Register a metadata item under the `name` ARGUMENT. + * + * [#7378] The argument is the effective key and `data.name` never overrides + * it — the contract's ruling, and what {@link toKeyedDefinition} exists to + * honour against two document-keyed stores. Read that helper before changing + * either branch below. */ async register(type: string, name: string, data: any): Promise { - const definition = typeof data === 'object' && data !== null - ? { ...data, name: data.name ?? name } - : data; + const definition = toKeyedDefinition(name, data); // Pass through the item's own source package id (when stamped by an // artifact loader) so provenance survives re-registration. Never // synthesize one here — unstamped items are runtime-authored by @@ -54,7 +102,12 @@ export class MetadataFacade { if (isObjectType(type)) { this.registerObjectBothPlaces(type, definition, packageId); } else { - this.registry.registerItem(type, definition, definition.id ? 'id' as any : 'name' as any, packageId); + // Always keyed on `name` — which {@link toKeyedDefinition} has just set + // to the argument. The former `definition.id ? 'id' : 'name'` keyField + // was the same defect as `data.name ?? name` one field over: a document + // carrying an `id` was filed under THAT, so `get(type, name)` missed it + // whenever the two disagreed (#7378). + this.registry.registerItem(type, definition, 'name' as any, packageId); } } diff --git a/packages/objectql/src/metadata-service-roundtrip-conformance.test.ts b/packages/objectql/src/metadata-service-roundtrip-conformance.test.ts index 8a1bd2e234..a1812841b5 100644 --- a/packages/objectql/src/metadata-service-roundtrip-conformance.test.ts +++ b/packages/objectql/src/metadata-service-roundtrip-conformance.test.ts @@ -38,18 +38,33 @@ * assertion the others get. The weaker match is scoped to the ONE subject that * needs it rather than applied to the whole table. * - * ## Divergences are PINNED, not resolved + * ## Two rows were RULED (#7378); one is still a pinned divergence * - * Three cases get different answers from `MetadataFacade` than from the other - * implementations and the contract's reference double. Each is recorded below - * as a `// DIVERGENCE` entry stating the measured behaviour — this file asserts - * what each implementation does TODAY and changes no shipped behaviour. Which - * answer is correct is a separate ruling, filed as its own card (see the - * per-divergence notes). If you are here because one of these tests failed - * after a behaviour change: that is the pin working. Update it in the PR that - * makes the ruling, not silently. + * This file used to carry three `// DIVERGENCE` entries — three cases + * `MetadataFacade` answered differently from every other implementation and + * from the contract's reference double. The maintainer ruling of 2026-08-11 + * (#7378, option (a) — **the `name` argument is the effective key and + * `data.name` never overrides it**) settled two of them, the contract TSDoc + * now says so, and `MetadataFacade` was aligned to it: * - * Refs #7223, #6725, PR #7211, #6745. + * - the effective key (`key-is-the-name-argument-object` / `-nonobject`) — + * now `RULED_1` below, asserting the ruled answer rather than the old + * `absent`; + * - the dropped non-object `data` (`primitive-data-roundtrips` and its array + * sibling) — no per-subject entry at all any more: the facade simply + * conforms, held to the table's own reference answer like every other + * subject. + * + * `DIVERGENCE_2` (the plural `objects` alias) survives, deliberately, with the + * measurement for why the same ruling could not carry it — read it before + * assuming it was overlooked. + * + * If you are here because one of these tests failed after a behaviour change: + * that is the pin working. A RULED row going red means an implementation + * drifted off a decided contract, and the fix belongs in the implementation; + * update the pin only in the PR that changes the ruling. + * + * Refs #7223, #7378, #6725, PR #7211, #6745. */ import { describe, it, expect } from 'vitest'; @@ -81,18 +96,30 @@ type RoundTrippingService = Pick>; + readonly divergences?: Readonly>; create(): RoundTrippingService; } @@ -159,26 +186,32 @@ class WritableFixtureLoader implements MetadataLoader { } /** - * ── DIVERGENCE 1 — the effective key is `data.name`, not the `name` argument ── + * ── RULED 1 — the effective key is the `name` ARGUMENT ── * - * Cases `key-is-the-name-argument-object` / `-nonobject`. + * Cases `key-is-the-name-argument-object` / `-nonobject`. Was DIVERGENCE 1. * - * `MetadataFacade.register` opens with - * `{ ...data, name: data.name ?? name }` and then hands the DOCUMENT to + * `MetadataFacade.register` used to open with + * `{ ...data, name: data.name ?? name }` and hand the DOCUMENT to * `SchemaRegistry.registerObject` / `registerItem`, which key on the document's - * own `name`. The `name` argument is therefore only a fallback for a document - * that carries none: when the two disagree, the item lands under `data.name` - * and `get(type, )` answers `undefined`, `exists` - * answers `false`, and `listNames` reports the other spelling. Measured on both - * an object-typed and a view-typed write. + * own `name`. The argument was therefore only a fallback for a document that + * carried none: when the two disagreed the item landed under `data.name`, and + * `get(type, )` answered `undefined`, `exists` + * answered `false`, and `listNames` reported the other spelling. * - * `MetadataManager` and `createMemoryMetadata` both key on the argument, as does - * the contract's reference double. The contract TSDoc names the parameter on - * both members (`@param name - Item name/identifier (snake_case)`) and says - * nothing about `data.name`, so nothing in-tree currently RULES which is right — - * which is why this is pinned as measured and filed, not fixed here. + * The maintainer ruling of 2026-08-11 (#7378, option (a)) settled it the other + * way — the argument is the effective key, `data.name` never overrides it — + * `IMetadataService.register` / `get` now say so, and the facade was aligned + * (`toKeyedDefinition`). All five implementations answer this row the same way + * today. + * + * What stays subject-specific is only the SHAPE of the answer, and only because + * the facade's stores are keyed by the document itself: reconciling the + * document to the argument means the `name` that comes back IS the argument, + * where the verbatim subjects hand back the authored `name` under the argument + * key. Both honour the ruling — the key is the argument in every case — so this + * entry asserts the ruled answer rather than suppressing the row. */ -const DIVERGENCE_1 = 'MetadataFacade keys on `data.name` when it disagrees with the `name` argument; the other implementations key on the argument. Pinned as measured (#7223).'; +const RULED_1 = 'MetadataFacade keys on the `name` argument (#7378 ruling (a)); reconciling its document-keyed stores to that argument also normalizes the stored `name` to it.'; /** * ── DIVERGENCE 2 — the plural `objects` type is aliased to `object` ── @@ -194,27 +227,67 @@ const DIVERGENCE_1 = 'MetadataFacade keys on `data.name` when it disagrees with * `MetadataManager` and `createMemoryMetadata` key their type stores on the * string they are handed, so the two spellings are two stores and the item is * invisible under the singular. + * + * ── Why the #7378 ruling did NOT carry this row (measured 2026-08-11) ── + * + * The ruling aligned the facade on the other two rows and named this one with + * them, but aligning it is not a facade-local change and the two candidate + * routes are both worse than the divergence: + * + * 1. **The alias that decides this row is `SchemaRegistry`'s, not the + * facade's.** `registry.getItem` and `registry.listItems` special-case + * BOTH spellings straight to `getObject` / `getAllObjects` + * (`registry.ts`, "Special handling for 'object' and 'objects' types"), so + * the READ this case makes is aliased one layer below anything + * `metadata-facade.ts` controls. Narrowing the facade's own `isObjectType` + * to the singular would therefore make `register('objects', n, d)` write + * into `metadata['objects']` while `get('objects', n)` still resolves + * through `registry.getItem` → `getObject(n)` → `undefined`: the write + * lands nowhere any read looks. That is #6725 EXACTLY, re-opened for the + * plural spelling — the silent loss this whole table exists because of, and + * the row would go green while the bug got worse. + * 2. **Removing the registry alias runs against the platform's own + * normalization direction.** Plural→singular folding is owned by the layers + * below this contract and is enforced there: `canonicalMetaType` + * (`PLURAL_TO_SINGULAR`) canonicalizes every `/meta` request type at the + * protocol boundary (#4432), `RestServer.metaTypeSingular` does it at the + * REST boundary, and `check:meta-type-normalized` is a CI gate whose whole + * job is to refuse a decision made on the un-normalized `:type` — three + * authorization bypasses (#3984, #5881, #6241) came from exactly that. + * `registry.test.ts` pins `listItems("objects")` as a deliberate alias. + * "The two spellings are one type" is a decided platform-wide position; + * this row asks for the opposite, and that is a ruling of its own, not an + * implementation detail this card can settle. + * + * So this stays pinned as measured, and the question — does `IMetadataService` + * key its type stores on the raw string (reference semantics) or on the + * canonical type (what the rest of the platform does)? — is escalated on #7378 + * rather than answered here. */ -const DIVERGENCE_2 = 'MetadataFacade aliases the plural `objects` type to `object`; the other implementations keep one store per type string. Pinned as measured (#7223).'; +const DIVERGENCE_2 = 'MetadataFacade aliases the plural `objects` type to `object` — through SchemaRegistry\'s own read-side special-case, not its own; the other implementations keep one store per type string. Still pinned as measured: the #7378 ruling did not carry this row (see the note above), and the open question is escalated there.'; /** - * ── DIVERGENCE 3 — a non-object `data` value is dropped ── + * ── RULED 3 — a non-object `data` value round-trips (no entry needed) ── * - * Case `primitive-data-roundtrips`. + * Cases `primitive-data-roundtrips`, `array-data-roundtrips`. Was DIVERGENCE 3, + * and is deliberately NOT replaced by an entry in `divergences` below: the + * facade now conforms to the table's own reference answer, so the row is held + * against it like every other subject's. * - * The contract declares `data: unknown`. `MetadataFacade.register` passes a - * non-object value through unchanged (its `{ ...data }` branch is guarded on - * `typeof data === 'object' && data !== null`) and then registers it under the - * document's own `name` — which a string does not have. The write is ACCEPTED - * (no throw), the registry logs `Registered setting: undefined`, and the value - * is readable back through nothing: `get` answers `undefined`, `exists` answers - * `false`, `listNames` is empty. Silent loss, the same family of failure as - * #6725 — which is the reason this row is in the table at all. + * What it used to measure: the contract declares `data: unknown`, and + * `MetadataFacade.register` passed a non-object value through unchanged (its + * `{ ...data }` branch is guarded on `typeof data === 'object' && data !== null`) + * and then registered it under the document's own `name` — which a string does + * not have. The write was ACCEPTED (no throw), the registry logged + * `Registered setting: undefined`, and the value was readable back through + * nothing. Silent loss, the same family as #6725. * - * `MetadataManager` and `createMemoryMetadata` store the value against the key - * and hand it straight back. + * Under the #7378 ruling the argument is the key, so a value with no `name` of + * its own HAS one; `toKeyedDefinition` boxes it as `{ name, content }`, which + * is the shape this class's own reads already unwrap. The array row is the + * sibling shape that `typeof data === 'object'` got wrong in the other + * direction — spread into `{ 0: …, 1: … }` rather than dropped. */ -const DIVERGENCE_3 = 'MetadataFacade silently drops a non-object `data` value — accepted by `register`, readable back through no member. The other implementations round-trip it. Pinned as measured (#7223).'; const IMPLEMENTATIONS: readonly PinnedImplementation[] = [ { @@ -238,10 +311,9 @@ const IMPLEMENTATIONS: readonly PinnedImplementation[] = [ label: 'MetadataFacade', documentFidelity: 'runtime-effective', divergences: { - 'key-is-the-name-argument-object': { kind: 'absent', note: DIVERGENCE_1 }, - 'key-is-the-name-argument-nonobject': { kind: 'absent', note: DIVERGENCE_1 }, + 'key-is-the-name-argument-object': { kind: 'readable-keyed-by-argument', note: RULED_1 }, + 'key-is-the-name-argument-nonobject': { kind: 'readable-keyed-by-argument', note: RULED_1 }, 'plural-objects-type-is-its-own-store': { kind: 'readable-as-last-write', note: DIVERGENCE_2 }, - 'primitive-data-roundtrips': { kind: 'absent', note: DIVERGENCE_3 }, }, create: () => new MetadataFacade(new SchemaRegistry({ multiTenant: false })), }, @@ -254,17 +326,49 @@ function lastWrittenDocument(testCase: MetadataRoundTripCase): unknown { /** * The answer this subject is held to for this case: the table's reference - * answer, unless the subject declares a divergence for it. + * answer, unless the subject declares a per-subject answer for it. */ function expectationFor( implementation: PinnedImplementation, testCase: MetadataRoundTripCase, ): { kind: 'readable'; document: unknown } | { kind: 'absent' } { - const divergence = implementation.divergences?.[testCase.id]; - if (!divergence) return testCase.expected; - return divergence.kind === 'absent' - ? { kind: 'absent' } - : { kind: 'readable', document: lastWrittenDocument(testCase) }; + const answer = implementation.divergences?.[testCase.id]; + if (!answer) return testCase.expected; + switch (answer.kind) { + case 'absent': + return { kind: 'absent' }; + case 'readable-as-last-write': + return { kind: 'readable', document: lastWrittenDocument(testCase) }; + case 'readable-keyed-by-argument': + // The authored document, with its own `name` reconciled to the + // effective key — see RULED_1. Every other authored key is asserted + // unchanged, so this cannot degrade into "something came back". + return { + kind: 'readable', + document: { + ...(lastWrittenDocument(testCase) as Record), + name: testCase.read.name, + }, + }; + } +} + +/** + * The `name` a case's written document carries when that is NOT the key the + * case reads — i.e. the spelling an implementation keying on `data.name` would + * file the item under. `undefined` when the case does not pose the question. + * + * [#7378] Asserting this stale spelling is ABSENT from `listNames` is what + * keeps the ruled rows from passing for the wrong reason: an implementation + * that stored the item twice, or that kept the document's own name as a second + * key, satisfies every other assertion on those rows and fails only this one. + */ +function staleDocumentName(testCase: MetadataRoundTripCase): string | undefined { + const written = lastWrittenDocument(testCase); + const documentName = (written as { name?: unknown } | undefined)?.name; + return typeof documentName === 'string' && documentName !== testCase.read.name + ? documentName + : undefined; } describe.each(IMPLEMENTATIONS)( @@ -306,6 +410,10 @@ describe.each(IMPLEMENTATIONS)( // overwriting would satisfy every assertion above on the // re-register rows and fail only this one. expect(names.filter((name) => name === testCase.read.name)).toHaveLength(1); + + // [#7378] …and never ALSO under the document's own name. + const stale = staleDocumentName(testCase); + if (stale !== undefined) expect(names).not.toContain(stale); } else { expect(got).toBeUndefined(); expect(exists).toBe(false); diff --git a/packages/spec/src/contracts/metadata-service-roundtrip-conformance.ts b/packages/spec/src/contracts/metadata-service-roundtrip-conformance.ts index 692a6a9f36..10d4f83c22 100644 --- a/packages/spec/src/contracts/metadata-service-roundtrip-conformance.ts +++ b/packages/spec/src/contracts/metadata-service-roundtrip-conformance.ts @@ -46,14 +46,26 @@ * ## What `expected` means, precisely * * **The reference semantics: a store keyed by `type` × the `name` ARGUMENT.** - * That is what the contract's own double implements, and what its parameter - * names say (`@param name - Item name/identifier (snake_case)` on both - * members). It is deliberately NOT a ruling that every shipped implementation - * currently satisfies it — three cases below are answered differently by - * `MetadataFacade` today, and those answers are pinned as measured, with a - * `// DIVERGENCE` marker, in the objectql driver. Pinning ≠ blessing: read the - * divergence notes there and the card they link before treating either answer - * as the intended one. + * That is what the contract's own double implements, what its parameter names + * say (`@param name - Item name/identifier (snake_case)` on both members) — + * and, since the maintainer ruling of 2026-08-11 on #7378 (option (a)), what + * {@link ../contracts/metadata-service | IMetadataService.register} RULES in + * so many words: the argument is the effective key and `data.name` never + * overrides it. + * + * That ruling settled two of the three rows `MetadataFacade` used to answer + * differently — the effective key (`key-is-the-name-argument-*`) and the + * dropped non-object `data` (`primitive-data-roundtrips` and its array + * sibling) — and `MetadataFacade` was aligned to the contract on both. What + * they pin in the objectql driver is now RULED BEHAVIOUR, not a measured + * divergence. + * + * ONE row is still measured-and-unresolved: `plural-objects-type-is-its-own-store`. + * The alias it measures is `SchemaRegistry`'s, not the facade's, and removing + * it collides with the platform's own plural→singular normalization direction — + * the objectql driver's surviving `// DIVERGENCE` note carries the evidence and + * the escalation. Pinning ≠ blessing: read that note and the card it links + * before treating either answer as the intended one. * * ## Deliberate scope * @@ -148,6 +160,12 @@ const PIN_CASED = objectDocument('Pin_Cased'); const PIN_KEYED_OBJECT = objectDocument('pin_data_name'); const PIN_KEYED_VIEW = viewDocument('pin_data_name_view'); +/** + * A non-object `data` that is nonetheless `typeof 'object'` — the shape a + * spread-to-key implementation corrupts rather than drops. + */ +const PIN_ARRAY = ['alpha', 'beta']; + const PIN_PLURAL = objectDocument('pin_plural'); const PIN_REMOVED = objectDocument('pin_removed'); @@ -245,7 +263,7 @@ export const METADATA_ROUNDTRIP_CASES: readonly MetadataRoundTripCase[] = [ writes: [{ type: 'object', name: 'pin_key', data: PIN_KEYED_OBJECT }], read: { type: 'object', name: 'pin_key' }, expected: { kind: 'readable', document: PIN_KEYED_OBJECT }, - why: 'Whether `name` or `data.name` is the key is the whole round-trip. The contract names the parameter on both members and says nothing about `data.name`, so the reference answer is the argument. **Shipped implementations disagree here** — see the DIVERGENCE notes in the objectql driver.', + why: 'Whether `name` or `data.name` is the key is the whole round-trip. **Ruled** (#7378, maintainer 2026-08-11, option (a)): the argument is the effective key and `data.name` never overrides it. Every shipped implementation now answers this way; `MetadataFacade` was aligned to it in the same PR.', }, { id: 'key-is-the-name-argument-nonobject', @@ -261,7 +279,7 @@ export const METADATA_ROUNDTRIP_CASES: readonly MetadataRoundTripCase[] = [ writes: [{ type: 'objects', name: 'pin_plural', data: PIN_PLURAL }], read: { type: 'object', name: 'pin_plural' }, expected: { kind: 'absent' }, - why: 'The two spellings of the object type. The reference store keys on the string it is given; some implementations alias the plural to the singular. **Shipped implementations disagree here** — see the DIVERGENCE notes in the objectql driver.', + why: 'The two spellings of the object type. The reference store keys on the string it is given; some implementations alias the plural to the singular. **Shipped implementations still disagree here, and this row is the one #7378 did NOT settle** — the alias is `SchemaRegistry`\'s own, on the READ side, and the objectql driver\'s surviving DIVERGENCE note carries the measurement and the escalation.', }, { id: 'primitive-data-roundtrips', @@ -269,7 +287,15 @@ export const METADATA_ROUNDTRIP_CASES: readonly MetadataRoundTripCase[] = [ writes: [{ type: 'setting', name: 'pin_flag', data: 'enabled' }], read: { type: 'setting', name: 'pin_flag' }, expected: { kind: 'readable', document: 'enabled' }, - why: '`data` is declared `unknown`, not `object`. An implementation that derives its key from `data.name` has nothing to derive it from here. **Shipped implementations disagree here** — see the DIVERGENCE notes in the objectql driver.', + why: '`data` is declared `unknown`, not `object`. An implementation that derives its key from `data.name` has nothing to derive it from here — which is why the #7378 ruling (the ARGUMENT is the key) is what makes this row answerable at all. Accepting the write and dropping the value is the one answer the ruling forbids.', + }, + { + id: 'array-data-roundtrips', + title: 'an array `data` value is readable back as an array', + writes: [{ type: 'setting', name: 'pin_list', data: PIN_ARRAY }], + read: { type: 'setting', name: 'pin_list' }, + expected: { kind: 'readable', document: PIN_ARRAY }, + why: 'The sibling shape of the row above, and the one a `typeof data === "object"` guard gets WRONG rather than drops: an array passes that test, so an implementation that keys by spreading the document turns [a, b] into { 0: a, 1: b } — silent corruption where the primitive row measured silent loss. Neither survives the #7378 ruling.', }, { id: 'absent-after-unregister', diff --git a/packages/spec/src/contracts/metadata-service.ts b/packages/spec/src/contracts/metadata-service.ts index 7297b20f70..3b94821a8b 100644 --- a/packages/spec/src/contracts/metadata-service.ts +++ b/packages/spec/src/contracts/metadata-service.ts @@ -226,9 +226,38 @@ export interface IMetadataService { * `options.notify === false` — see {@link MetadataWriteOptions.notify} * before silencing it. * + * ## The `name` ARGUMENT is the effective key (#7378) + * + * `name` — not `data.name` — is what this item is stored under, and it is + * what {@link get}, {@link exists}, {@link listNames} and + * {@link unregister} address it by. When a document carries a `name` of its + * own and the two disagree, **the argument wins and `data.name` never + * overrides it**: `register(t, n, d)` followed by `get(t, n)` holds for + * every `d`, including one whose own `name` says something else, and + * including a `data` that is not an object at all and so has no `name` to + * derive (`data` is declared `unknown`, not `object`). + * + * An implementation whose store derives the key from the document — + * `SchemaRegistry`, whose objects are keyed by their own identity — must + * reconcile the document to the argument rather than the other way round; + * what it may NOT do is accept the write and file it somewhere the + * caller's own `name` cannot reach it. `METADATA_ROUNDTRIP_CASES` + * (`./metadata-service-roundtrip-conformance`) is the executable form of + * this paragraph. + * + * Maintainer ruling of 2026-08-11 on #7378, option (a), taken because it is + * what 4 of the 5 measured implementations already did and what this + * member's own `@param name` already implied. Option (c) — **refuse a + * `data.name !== name` disagreement loudly** instead of silently resolving + * it either way — is recorded there as the long-term strictness candidate + * and PARKED for v18 consideration: it is the only answer that cannot + * misplace an item, and it is a behaviour change on every implementation, + * so it is not this contract's rule today. Do not implement it ahead of + * that ruling. + * * @param type - Metadata type (e.g. 'object', 'view', 'flow') - * @param name - Item name/identifier (snake_case) - * @param data - The metadata definition to register + * @param name - Item name/identifier (snake_case) — the effective storage key + * @param data - The metadata definition to register; a `name` it carries does NOT displace the argument * @param options - Write options; `{ notify: false }` suppresses the watcher event */ register(type: string, name: string, data: unknown, options?: MetadataWriteOptions): Promise; @@ -255,12 +284,19 @@ export interface IMetadataService { /** * Get a metadata item by type and name * + * `name` is the key {@link register} was called with — the ARGUMENT, never + * a `name` the stored document happens to carry (#7378, maintainer ruling + * 2026-08-11). So `register(t, n, d)` → `get(t, n)` resolves for every `d`, + * and the document's own `name` disagreeing with `n` changes nothing about + * where it is found. See {@link register} for the full statement and for + * the strictness option parked behind it. + * * `undefined` is AMBIGUOUS by construction — it means "not found" *and* * "every loader that could hold it failed". Prefer {@link getDiagnosed} * wherever the difference could change a decision (#5840). * * @param type - Metadata type - * @param name - Item name/identifier + * @param name - Item name/identifier — the key `register` was called with * @returns The metadata definition, or undefined if not found */ get(type: string, name: string): Promise;