Skip to content

Commit 9051802

Browse files
os-zhuangclaude
andauthored
fix(objectql,spec): rule the name argument as IMetadataService's effective key (#7378) (#7511)
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. Claude-Session: https://claude.ai/code/session_01KtffEdkV9BA8f4yQXGXP9E Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1530870 commit 9051802

5 files changed

Lines changed: 340 additions & 72 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@objectstack/objectql": patch
3+
"@objectstack/spec": patch
4+
---
5+
6+
fix(objectql,spec): the `name` argument is `IMetadataService`'s effective key (#7378)
7+
8+
`register(type, name, data)` → `get(type, name)` now holds on `MetadataFacade`
9+
for every `data`, which is what the other four measured implementations
10+
(`MetadataManager` with and without a writable loader, `createMemoryMetadata`,
11+
and the contract's own reference double) already did. Maintainer ruling of
12+
2026-08-11 on #7378, option (a): **the argument is the effective key and
13+
`data.name` never overrides it.**
14+
15+
**Contract (`@objectstack/spec`).** `IMetadataService.register` and `.get` now
16+
state the rule instead of leaving it to the `@param` names — including for a
17+
`data` that is not an object and so has no `name` to derive, since `data` is
18+
declared `unknown`. `METADATA_ROUNDTRIP_CASES` gains an `array-data-roundtrips`
19+
row: an array passes a `typeof data === 'object'` guard, so a store that keys by
20+
spreading the document corrupts `[a, b]` into `{ 0: a, 1: b }` — the sibling of
21+
the primitive row's silent loss.
22+
23+
**Behaviour change (`@objectstack/objectql`).** Two `MetadataFacade.register`
24+
paths that derived the storage key from the document now derive it from the
25+
argument:
26+
27+
- a document whose own `name` (or `id`) disagreed with the `name` argument was
28+
filed under the document's spelling, so `get`/`exists` missed it and
29+
`listNames` reported the other name. It is now stored, read, listed and
30+
unregistered under the argument, with the stored `name` reconciled to it.
31+
- a non-object `data` — a string, number, boolean, array or `null` — was
32+
accepted with no throw and then filed under the literal key `undefined`,
33+
readable back through no member of the class. It is now boxed as
34+
`{ name, content }`, the shape this class's own reads already unwrap, and
35+
round-trips unchanged.
36+
37+
A host that relied on `MetadataFacade.register` keying by `data.name` or
38+
`data.id` rather than by the argument it passed will see items move to the
39+
argument's key. No in-tree caller does; the class reaches hosts only through the
40+
package's root and `core` exports.
41+
42+
Not ruled and deliberately unchanged: the plural `objects` type alias (that
43+
alias is `SchemaRegistry`'s own read-side special-case, and its conformance pin
44+
stays a measured divergence with the escalation recorded on #7378), and the
45+
loud-refusal option (c), parked as a v18 strictness candidate.

‎packages/objectql/src/metadata-facade.ts‎

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,51 @@ function isObjectType(type: string): boolean {
2121
*/
2222
const RUNTIME_AUTHORED_PACKAGE_ID = 'sys_metadata';
2323

24+
/**
25+
* [#7378] Reconcile a `register(type, name, data)` payload to the key the
26+
* contract says it is stored under: **the `name` ARGUMENT**.
27+
*
28+
* `IMetadataService.register` (`@objectstack/spec/contracts`) rules that the
29+
* argument is the effective key and `data.name` never overrides it (maintainer
30+
* ruling 2026-08-11 on #7378, option (a)). Every store this class writes into
31+
* derives its key from the DOCUMENT — `SchemaRegistry.registerItem` reads
32+
* `item[keyField]`, `registerObject` keys `objectContributors` on the schema's
33+
* own `name` — so the only way to honour the argument here is to reconcile the
34+
* document to it before either store sees it. Two shapes, one rule:
35+
*
36+
* - **An object document** keeps every key it was authored with, with `name`
37+
* set to the argument. This is the same normalization
38+
* `ObjectQL.registerMetadataCollections` already applies on the plugin
39+
* ingest path (`item.name === itemName ? item : { ...item, name: itemName }`,
40+
* engine.ts), so a facade write and a plugin write agree about identity
41+
* rather than each keying on a different field. It is also the only
42+
* coherent answer for the `object` type specifically: an object's `name` IS
43+
* its identity to `getObject`, to the data plane and to every driver, so a
44+
* body whose `name` disagreed with its registry key would be dispatched on
45+
* under one spelling and addressable under the other.
46+
* - **Anything else** — a string, a number, a boolean, an array, `null` — is
47+
* boxed as `{ name, content }`. `data` is declared `unknown`, not `object`,
48+
* and a primitive has no `name` to key on: the previous code passed it
49+
* through untouched, `registerItem` read `item['name']` off a string, and
50+
* the value was filed under the literal key `undefined` — accepted with no
51+
* throw and readable back through no member of this class (#7378 cell 3).
52+
* The box is not a new convention: this class's own reads already unwrap it
53+
* (`get`/`list` return `item?.content ?? item`, `listNames` reads
54+
* `item?.name`), so the write half now produces exactly what the read half
55+
* was already prepared to consume. Arrays are boxed rather than spread for
56+
* the same reason — `{ ...[1, 2] }` is `{ 0: 1, 1: 2 }`, which is the same
57+
* silent corruption one shape over.
58+
*
59+
* ⛔ Not a ruling on the DISAGREEMENT itself. Refusing `data.name !== name`
60+
* loudly (option (c)) is recorded on #7378 as the v18 strictness candidate and
61+
* is deliberately NOT implemented here.
62+
*/
63+
function toKeyedDefinition(name: string, data: unknown): any {
64+
return typeof data === 'object' && data !== null && !Array.isArray(data)
65+
? { ...(data as Record<string, unknown>), name }
66+
: { name, content: data };
67+
}
68+
2469
/**
2570
* MetadataFacade
2671
*
@@ -39,12 +84,15 @@ export class MetadataFacade {
3984
constructor(private registry: SchemaRegistry) {}
4085

4186
/**
42-
* Register a metadata item
87+
* Register a metadata item under the `name` ARGUMENT.
88+
*
89+
* [#7378] The argument is the effective key and `data.name` never overrides
90+
* it — the contract's ruling, and what {@link toKeyedDefinition} exists to
91+
* honour against two document-keyed stores. Read that helper before changing
92+
* either branch below.
4393
*/
4494
async register(type: string, name: string, data: any): Promise<void> {
45-
const definition = typeof data === 'object' && data !== null
46-
? { ...data, name: data.name ?? name }
47-
: data;
95+
const definition = toKeyedDefinition(name, data);
4896
// Pass through the item's own source package id (when stamped by an
4997
// artifact loader) so provenance survives re-registration. Never
5098
// synthesize one here — unstamped items are runtime-authored by
@@ -54,7 +102,12 @@ export class MetadataFacade {
54102
if (isObjectType(type)) {
55103
this.registerObjectBothPlaces(type, definition, packageId);
56104
} else {
57-
this.registry.registerItem(type, definition, definition.id ? 'id' as any : 'name' as any, packageId);
105+
// Always keyed on `name` — which {@link toKeyedDefinition} has just set
106+
// to the argument. The former `definition.id ? 'id' : 'name'` keyField
107+
// was the same defect as `data.name ?? name` one field over: a document
108+
// carrying an `id` was filed under THAT, so `get(type, name)` missed it
109+
// whenever the two disagreed (#7378).
110+
this.registry.registerItem(type, definition, 'name' as any, packageId);
58111
}
59112
}
60113

0 commit comments

Comments
 (0)