From 38bcd17624dcaf946a5d4a867480ac78f527884b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 05:09:28 +0000 Subject: [PATCH] refactor(metadata): split the stored envelope from the authored body at the `api` parse boundary (#5309) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ApiEndpointSchema` is parsed against STORED ROWS at two sites — `buildEndpointIndex` (the load-time backstop) and `gateApiItemsForPublish` (the publish gate) — and a stored row carries the metadata layer's own bookkeeping (`packageId`, `state`, `version`, `published*`), which is not endpoint vocabulary. Only the schema's unknown-key STRIPPING kept those keys from being judged, which is why `api` could not leave #4001's STILL_STRIP list. `peelStoredEnvelope` (new `packages/metadata/src/stored-envelope.ts`) takes the envelope off before the body parse, using the metadata layer's own existing body-selection rule (`metadata ?? row`) plus the declared bookkeeping key list. It returns views and never mutates the row, so every existing envelope reader is untouched and `publishedDefinition` still snapshots the same bytes. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA --- .../metadata-stored-envelope-body-split.md | 42 ++++ .../metadata/src/endpoint-matcher.test.ts | 93 ++++++++- packages/metadata/src/endpoint-matcher.ts | 32 ++- packages/metadata/src/metadata-manager.ts | 26 ++- .../src/publish-endpoint-gate.test.ts | 84 +++++++- packages/metadata/src/stored-envelope.test.ts | 165 +++++++++++++++ packages/metadata/src/stored-envelope.ts | 193 ++++++++++++++++++ 7 files changed, 618 insertions(+), 17 deletions(-) create mode 100644 .changeset/metadata-stored-envelope-body-split.md create mode 100644 packages/metadata/src/stored-envelope.test.ts create mode 100644 packages/metadata/src/stored-envelope.ts diff --git a/.changeset/metadata-stored-envelope-body-split.md b/.changeset/metadata-stored-envelope-body-split.md new file mode 100644 index 0000000000..8c4a3f4de9 --- /dev/null +++ b/.changeset/metadata-stored-envelope-body-split.md @@ -0,0 +1,42 @@ +--- +"@objectstack/metadata": patch +--- + +refactor(metadata): peel the stored envelope before an `api` row is parsed as an endpoint (#5309) + +Internal refactor — no authored format changes, no observable acceptance change +for the shapes the platform stores today. + +A metadata *type name* is worn by two different documents: the **authored +declaration** (exactly its spec vocabulary) and the **stored row** (that +declaration plus the metadata layer's own bookkeeping — `packageId`, `state`, +`version`, `publishedDefinition`, `publishedAt`, `publishedBy`, written by +`MetadataManager.register` / `publishPackage` and read back by `publishPackage`'s +package filter). Both `ApiEndpointSchema` parse sites — `buildEndpointIndex` (the +load-time backstop) and `gateApiItemsForPublish` (the publish gate) — used to hand +the whole stored row to the schema, and only its unknown-key *stripping* kept the +bookkeeping from being judged as endpoint vocabulary. + +`peelStoredEnvelope` (`packages/metadata/src/stored-envelope.ts`) now takes the +envelope off first, so the schema sees the authored body and nothing else: + +- a row carrying a `metadata` value IS an envelope around it — the body is that + value, everything beside it is bookkeeping. This is the `data.metadata ?? data` + rule the publish gate, `publishedDefinition` and `getPublished` already shared; +- otherwise the body is the row minus the declared bookkeeping keys. + +The peel returns views and never mutates the row, so every existing envelope +reader (`publishPackage`'s `packageId` filter, `query`'s `state` / `packageId` +filters, `revertPackage`) is untouched, and `publishedDefinition` still snapshots +`data.metadata ?? data` verbatim. + +One consequence worth naming: `buildEndpointIndex` was the last reader that did +NOT follow the layer's body-selection rule, so a publish envelope +(`{ name, packageId, state, metadata: {…} }`) used to pass the publish gate and +then be excluded from the endpoint index — its route answered 404. The two doors +now read the same document. + +This is the prerequisite for tightening `ApiEndpointSchema` (#5384): with the +schema flipped to `strictObject` locally, `packages/metadata` went from 11 failing +tests to 1, and the one left is an authored non-vocabulary key being refused by +name — which is what that tightening is for. diff --git a/packages/metadata/src/endpoint-matcher.test.ts b/packages/metadata/src/endpoint-matcher.test.ts index 2ad68e57bc..7f57ff4e7f 100644 --- a/packages/metadata/src/endpoint-matcher.test.ts +++ b/packages/metadata/src/endpoint-matcher.test.ts @@ -150,9 +150,16 @@ describe('buildEndpointIndex', () => { // What remains true, and is what this case now pins: a stored row carries // BOTH the ADR-0010 envelope (declared ⇒ survives) and the metadata layer's // own bookkeeping (`packageId` / `state`, written by `MetadataManager` and - // NOT endpoint vocabulary ⇒ stripped). That split is the measured reason - // `api` sits on the #4001 campaign's STILL_STRIP list: closing this shape - // would make every stored row unparseable here. + // NOT endpoint vocabulary ⇒ never reaches the body parse). + // + // [#5309] The REASON the bookkeeping is gone changed, and that is the whole + // point of the split. It used to be the schema's unknown-key STRIPPING that + // ate it — which is why `api` could not leave #4001's STILL_STRIP list + // (#5271 measured it: closing the shape made every stored row fail with + // `unrecognized_keys: ['packageId', 'state']`). Now `peelStoredEnvelope` + // takes the envelope off BEFORE `ApiEndpointSchema` sees anything, so this + // case passes on a strict schema too. `stored-envelope.test.ts` pins the peel + // itself; the cases below pin it end to end through the index. it('keeps the ADR-0010 envelope and strips the metadata layer’s bookkeeping', () => { const index = buildEndpointIndex( [{ @@ -179,6 +186,86 @@ describe('buildEndpointIndex', () => { }); }); +// ── [#5309] The stored ENVELOPE / authored BODY split ────────────────────── +// +// These pin the peel where it matters — through the index — rather than only +// in `stored-envelope.test.ts`. The measurement they exist to protect: with +// `ApiEndpointSchema` flipped to `strictObject` (a local probe, never +// committed — that flip is #5384's), a stored row used to fail with +// `unrecognized_keys: ['packageId', 'state']` and its route answered 404. It +// no longer can, because the envelope never reaches the schema. +describe('#5309 — the envelope never reaches the body parse', () => { + /** Every bookkeeping key `publishPackage` stamps onto a published row. */ + const publishEnvelope = { + packageId: 'com.objectstack.showcase', + package: 'com.objectstack.showcase', + state: 'active', + version: 3, + publishedAt: '2026-08-08T00:00:00.000Z', + publishedBy: 'usr_admin', + publishedDefinition: { name: 'list_tasks', path: '/api/v1/apps/showcase/tasks' }, + }; + + it('indexes a FLAT published row and hands back a body carrying no bookkeeping', () => { + const logger = makeLogger(); + const index = buildEndpointIndex([{ ...endpoint(), ...publishEnvelope }], logger); + + const hit = index.get('GET /api/v1/apps/showcase/tasks'); + expect(hit, 'a published row must index through the split').toBeDefined(); + // The declaration survives whole … + expect(hit!.name).toBe('list_tasks'); + expect(hit!.target).toBe('showcase_task'); + // … the schema default is still materialized (contract, not incidental) … + expect(hit!.authRequired).toBe(true); + // … and not one bookkeeping key is on the answer. + const body = hit as unknown as Record; + for (const key of Object.keys(publishEnvelope)) { + expect(body, `envelope key '${key}' leaked into the endpoint body`).not.toHaveProperty(key); + } + expect(logger.error).not.toHaveBeenCalled(); + }); + + it('indexes a WRAPPED publish envelope off its `metadata` body', () => { + // `{ name, packageId, state, metadata: {…} }` is the envelope shape + // `publishPackage` gates (`data.metadata ?? data`) and `revertPackage` + // writes. This index is now the same reader: before the split it parsed the + // OUTER object, found no `path`/`method`, and excluded a row publish had + // just accepted — the two doors disagreeing about what the document is. + const logger = makeLogger(); + const index = buildEndpointIndex( + [{ name: 'list_tasks', packageId: 'com.objectstack.showcase', state: 'active', metadata: endpoint() }], + logger, + ); + + const hit = index.get('GET /api/v1/apps/showcase/tasks'); + expect(hit, 'a publish envelope must index off its body').toBeDefined(); + expect(hit!.name).toBe('list_tasks'); + expect(logger.error).not.toHaveBeenCalled(); + }); + + it('names an unparseable WRAPPED row by its envelope `name`, not ``', () => { + const logger = makeLogger(); + const index = buildEndpointIndex( + [{ name: 'broken_ep', packageId: 'com.objectstack.showcase', metadata: { path: '/api/v1/apps/showcase/x' } }], + logger, + ); + + expect(index.size).toBe(0); + expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.error.mock.calls[0][0]).toContain('broken_ep'); + }); + + it('leaves an authored declaration — no envelope at all — byte-identical', () => { + const authored = endpoint(); + const before = structuredClone(authored); + const index = buildEndpointIndex([authored], makeLogger()); + + expect(index.get('GET /api/v1/apps/showcase/tasks')).toBeDefined(); + // The peel is a VIEW: it must never mutate the row it was handed. + expect(authored).toEqual(before); + }); +}); + describe('parse failure — loud skip, no collateral damage', () => { it('skips an unparseable stored item, logs it at error level, and keeps the good ones', () => { const logger = makeLogger(); diff --git a/packages/metadata/src/endpoint-matcher.ts b/packages/metadata/src/endpoint-matcher.ts index ed9d748759..bdb1f29cf2 100644 --- a/packages/metadata/src/endpoint-matcher.ts +++ b/packages/metadata/src/endpoint-matcher.ts @@ -38,6 +38,24 @@ * §7-5 keeps RFC 3986 canonicalization as an explicit open question — it is * a vocabulary-level decision, not an implementation detail to smuggle in.) * + * ## Envelope off, body parsed (#5309) + * + * What arrives here is a STORED ROW, not an authored declaration: the metadata + * layer wraps every row in its own bookkeeping — `packageId`, `state`, + * `version`, `publishedDefinition`, … — either flat beside the body or under a + * `metadata` key (the publish envelope). None of that is endpoint vocabulary, + * so `peelStoredEnvelope` takes it off and `ApiEndpointSchema` sees the + * authored body alone. Before the split, the schema's unknown-key STRIPPING + * was load-bearing here: it silently ate the bookkeeping, which is exactly why + * `api` could not be closed (#5271's measurement, #5384's job). Stripping is no + * longer what makes a stored row parse. + * + * The body-selection half (`metadata ?? row`) is the metadata layer's existing + * rule — `publishPackage`'s snapshot, `getPublished`, `gateApiItemsForPublish` + * all use it — and this module now follows it too, so a publish envelope that + * passes the publish gate is the same document this index serves. It used to + * be the one reader that disagreed. + * * ## Loud, never half-valid * * Every stored item is `ApiEndpointSchema.safeParse`-d. The answer handed back @@ -108,6 +126,7 @@ import { } from '@objectstack/spec/api'; import type { ApiEndpointMatch } from '@objectstack/spec/contracts'; import type { Logger } from '@objectstack/spec/contracts'; +import { peelStoredEnvelope, storedItemName } from './stored-envelope.js'; /** * Upper-case a request verb so `method` compares case-insensitively. @@ -158,15 +177,18 @@ export function buildEndpointIndex(items: readonly unknown[], logger: Logger): E const index = new Map(); for (const item of items) { - const parsed = ApiEndpointSchema.safeParse(item); + // [#5309] Envelope OFF before the body parse. A stored row carries the + // metadata layer's bookkeeping (`packageId`, `state`, …) which is not + // endpoint vocabulary; `ApiEndpointSchema` judges the authored body and + // nothing else. See `stored-envelope.ts` for why this is the fix rather + // than teaching the vocabulary two storage keys. + const peeled = peelStoredEnvelope(item); + const parsed = ApiEndpointSchema.safeParse(peeled.body); if (!parsed.success) { // LOUD skip (contract: "MUST skip (loudly) any stored item that fails to // parse rather than returning a half-valid shape"). Name the item so the // author can find it; say what the consequence is. - const declaredName = - item && typeof item === 'object' && typeof (item as { name?: unknown }).name === 'string' - ? (item as { name: string }).name - : ''; + const declaredName = storedItemName(peeled) ?? ''; logger.error( `[EndpointMatcher] stored api item '${declaredName}' does not satisfy ApiEndpointSchema — ` + `it is EXCLUDED from endpoint matching and its declared route will answer 404. ` + diff --git a/packages/metadata/src/metadata-manager.ts b/packages/metadata/src/metadata-manager.ts index a55537a0d8..9346e1fc01 100644 --- a/packages/metadata/src/metadata-manager.ts +++ b/packages/metadata/src/metadata-manager.ts @@ -71,6 +71,9 @@ import type { MetaRef, } from '@objectstack/metadata-core'; import { EndpointMatcher } from './endpoint-matcher.js'; +// [#5309] The stored ENVELOPE / authored BODY split — peeled before any spec +// schema parses a stored row. See `stored-envelope.ts`. +import { peelStoredEnvelope } from './stored-envelope.js'; import type { ApiEndpointMatch } from '@objectstack/spec/contracts'; /** @@ -1659,10 +1662,16 @@ export class MetadataManager implements IMetadataService { * ## What it judges, and on what * * The registry stores either a raw spec document or a publish envelope - * (`{ name, packageId, state, metadata: {…spec} }`); the endpoint is read - * out with the SAME rule this method's caller uses for - * `publishedDefinition` (`data.metadata ?? data`), so publish gates exactly - * the document publish is about to snapshot. An item that does not satisfy + * (`{ name, packageId, state, metadata: {…spec} }`), and in BOTH shapes the + * row carries the metadata layer's bookkeeping. [#5309] The envelope is + * peeled off first (`peelStoredEnvelope`) and the gate judges the authored + * BODY: the wrapped half of that peel is the `data.metadata ?? data` rule + * this method used to spell inline — the same document `publishedDefinition` + * snapshots — and the flat half additionally removes `packageId` / `state` / + * `version` / `published*`, which are storage identity, never endpoint + * vocabulary. (What publish SNAPSHOTS is unchanged: `publishedDefinition` + * still stores `data.metadata ?? data` verbatim, envelope included, because + * `revertPackage` restores from it.) An item whose body does not satisfy * `ApiEndpointSchema` fails here too — not extra strictness but a * precondition: an unparsed shape cannot be gated, and it could never be * served either (the matcher's own loud skip refuses it at load). @@ -1686,8 +1695,13 @@ export class MetadataManager implements IMetadataService { const gatedItems: Array<{ name: string }> = []; for (const item of apiItems) { - const document = item.data?.metadata ?? item.data; - const parsed = ApiEndpointSchema.safeParse(document); + // [#5309] Envelope OFF before the body parse — the same peel the load-time + // backstop applies (`buildEndpointIndex`), so the two doors judge the same + // document. The wrapped half of the rule is the `data.metadata ?? data` + // this line used to spell inline; the flat half additionally takes off the + // bookkeeping (`packageId`, `state`, …) that shares a level with the body. + const { body } = peelStoredEnvelope(item.data); + const parsed = ApiEndpointSchema.safeParse(body); if (!parsed.success) { for (const issue of parsed.error.issues) { errors.push({ diff --git a/packages/metadata/src/publish-endpoint-gate.test.ts b/packages/metadata/src/publish-endpoint-gate.test.ts index 36b3766913..65e532d306 100644 --- a/packages/metadata/src/publish-endpoint-gate.test.ts +++ b/packages/metadata/src/publish-endpoint-gate.test.ts @@ -32,8 +32,17 @@ vi.mock('@objectstack/core', () => ({ const PKG = 'com.acme.endpoints'; const NS = 'acme'; -/** A stored `api` item that passes every gate under `namespace: 'acme'`. */ -function apiItem(over: Record = {}): Record { +/** + * The AUTHORED endpoint document — exactly `ApiEndpointSchema` vocabulary, + * nothing else. Passes every gate under `namespace: 'acme'`. + * + * [#5309] Split out of `apiItem` so the publish-envelope case below can put a + * real authored body under `metadata`. It used to nest `apiItem()` there, which + * put `packageId` / `state` INSIDE the body as well as on the envelope — an + * accident of fixture reuse that only the schema's unknown-key stripping made + * invisible. + */ +function endpointBody(over: Record = {}): Record { return { name: 'list_things', path: `/api/v1/apps/${NS}/things`, @@ -41,6 +50,18 @@ function apiItem(over: Record = {}): Record { type: 'object_operation', target: 'acme_thing', objectParams: { object: 'acme_thing', operation: 'find' }, + ...over, + }; +} + +/** + * A STORED `api` item in the flat shape: the authored body plus the metadata + * layer's bookkeeping at one level, which is what `MetadataManager.register` + * holds and what `publishPackage` filters by. + */ +function apiItem(over: Record = {}): Record { + return { + ...endpointBody(), packageId: PKG, state: 'draft', ...over, @@ -199,7 +220,9 @@ describe('#5189 — publishPackage gates `api` items', () => { name: 'open_things', packageId: PKG, state: 'draft', - metadata: apiItem({ name: 'open_things', authRequired: false }), + // [#5309] The body under `metadata` is the AUTHORED document; the + // bookkeeping belongs on the envelope around it, and only there. + metadata: endpointBody({ name: 'open_things', authRequired: false }), }); const result = await manager.publishPackage(PKG, { namespace: NS }); @@ -223,6 +246,61 @@ describe('#5189 — publishPackage gates `api` items', () => { expect(result.itemsPublished).toBe(2); }); + // ── [#5309] The stored ENVELOPE / authored BODY split ──────────────── + // + // `apiItem()` above is already a stored row: it carries `packageId` and + // `state`, which are storage identity and not endpoint vocabulary. Every + // case in this file therefore depends on those keys not being judged as + // vocabulary — silently, via the schema's unknown-key STRIPPING, until the + // split made it explicit. These two pin it directly, so the dependency is + // stated rather than assumed. + it('#5309 — gates the BODY: a fully-published row yields the D6 verdict, not a schema error', async () => { + await manager.register('api', 'open_things', apiItem({ + name: 'open_things', + authRequired: false, + // Everything `publishPackage` stamps onto a row it has published once. + version: 2, + publishedAt: '2026-08-08T00:00:00.000Z', + publishedBy: 'usr_admin', + publishedDefinition: { name: 'open_things' }, + package: PKG, + state: 'active', + })); + + const result = await manager.publishPackage(PKG, { namespace: NS }); + + expect(result.success).toBe(false); + const messages = (result.validationErrors ?? []).map(e => e.message); + // The verdict the gate exists to give … + expect(messages.some(m => m.includes('authRequired: false'))).toBe(true); + // … and NOT the "cannot be gated" precondition failure, which is what a + // schema that judged the bookkeeping would produce instead. + expect(messages.some(m => m.includes('does not satisfy ApiEndpointSchema'))).toBe(false); + }); + + it('#5309 — a published row still indexes: publish and the matcher read the same body', async () => { + // End to end through both doors of the split. `publishPackage` re-registers + // the row with `state`/`version`/`published*` stamped on; the load-time + // backstop must still find the declaration underneath. + await manager.register('api', 'list_things', apiItem()); + + const published = await manager.publishPackage(PKG, { namespace: NS }); + expect(published.success).toBe(true); + + const stored = (await manager.get('api', 'list_things')) as Record; + expect(stored.state).toBe('active'); + expect(stored.version).toBe(1); + + const match = await manager.matchEndpoint({ method: 'GET', path: `/api/v1/apps/${NS}/things` }); + expect(match?.endpoint.name).toBe('list_things'); + expect(match?.endpoint.authRequired).toBe(true); + // The served declaration carries no storage bookkeeping. + const body = match!.endpoint as unknown as Record; + expect(body).not.toHaveProperty('packageId'); + expect(body).not.toHaveProperty('state'); + expect(body).not.toHaveProperty('publishedDefinition'); + }); + it('fails the WHOLE publish, leaving the good items unpublished (publish is atomic)', async () => { await manager.register('api', 'good_things', apiItem({ name: 'good_things' })); await manager.register('api', 'open_things', apiItem({ diff --git a/packages/metadata/src/stored-envelope.test.ts b/packages/metadata/src/stored-envelope.test.ts new file mode 100644 index 0000000000..d767803cf2 --- /dev/null +++ b/packages/metadata/src/stored-envelope.test.ts @@ -0,0 +1,165 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5309 — the stored ENVELOPE / authored BODY split. + * + * `peelStoredEnvelope` is the parse boundary between the metadata layer's + * bookkeeping and the document a spec schema is entitled to judge. Two callers + * depend on it agreeing with itself: `buildEndpointIndex` (the load-time + * backstop) and `gateApiItemsForPublish` (the publish gate). Every clause of + * the rule is pinned here, because the two callers' own tests can only observe + * its consequences. + * + * The rule, restated: a row that carries a `metadata` value IS an envelope + * around it (everything beside it is bookkeeping, by construction); otherwise + * the body is the row minus the declared `STORED_ENVELOPE_KEYS`. + */ + +import { describe, it, expect } from 'vitest'; +import { + peelStoredEnvelope, + storedItemName, + STORED_ENVELOPE_KEYS, + STORED_BODY_KEY, +} from './stored-envelope.js'; + +const AUTHORED = { + name: 'list_tasks', + path: '/api/v1/apps/showcase/tasks', + method: 'GET', + type: 'object_operation', + target: 'showcase_task', +} as const; + +describe('peelStoredEnvelope — the flat shape', () => { + it('takes every declared bookkeeping key off the body and keeps it on the envelope', () => { + const row = { + ...AUTHORED, + package: 'com.objectstack.showcase', + packageId: 'com.objectstack.showcase', + state: 'active', + version: 3, + publishedAt: '2026-08-08T00:00:00.000Z', + publishedBy: 'usr_admin', + publishedDefinition: { name: 'list_tasks' }, + }; + + const { envelope, body, wrapped } = peelStoredEnvelope(row); + + expect(wrapped).toBe(false); + expect(body).toEqual(AUTHORED); + for (const key of STORED_ENVELOPE_KEYS) { + expect(body, `'${key}' must not reach the body`).not.toHaveProperty(key); + expect(envelope, `'${key}' must stay on the envelope`).toHaveProperty(key); + } + expect(envelope.packageId).toBe('com.objectstack.showcase'); + expect(envelope.state).toBe('active'); + }); + + it('keeps a key that is body vocabulary — the list is bookkeeping, not a denylist of names', () => { + // `name` is written by nobody but the author and is required by every + // metadata schema; it must survive the peel on BOTH shapes. + const { body } = peelStoredEnvelope({ ...AUTHORED, packageId: 'pkg' }); + expect((body as Record).name).toBe('list_tasks'); + }); + + it('hands back the SAME object when the row carries no bookkeeping at all', () => { + // An authored declaration must not even change identity: nothing about an + // authoring parse may shift because storage learned to peel. + const authored = { ...AUTHORED }; + const { envelope, body, wrapped } = peelStoredEnvelope(authored); + + expect(body).toBe(authored); + expect(wrapped).toBe(false); + expect(envelope).toEqual({}); + }); + + it('never mutates the row it was handed', () => { + const row = { ...AUTHORED, packageId: 'pkg', state: 'draft' }; + const before = structuredClone(row); + + peelStoredEnvelope(row); + + expect(row).toEqual(before); + }); +}); + +describe('peelStoredEnvelope — the publish envelope shape', () => { + it('reads the body out of `metadata` and treats every sibling key as envelope', () => { + const row = { + name: 'list_tasks', + packageId: 'com.objectstack.showcase', + state: 'draft', + // An outer key that is NOT on the declared list is still envelope here: + // the row said so by wrapping its body. + updatedAt: '2026-08-08T00:00:00.000Z', + [STORED_BODY_KEY]: AUTHORED, + }; + + const { envelope, body, wrapped } = peelStoredEnvelope(row); + + expect(wrapped).toBe(true); + expect(body).toBe(row.metadata); + expect(envelope).toEqual({ + name: 'list_tasks', + packageId: 'com.objectstack.showcase', + state: 'draft', + updatedAt: '2026-08-08T00:00:00.000Z', + }); + }); + + it('falls through to the flat shape when `metadata` is null or undefined', () => { + // Mirrors the `data.metadata ?? data` rule this peel replaces — a row with + // an empty body slot is judged as the flat row it otherwise is. + for (const empty of [null, undefined]) { + const { body, wrapped } = peelStoredEnvelope({ ...AUTHORED, packageId: 'pkg', metadata: empty }); + expect(wrapped).toBe(false); + expect(body).toEqual(AUTHORED); + } + }); + + it('hands a non-object `metadata` through as the body, for the schema to refuse', () => { + // Peeling is not judging: a body of the wrong TYPE is the schema's finding + // to report, with its own message, not something to silently repair here. + const { body, wrapped } = peelStoredEnvelope({ name: 'x', metadata: 'not-a-document' }); + expect(wrapped).toBe(true); + expect(body).toBe('not-a-document'); + }); +}); + +describe('peelStoredEnvelope — degenerate inputs stay the schema’s problem', () => { + it.each([ + ['undefined', undefined], + ['null', null], + ['a string', 'nope'], + ['a number', 7], + ['an array', [{ name: 'x' }]], + ])('hands %s straight back as the body with an empty envelope', (_label, input) => { + const { envelope, body, wrapped } = peelStoredEnvelope(input); + expect(body).toBe(input); + expect(wrapped).toBe(false); + expect(envelope).toEqual({}); + }); +}); + +describe('storedItemName — the row’s own name, for a diagnostic', () => { + it('reads the flat shape’s body name', () => { + expect(storedItemName(peelStoredEnvelope({ ...AUTHORED, packageId: 'pkg' }))).toBe('list_tasks'); + }); + + it('prefers the envelope name on the publish envelope shape', () => { + // Same answer `item.name` gave before the split — the outer name is the + // name the metadata layer knows the row by. + const peeled = peelStoredEnvelope({ name: 'outer_name', metadata: { ...AUTHORED, name: 'inner_name' } }); + expect(storedItemName(peeled)).toBe('outer_name'); + }); + + it('falls back to the body name when the envelope carries none', () => { + expect(storedItemName(peelStoredEnvelope({ metadata: AUTHORED }))).toBe('list_tasks'); + }); + + it('returns undefined when no name can be found — the caller decides the placeholder', () => { + expect(storedItemName(peelStoredEnvelope({ path: '/x' }))).toBeUndefined(); + expect(storedItemName(peelStoredEnvelope(undefined))).toBeUndefined(); + }); +}); diff --git a/packages/metadata/src/stored-envelope.ts b/packages/metadata/src/stored-envelope.ts new file mode 100644 index 0000000000..9ac6b99704 --- /dev/null +++ b/packages/metadata/src/stored-envelope.ts @@ -0,0 +1,193 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The stored ENVELOPE / authored BODY split (#5309). + * + * ## The defect this exists to remove + * + * A metadata *type name* is worn by two different documents in this codebase: + * + * - the **authored declaration** — exactly the vocabulary its spec schema + * declares, written by a human or (ADR-0033) an AI author; + * - the **stored row** — that declaration plus the metadata layer's own + * bookkeeping: `packageId`, `state`, `version`, `publishedDefinition`, + * `publishedAt`, `publishedBy`, written by {@link + * import('./metadata-manager.js').MetadataManager.register} and + * `publishPackage`, and read back by `publishPackage`'s own package filter. + * + * Those bookkeeping keys are NOT vocabulary. As long as a spec schema strips + * unknown keys the difference is invisible — and that is precisely the problem: + * the same permissiveness that lets `packageId` through also lets a `cacheTTL` + * / `outputMappings` typo through, so an author's policy or projection silently + * does not ship (ADR-0078). `api` sits on the #4001 campaign's `STILL_STRIP` + * list for exactly this reason, measured in #5271: closing `ApiEndpointSchema` + * made every stored row fail with `unrecognized_keys: ['packageId', 'state']`, + * so the load-time backstop excluded the endpoint (its route answered 404) and + * the publish gate reported a schema error instead of its ADR-0121 D6 verdict. + * + * The fix is NOT to teach a vocabulary two storage keys — that would make the + * author-facing contract describe the storage layer, the trade Prime Directive + * #12 refuses. It is to peel the envelope off **before** the body is handed to + * its schema, which is what this module does. + * + * ## The rule, in one sentence + * + * A stored row is an envelope around a body: when the row carries a + * {@link STORED_BODY_KEY} value the body is that value and everything beside + * it is envelope; otherwise the body is the row minus the declared + * {@link STORED_ENVELOPE_KEYS}. + * + * Neither half is invented here. `metadata ?? row` is already the metadata + * layer's body-selection rule in three places — `publishPackage`'s + * `publishedDefinition` snapshot, `getPublished`'s fallback, and + * `gateApiItemsForPublish` — and {@link STORED_ENVELOPE_KEYS} is the list of + * keys `MetadataManager` itself writes onto, or filters rows by. This module + * gives that rule ONE spelling so the two `ApiEndpointSchema` parse sites + * (`buildEndpointIndex`, `gateApiItemsForPublish`) can never disagree about + * what the document is. + * + * ## What it deliberately does NOT do + * + * - **It never mutates the row.** It returns views, so every existing reader + * of the envelope (`publishPackage`'s `meta?.packageId === packageId` + * filter, `query`'s `state` / `packageId` filters, `revertPackage`) keeps + * reading exactly the object it read before. + * - **It does not re-shape what publish snapshots.** `publishedDefinition` + * keeps its current bytes (`structuredClone(data.metadata ?? data)`), + * envelope keys included, because `revertPackage` restores from it. Peeling + * the snapshot is a stored-format change; this module is the parse + * boundary only. + * - **It is not a schema.** It removes keys the storage layer put there; it + * makes no judgement about the body, which is the schema's job and stays + * the schema's job. + * + * @see packages/spec/src/api/endpoint.zod.ts — the `STILL_STRIP` note this + * split is the prerequisite for closing (#5384 owns the closing itself). + */ + +/** + * The metadata layer's bookkeeping keys on a stored row. + * + * Every entry is written by `MetadataManager` or read by it as row identity — + * none of them is authorable vocabulary for any metadata type: + * + * | key | written by | read by | + * |---|---|---| + * | `packageId` | callers / publish envelope | `publishPackage`, `unregisterPackage`, `revertPackage`, `query`, realtime event | + * | `package` | legacy package stamp | the same three package filters | + * | `state` | `publishPackage`, `revertPackage` | `query`, the dependency-published check | + * | `version` | `publishPackage` | `publishPackage` (next version) | + * | `publishedDefinition` | `publishPackage` | `revertPackage`, `getPublished`, the dependency-published check | + * | `publishedAt` / `publishedBy` | `publishPackage` | publish bookkeeping | + * + * Adding a key here is a real decision: it removes that key from every body a + * peeled parse sees. Add one only when `MetadataManager` itself writes or + * filters on it. + */ +export const STORED_ENVELOPE_KEYS = Object.freeze([ + 'package', + 'packageId', + 'publishedAt', + 'publishedBy', + 'publishedDefinition', + 'state', + 'version', +] as const); + +/** + * The key a stored row uses to carry its authored body — the publish envelope + * shape `{ name, packageId, state, metadata: {…authored} }`. + */ +export const STORED_BODY_KEY = 'metadata'; + +const ENVELOPE_KEYS = new Set([...STORED_ENVELOPE_KEYS, STORED_BODY_KEY]); + +const EMPTY_ENVELOPE: Readonly> = Object.freeze({}); + +/** A stored row split into the layer's bookkeeping and the authored document. */ +export interface PeeledStoredItem { + /** + * The bookkeeping the metadata layer wrote around the body. Frozen, and a + * VIEW — the stored row itself is untouched. + */ + readonly envelope: Readonly>; + /** + * The authored document, ready for its spec schema. `unknown` on purpose: + * peeling an envelope says nothing about whether the body is valid. + */ + readonly body: unknown; + /** `true` when the row carried its body under {@link STORED_BODY_KEY}. */ + readonly wrapped: boolean; +} + +/** + * Split a stored metadata row into its envelope and its authored body. + * + * Total by construction — a non-object row (or `undefined`) is handed back as + * the body with an empty envelope, so the caller's schema reports the real + * problem instead of this function inventing one. + * + * @param item a value read out of the registry or a loader. + */ +export function peelStoredEnvelope(item: unknown): PeeledStoredItem { + if (item === null || typeof item !== 'object' || Array.isArray(item)) { + return { envelope: EMPTY_ENVELOPE, body: item, wrapped: false }; + } + + const row = item as Record; + const wrappedBody = row[STORED_BODY_KEY]; + + // The publish-envelope shape. Everything beside the body IS envelope by + // construction, so no key list is consulted — the row said so itself. + // `!= null` (not `in`) mirrors the `data.metadata ?? data` rule this + // replaces, so a row with `metadata: null` keeps falling through to the flat + // branch exactly as it does today. + if (wrappedBody !== undefined && wrappedBody !== null) { + const envelope: Record = {}; + for (const key of Object.keys(row)) { + if (key === STORED_BODY_KEY) continue; + envelope[key] = row[key]; + } + return { envelope: Object.freeze(envelope), body: wrappedBody, wrapped: true }; + } + + // The flat shape: body and bookkeeping share one level, so the declared key + // list is the only thing that can tell them apart. + let envelope: Record | undefined; + for (const key of Object.keys(row)) { + if (!ENVELOPE_KEYS.has(key)) continue; + envelope ??= {}; + envelope[key] = row[key]; + } + + // An authored declaration carries no bookkeeping at all — hand back the very + // object that came in, so nothing about an authored parse changes, not even + // object identity. + if (!envelope) return { envelope: EMPTY_ENVELOPE, body: row, wrapped: false }; + + const body: Record = {}; + for (const key of Object.keys(row)) { + if (ENVELOPE_KEYS.has(key)) continue; + body[key] = row[key]; + } + return { envelope: Object.freeze(envelope), body, wrapped: false }; +} + +/** + * The `name` a stored row is known by, for a diagnostic that must name the + * offending row even when its body failed to parse. + * + * Reads the envelope first and the body second, which is the SAME answer + * `item.name` gave before the split: on the flat shape `name` lives in the + * body, on the publish envelope it lives outside `metadata`. + */ +export function storedItemName(peeled: PeeledStoredItem): string | undefined { + const fromEnvelope = peeled.envelope.name; + if (typeof fromEnvelope === 'string') return fromEnvelope; + const body = peeled.body; + if (body && typeof body === 'object' && !Array.isArray(body)) { + const fromBody = (body as { name?: unknown }).name; + if (typeof fromBody === 'string') return fromBody; + } + return undefined; +}