refactor(metadata): split the stored envelope from the authored body at the api parse boundary (#5309)

.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.

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<string, unknown>;
+    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 `<unnamed>`', () => {
+    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();

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<string, ApiEndpoint>();
 
   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
-          : '<unnamed>';
+      const declaredName = storedItemName(peeled) ?? '<unnamed>';
       logger.error(
         `[EndpointMatcher] stored api item '${declaredName}' does not satisfy ApiEndpointSchema — ` +
           `it is EXCLUDED from endpoint matching and its declared route will answer 404. ` +

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({

packages/metadata/src/publish-endpoint-gate.test.ts

@@ -32,15 +32,36 @@ 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<string, unknown> = {}): Record<string, unknown> {
+/**
+ * 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<string, unknown> = {}): Record<string, unknown> {
   return {
     name: 'list_things',
     path: `/api/v1/apps/${NS}/things`,
     method: 'GET',
     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<string, unknown> = {}): Record<string, unknown> {
+  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<string, unknown>;
+      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<string, unknown>;
+      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({