Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/metadata-facade-content-field-roundtrip.md
Original file line numberDiff line numberDiff line change
@@ -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.
96 changes: 96 additions & 0 deletions packages/objectql/src/metadata-facade.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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']);
});
});
54 changes: 45 additions & 9 deletions packages/objectql/src/metadata-facade.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand All@@ -198,23 +220,28 @@ export class MetadataFacade {
*/
async get(type: string, name: string, currentPackageId?: string): Promise<any> {
// [#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<any[]> {
const items = this.registry.listItems(canonicalMetadataServiceType(type));
return items.map((item: any) => item?.content ?? item);
return this.registry.listItems(canonicalMetadataServiceType(type));
}

/**
Expand DownExpand Up@@ -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<string[]> {
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);
}

/**
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<string, unknown> | 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 —
Expand Down
Loading