Skip to content

Commit fdca3a1

Browse files
os-zhuangclaude
andauthored
fix(objectql): MetadataFacade reads return the stored document — content is a real authorable field, not a storage envelope (#8377)
* fix(objectql): MetadataFacade reads return the stored document — content is a real authorable field, not a storage envelope (#7519) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy * chore: changeset for the MetadataFacade content-field round-trip fix (#7519) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e41c1f2 commit fdca3a1

4 files changed

Lines changed: 166 additions & 9 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
`MetadataFacade.get` / `list` / `listNames` no longer unwrap stored items through `item?.content ?? item`. `content` is a real authorable field (`doc`, `knowledge_document`), so a document registered with a `content` field read back as that field's value — a doc came back as its raw Markdown string instead of the document, silently (truthy and string-typed, so downstream `?.name` reads yielded `undefined` rather than throwing). The storage envelope the unwrap presumed has no producer anywhere in the tree: the facade's own interim `{ name, content }` boxing of non-object values — the only writer that ever produced one — was already removed in favour of a loud refusal under the #7378 register ruling, and DB hydration registers the parsed document itself. All three reads now return the stored document verbatim, restoring the ruled `register(t, n, d)` → `get(t, n)` round-trip for every metadata type that authors a `content` field. No replacement envelope key is introduced, so no other authorable key can inherit the collision.

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

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,3 +228,99 @@ describe('MetadataFacade object write/read round-trip', () => {
228228
await expect(facade.unregister('object', 'absent')).resolves.toBeUndefined();
229229
});
230230
});
231+
232+
/**
233+
* [#7519] `content` is a REAL authorable field, not this facade's storage
234+
* envelope.
235+
*
236+
* Every read member used to unwrap `item?.content ?? item`, presuming
237+
* `content` marked the facade's own wrapper — but `doc.zod.ts` (raw Markdown)
238+
* and `knowledge-document.zod.ts` both declare `content` as an authored
239+
* field, so a doc registered through the facade read back as its Markdown
240+
* STRING: truthy and string-typed, so nothing threw and downstream `?.name`
241+
* reads silently yielded `undefined`. That is the silent non-round-trip the
242+
* #7378 ruling (2026-08-12) forbids: `register(t, n, d)` → `get(t, n)`
243+
* round-trips or refuses loudly.
244+
*
245+
* The envelope the unwrap presumed has NO producer (measured on `main`, not
246+
* assumed — the full ledger is in `get`'s header in metadata-facade.ts), so
247+
* the fix removes the unwrap rather than renaming the envelope key: any
248+
* replacement key would merely reschedule this collision onto the next
249+
* authorable field.
250+
*/
251+
describe('MetadataFacade reads return the stored document, not its `content` field (#7519)', () => {
252+
let registry: SchemaRegistry;
253+
let facade: MetadataFacade;
254+
255+
beforeEach(() => {
256+
registry = new SchemaRegistry({ multiTenant: false });
257+
facade = new MetadataFacade(registry);
258+
});
259+
260+
const MARKDOWN = '# Getting started\n\nWrite your first object.';
261+
const docDocument = () => ({
262+
name: 'getting_started',
263+
label: 'Getting started',
264+
content: MARKDOWN,
265+
});
266+
267+
it('get returns the registered doc document, not its Markdown string', async () => {
268+
await facade.register('doc', 'getting_started', docDocument());
269+
270+
const got = (await facade.get('doc', 'getting_started')) as any;
271+
// The defect shape was `got === MARKDOWN` — truthy and defined, so a
272+
// bare toBeDefined() would have passed. Assert the DOCUMENT came back.
273+
expect(got).toBeDefined();
274+
expect(got).not.toBe(MARKDOWN);
275+
expect(got.name).toBe('getting_started');
276+
expect(got.content).toBe(MARKDOWN);
277+
});
278+
279+
it('list returns doc documents, not Markdown strings', async () => {
280+
await facade.register('doc', 'getting_started', docDocument());
281+
await facade.register('doc', 'faq', { name: 'faq', content: '# FAQ' });
282+
283+
const listed = (await facade.list('doc')) as any[];
284+
expect(listed).toHaveLength(2);
285+
expect(listed.map((d) => d?.name).sort()).toEqual(['faq', 'getting_started']);
286+
expect(listed.every((d) => typeof d === 'object' && d !== null)).toBe(true);
287+
});
288+
289+
it('listNames reads names off the documents themselves', async () => {
290+
await facade.register('doc', 'getting_started', docDocument());
291+
292+
expect(await facade.listNames('doc')).toEqual(['getting_started']);
293+
});
294+
295+
it('exists and getEntry agree the document is there, whole', async () => {
296+
await facade.register('doc', 'getting_started', docDocument());
297+
298+
expect(await facade.exists('doc', 'getting_started')).toBe(true);
299+
const entry = facade.getEntry('doc', 'getting_started') as any;
300+
expect(entry.content).toBe(MARKDOWN);
301+
});
302+
303+
it('a knowledge_document with a `content` field round-trips whole too', async () => {
304+
// The second live type the issue names (knowledge-document.zod.ts) —
305+
// pinned so the fix cannot be read as doc-specific.
306+
await facade.register('knowledge_document', 'onboarding_kb', {
307+
name: 'onboarding_kb',
308+
content: 'Answer the onboarding questions from this corpus.',
309+
});
310+
311+
const got = (await facade.get('knowledge_document', 'onboarding_kb')) as any;
312+
expect(got.name).toBe('onboarding_kb');
313+
expect(typeof got.content).toBe('string');
314+
});
315+
316+
it('a document WITHOUT a `content` field still round-trips unchanged (the surviving path)', async () => {
317+
// The other direction pinned: removing the unwrap must not trade one
318+
// class of correct read for another. Content-less items were read
319+
// correctly before this fix (the `??` fell through) and must stay so.
320+
await facade.register('view', 'plain_view', { name: 'plain_view', label: 'Plain', type: 'grid' });
321+
322+
const got = (await facade.get('view', 'plain_view')) as any;
323+
expect(got).toMatchObject({ name: 'plain_view', label: 'Plain', type: 'grid' });
324+
expect(await facade.listNames('view')).toEqual(['plain_view']);
325+
});
326+
});

