From c8fd2b8613431403294fc542b1e0caf116eecfb8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 09:55:44 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(objectql):=20MetadataFacade=20reads=20r?= =?UTF-8?q?eturn=20the=20stored=20document=20=E2=80=94=20content=20is=20a?= =?UTF-8?q?=20real=20authorable=20field,=20not=20a=20storage=20envelope=20?= =?UTF-8?q?(#7519)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy --- packages/objectql/src/metadata-facade.test.ts | 96 +++++++++++++++++++ packages/objectql/src/metadata-facade.ts | 54 +++++++++-- ...data-service-roundtrip-conformance.test.ts | 20 ++++ 3 files changed, 161 insertions(+), 9 deletions(-) diff --git a/packages/objectql/src/metadata-facade.test.ts b/packages/objectql/src/metadata-facade.test.ts index 535a39987e..248158b024 100644 --- a/packages/objectql/src/metadata-facade.test.ts +++ b/packages/objectql/src/metadata-facade.test.ts @@ -228,3 +228,99 @@ describe('MetadataFacade object write/read round-trip', () => { await expect(facade.unregister('object', 'absent')).resolves.toBeUndefined(); }); }); + +/** + * [#7519] `content` is a REAL authorable field, not this facade's storage + * envelope. + * + * Every read member used to unwrap `item?.content ?? item`, presuming + * `content` marked the facade's own wrapper — but `doc.zod.ts` (raw Markdown) + * and `knowledge-document.zod.ts` both declare `content` as an authored + * field, so a doc registered through the facade read back as its Markdown + * STRING: truthy and string-typed, so nothing threw and downstream `?.name` + * reads silently yielded `undefined`. That is the silent non-round-trip the + * #7378 ruling (2026-08-12) forbids: `register(t, n, d)` → `get(t, n)` + * round-trips or refuses loudly. + * + * The envelope the unwrap presumed has NO producer (measured on `main`, not + * assumed — the full ledger is in `get`'s header in metadata-facade.ts), so + * the fix removes the unwrap rather than renaming the envelope key: any + * replacement key would merely reschedule this collision onto the next + * authorable field. + */ +describe('MetadataFacade reads return the stored document, not its `content` field (#7519)', () => { + let registry: SchemaRegistry; + let facade: MetadataFacade; + + beforeEach(() => { + registry = new SchemaRegistry({ multiTenant: false }); + facade = new MetadataFacade(registry); + }); + + const MARKDOWN = '# Getting started\n\nWrite your first object.'; + const docDocument = () => ({ + name: 'getting_started', + label: 'Getting started', + content: MARKDOWN, + }); + + it('get returns the registered doc document, not its Markdown string', async () => { + await facade.register('doc', 'getting_started', docDocument()); + + const got = (await facade.get('doc', 'getting_started')) as any; + // The defect shape was `got === MARKDOWN` — truthy and defined, so a + // bare toBeDefined() would have passed. Assert the DOCUMENT came back. + expect(got).toBeDefined(); + expect(got).not.toBe(MARKDOWN); + expect(got.name).toBe('getting_started'); + expect(got.content).toBe(MARKDOWN); + }); + + it('list returns doc documents, not Markdown strings', async () => { + await facade.register('doc', 'getting_started', docDocument()); + await facade.register('doc', 'faq', { name: 'faq', content: '# FAQ' }); + + const listed = (await facade.list('doc')) as any[]; + expect(listed).toHaveLength(2); + expect(listed.map((d) => d?.name).sort()).toEqual(['faq', 'getting_started']); + expect(listed.every((d) => typeof d === 'object' && d !== null)).toBe(true); + }); + + it('listNames reads names off the documents themselves', async () => { + await facade.register('doc', 'getting_started', docDocument()); + + expect(await facade.listNames('doc')).toEqual(['getting_started']); + }); + + it('exists and getEntry agree the document is there, whole', async () => { + await facade.register('doc', 'getting_started', docDocument()); + + expect(await facade.exists('doc', 'getting_started')).toBe(true); + const entry = facade.getEntry('doc', 'getting_started') as any; + expect(entry.content).toBe(MARKDOWN); + }); + + it('a knowledge_document with a `content` field round-trips whole too', async () => { + // The second live type the issue names (knowledge-document.zod.ts) — + // pinned so the fix cannot be read as doc-specific. + await facade.register('knowledge_document', 'onboarding_kb', { + name: 'onboarding_kb', + content: 'Answer the onboarding questions from this corpus.', + }); + + const got = (await facade.get('knowledge_document', 'onboarding_kb')) as any; + expect(got.name).toBe('onboarding_kb'); + expect(typeof got.content).toBe('string'); + }); + + it('a document WITHOUT a `content` field still round-trips unchanged (the surviving path)', async () => { + // The other direction pinned: removing the unwrap must not trade one + // class of correct read for another. Content-less items were read + // correctly before this fix (the `??` fell through) and must stay so. + await facade.register('view', 'plain_view', { name: 'plain_view', label: 'Plain', type: 'grid' }); + + const got = (await facade.get('view', 'plain_view')) as any; + expect(got).toMatchObject({ name: 'plain_view', label: 'Plain', type: 'grid' }); + expect(await facade.listNames('view')).toEqual(['plain_view']); + }); +}); diff --git a/packages/objectql/src/metadata-facade.ts b/packages/objectql/src/metadata-facade.ts index 719e0e00d8..48d6df63d0 100644 --- a/packages/objectql/src/metadata-facade.ts +++ b/packages/objectql/src/metadata-facade.ts @@ -188,7 +188,29 @@ export class MetadataFacade { } /** - * Get a metadata item by type and name. + * Get a metadata item by type and name — the STORED document, verbatim from + * the registry. + * + * [#7519] An `item?.content ?? item` unwrap used to sit on this return (and + * on {@link list} / {@link listNames}), presuming `content` was this + * facade's own storage envelope. But `content` is a REAL authorable field — + * `doc.zod.ts` (raw Markdown) and `knowledge-document.zod.ts` both declare + * it — so `register('doc', n, document)` → `get('doc', n)` answered the + * Markdown STRING instead of the document: truthy, string-typed, and silent + * (downstream `?.name` reads yield `undefined` rather than throwing) — + * exactly the silent non-round-trip the #7378 ruling forbids + * (`register(t, n, d)` → `get(t, n)` round-trips or is refused loudly). + * + * The envelope the unwrap presumed has NO producer. Measured, not assumed: + * the only writer that ever produced one was this class's own interim + * `{ name, content }` boxing of non-object values (#7511 cell 3), which + * #8349 removed in favour of the shared guard's loud refusal (see + * {@link toKeyedDefinition}); DB hydration (`loadMetaFromDb`, + * metadata-protocol) parses the `sys_metadata.metadata` column and registers + * the document itself; every other in-tree `registerItem` caller stores the + * document as-is. With no envelope on any write path, an unwrap on the read + * path can only corrupt — so there is none, and no replacement envelope key + * either (any key chosen would just collide with the next authorable field). * * `currentPackageId` (ADR-0048) opts into package-scoped resolution: when two * installed packages ship an item of the same `type`/`name`, the registry @@ -198,23 +220,28 @@ export class MetadataFacade { */ async get(type: string, name: string, currentPackageId?: string): Promise { // [#7378 row 2] Read the store `register` wrote: the canonical type. - const item = this.registry.getItem(canonicalMetadataServiceType(type), name, currentPackageId) as any; - return item?.content ?? item; + return this.registry.getItem(canonicalMetadataServiceType(type), name, currentPackageId); } /** - * Get the raw entry (with metadata wrapper) + * Get the raw stored entry, synchronously and without package-scoped + * resolution. ([#7519] Historically documented as "with metadata wrapper" — + * there is no wrapper; {@link get} returns the same stored document. This + * member survives as the sync, scope-free variant.) */ getEntry(type: string, name: string): any { return this.registry.getItem(canonicalMetadataServiceType(type), name); } /** - * List all items of a type + * List all items of a type — the stored documents, verbatim. + * + * [#7519] The former `item?.content ?? item` map is gone for the reason + * {@link get}'s header carries in full: `content` is a real authorable + * field, and the envelope the unwrap presumed has no producer. */ async list(type: string): Promise { - const items = this.registry.listItems(canonicalMetadataServiceType(type)); - return items.map((item: any) => item?.content ?? item); + return this.registry.listItems(canonicalMetadataServiceType(type)); } /** @@ -252,11 +279,20 @@ export class MetadataFacade { } /** - * List all names of metadata items of a given type + * List all names of metadata items of a given type. + * + * [#7519] The former `item?.content?.name` fallback limb was the same + * envelope presumption {@link get}'s header retires, one member over — and + * it was dead in both directions: on an envelope-shaped entry nothing + * produces it would have read a name out of the wrapper, and on a real + * `doc` (whose `content` is a Markdown STRING) `content?.name` is + * `undefined` anyway. Every admitted document carries `name` — + * {@link toKeyedDefinition} sets it from the argument on this class's own + * writes, and every in-tree `registerItem` caller keys by `name`. */ async listNames(type: string): Promise { const items = this.registry.listItems(canonicalMetadataServiceType(type)); - return items.map((item: any) => item?.name ?? item?.content?.name ?? '').filter(Boolean); + return items.map((item: any) => item?.name ?? '').filter(Boolean); } /** diff --git a/packages/objectql/src/metadata-service-roundtrip-conformance.test.ts b/packages/objectql/src/metadata-service-roundtrip-conformance.test.ts index be3fb8e092..0bed349062 100644 --- a/packages/objectql/src/metadata-service-roundtrip-conformance.test.ts +++ b/packages/objectql/src/metadata-service-roundtrip-conformance.test.ts @@ -385,6 +385,26 @@ describe.each(IMPLEMENTATIONS)('#7378 ruled behaviour, beyond the table [$label] expect(await service.exists('view', 'pin_agreeing')).toBe(true); }); + 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 () => { + // MetadataFacade unwrapped `item?.content ?? item` on every read, so a + // registered doc came back as its Markdown STRING — truthy and defined, + // which is why the assertion below names the document's keys rather + // than stopping at toBeDefined(). The other three subjects always + // passed this; it pins the whole family to one answer. + const service = implementation.create(); + const markdown = '# Pin\n\nThe content field belongs to the author, not the store.'; + await service.register('doc', 'pin_content_field', { + name: 'pin_content_field', + label: 'Content pin', + content: markdown, + }); + const got = (await service.get('doc', 'pin_content_field')) as Record | undefined; + expect(got).toBeDefined(); + expect(got).toMatchObject({ name: 'pin_content_field', content: markdown }); + expect(await service.exists('doc', 'pin_content_field')).toBe(true); + expect(await service.listNames('doc')).toContain('pin_content_field'); + }); + it("row 2 converges in BOTH directions: register('object', …) is readable through the plural spelling", async () => { // The table's ruled row covers plural-write → singular-read; this is // the reverse read, so the fold cannot be a write-side special case — From 18991f8a9aa2b07295c6cdf9b2761fa929afefc2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 10:05:26 +0000 Subject: [PATCH 2/2] chore: changeset for the MetadataFacade content-field round-trip fix (#7519) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy --- .changeset/metadata-facade-content-field-roundtrip.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/metadata-facade-content-field-roundtrip.md diff --git a/.changeset/metadata-facade-content-field-roundtrip.md b/.changeset/metadata-facade-content-field-roundtrip.md new file mode 100644 index 0000000000..eea30d0562 --- /dev/null +++ b/.changeset/metadata-facade-content-field-roundtrip.md @@ -0,0 +1,5 @@ +--- +"@objectstack/objectql": patch +--- + +`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.