packages/objectql/src/metadata-facade.ts

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,29 @@ export class MetadataFacade {
188188
}
189189

190190
/**
191-
* Get a metadata item by type and name.
191+
* Get a metadata item by type and name — the STORED document, verbatim from
192+
* the registry.
193+
*
194+
* [#7519] An `item?.content ?? item` unwrap used to sit on this return (and
195+
* on {@link list} / {@link listNames}), presuming `content` was this
196+
* facade's own storage envelope. But `content` is a REAL authorable field —
197+
* `doc.zod.ts` (raw Markdown) and `knowledge-document.zod.ts` both declare
198+
* it — so `register('doc', n, document)` → `get('doc', n)` answered the
199+
* Markdown STRING instead of the document: truthy, string-typed, and silent
200+
* (downstream `?.name` reads yield `undefined` rather than throwing) —
201+
* exactly the silent non-round-trip the #7378 ruling forbids
202+
* (`register(t, n, d)` → `get(t, n)` round-trips or is refused loudly).
203+
*
204+
* The envelope the unwrap presumed has NO producer. Measured, not assumed:
205+
* the only writer that ever produced one was this class's own interim
206+
* `{ name, content }` boxing of non-object values (#7511 cell 3), which
207+
* #8349 removed in favour of the shared guard's loud refusal (see
208+
* {@link toKeyedDefinition}); DB hydration (`loadMetaFromDb`,
209+
* metadata-protocol) parses the `sys_metadata.metadata` column and registers
210+
* the document itself; every other in-tree `registerItem` caller stores the
211+
* document as-is. With no envelope on any write path, an unwrap on the read
212+
* path can only corrupt — so there is none, and no replacement envelope key
213+
* either (any key chosen would just collide with the next authorable field).
192214
*
193215
* `currentPackageId` (ADR-0048) opts into package-scoped resolution: when two
194216
* installed packages ship an item of the same `type`/`name`, the registry
@@ -198,23 +220,28 @@ export class MetadataFacade {
198220
*/
199221
async get(type: string, name: string, currentPackageId?: string): Promise<any> {
200222
// [#7378 row 2] Read the store `register` wrote: the canonical type.
201-
const item = this.registry.getItem(canonicalMetadataServiceType(type), name, currentPackageId) as any;
202-
return item?.content ?? item;
223+
return this.registry.getItem(canonicalMetadataServiceType(type), name, currentPackageId);
203224
}
204225

205226
/**
206-
* Get the raw entry (with metadata wrapper)
227+
* Get the raw stored entry, synchronously and without package-scoped
228+
* resolution. ([#7519] Historically documented as "with metadata wrapper" —
229+
* there is no wrapper; {@link get} returns the same stored document. This
230+
* member survives as the sync, scope-free variant.)
207231
*/
208232
getEntry(type: string, name: string): any {
209233
return this.registry.getItem(canonicalMetadataServiceType(type), name);
210234
}
211235

212236
/**
213-
* List all items of a type
237+
* List all items of a type — the stored documents, verbatim.
238+
*
239+
* [#7519] The former `item?.content ?? item` map is gone for the reason
240+
* {@link get}'s header carries in full: `content` is a real authorable
241+
* field, and the envelope the unwrap presumed has no producer.
214242
*/
215243
async list(type: string): Promise<any[]> {
216-
const items = this.registry.listItems(canonicalMetadataServiceType(type));
217-
return items.map((item: any) => item?.content ?? item);
244+
return this.registry.listItems(canonicalMetadataServiceType(type));
218245
}
219246

220247
/**
@@ -252,11 +279,20 @@ export class MetadataFacade {
252279
}
253280

254281
/**
255-
* List all names of metadata items of a given type
282+
* List all names of metadata items of a given type.
283+
*
284+
* [#7519] The former `item?.content?.name` fallback limb was the same
285+
* envelope presumption {@link get}'s header retires, one member over — and
286+
* it was dead in both directions: on an envelope-shaped entry nothing
287+
* produces it would have read a name out of the wrapper, and on a real
288+
* `doc` (whose `content` is a Markdown STRING) `content?.name` is
289+
* `undefined` anyway. Every admitted document carries `name` —
290+
* {@link toKeyedDefinition} sets it from the argument on this class's own
291+
* writes, and every in-tree `registerItem` caller keys by `name`.
256292
*/
257293
async listNames(type: string): Promise<string[]> {
258294
const items = this.registry.listItems(canonicalMetadataServiceType(type));
259-
return items.map((item: any) => item?.name ?? item?.content?.name ?? '').filter(Boolean);
295+
return items.map((item: any) => item?.name ?? '').filter(Boolean);
260296
}
261297

262298
/**

packages/objectql/src/metadata-service-roundtrip-conformance.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,26 @@ describe.each(IMPLEMENTATIONS)('#7378 ruled behaviour, beyond the table [$label]
385385
expect(await service.exists('view', 'pin_agreeing')).toBe(true);
386386
});
387387

388+
it('#7519: a document carrying a REAL `content` field round-trips WHOLE — `content` is authorable (doc.zod.ts / knowledge-document.zod.ts), never a storage envelope', async () => {
389+
// MetadataFacade unwrapped `item?.content ?? item` on every read, so a
390+
// registered doc came back as its Markdown STRING — truthy and defined,
391+
// which is why the assertion below names the document's keys rather
392+
// than stopping at toBeDefined(). The other three subjects always
393+
// passed this; it pins the whole family to one answer.
394+
const service = implementation.create();
395+
const markdown = '# Pin\n\nThe content field belongs to the author, not the store.';
396+
await service.register('doc', 'pin_content_field', {
397+
name: 'pin_content_field',
398+
label: 'Content pin',
399+
content: markdown,
400+
});
401+
const got = (await service.get('doc', 'pin_content_field')) as Record<string, unknown> | undefined;
402+
expect(got).toBeDefined();
403+
expect(got).toMatchObject({ name: 'pin_content_field', content: markdown });
404+
expect(await service.exists('doc', 'pin_content_field')).toBe(true);
405+
expect(await service.listNames('doc')).toContain('pin_content_field');
406+
});
407+
388408
it("row 2 converges in BOTH directions: register('object', …) is readable through the plural spelling", async () => {
389409
// The table's ruled row covers plural-write → singular-read; this is
390410
// the reverse read, so the fold cannot be a write-side special case —

0 commit comments

Comments
 (0)