fix(metadata-protocol): saveMetaItem's missing-item refusal declares 400 INVALID_REQUEST instead of answering 500

.changeset/hungry-donkeys-shout.md

@@ -0,0 +1,9 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+Declare `saveMetaItem`'s missing-item refusal as a real ADR-0112 envelope: `400` / `INVALID_REQUEST`, was an undeclared throw served as `500 INTERNAL_ERROR`.
+
+`PUT /api/v1/meta/:type/:name` unwraps the `{ item }` / `{ metadata }` envelope shapes before calling the protocol, so a caller sending `{"item": null}` or `{"metadata": null}` reached a guard that declared neither `code` nor `status` — the only refusal in the method that did not. With no status to read, the REST boundary defaulted to a server fault, so an authoring mistake was reported as `500 INTERNAL_ERROR` and the guard's own sentence was withheld by the ADR-0112 disclosure rule and replaced with a generic fallback. Callers now receive `400` with the refusal quoted and the remedy named.
+
+Unchanged: a missing, empty or literal-`null` request body never reached this guard and still answers `422 INVALID_METADATA` from the per-type schema parse. No new error code is introduced — `INVALID_REQUEST` is already registered to this package in the ADR-0112 ledger, and is what the structurally identical opening guard in `rollbackMetaItem` already uses.

packages/metadata-protocol/src/protocol.save-meta-missing-item.test.ts

@@ -0,0 +1,148 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #8818 — `saveMetaItem`'s opening guard was the ONE refusal in the method
+ * that declared no ADR-0112 envelope, so consumers applying the rule withheld
+ * its sentence and the REST boundary served it as a server fault.
+ *
+ * ## What was measured, end to end, before the fix
+ *
+ * The card was filed as an observation with an explicitly unverified premise
+ * ("read from source, no victim measured"), so the premise was probed against
+ * a real server (`pnpm dev:crm -- --fresh`, `PUT /api/v1/meta/view/:name`,
+ * authenticated as the seeded platform admin) before a line was changed:
+ *
+ * | request body | reaches this guard? | answered (before) |
+ * |---|---|---|
+ * | *(no body at all)* | no | `422 INVALID_METADATA` |
+ * | `null` | no | `422 INVALID_METADATA` |
+ * | `{}` | no | `422 INVALID_METADATA` |
+ * | `{"item": null}` | **YES** | **`500 INTERNAL_ERROR`** |
+ * | `{"metadata": null}` | **YES** | **`500 INTERNAL_ERROR`** |
+ *
+ * So the honest outcomes the card itself listed — "unreachable, leave it
+ * alone" and "a programming-error guard, not an authoring refusal" — are both
+ * FALSE, and the reason is precise: `PUT /meta/:type/:name` unwraps the
+ * `{ item }` / `{ metadata }` envelope shapes before calling, so an
+ * explicitly-null envelope arrives as `item: null` and lands here, while a
+ * missing/empty/`null` BODY folds to `{}` (truthy) and is refused downstream
+ * by the per-type Zod parse. The reachable population is caller-authored JSON
+ * only; every internal caller passes a concrete document.
+ *
+ * The cost was also LARGER than filed. The card predicted the sentence would
+ * degrade to a consumer's generic fallback; what the wire actually did was
+ * answer **500 `INTERNAL_ERROR`** — because `handleRouteError` has no status
+ * to read and defaults to a server fault. A 500 does not merely tell the
+ * author less, it tells them something FALSE: that the server broke and the
+ * request is worth retrying, when it can never succeed unchanged.
+ *
+ * ## Why the `clientFacingFailureText` assertion is the point of this file
+ *
+ * A test asserting only `code`/`status` on the thrown error would pass without
+ * demonstrating the thing the card is about. `declaresClientRefusal` is a
+ * POSITIVE list keyed on a 4xx `status`, so the assertion that matters is that
+ * the sentence now SURVIVES the rule rather than being replaced by the
+ * caller's fallback — and that assertion is only evidence next to its control:
+ * the same helper, handed the bare `Error` this guard used to throw, still
+ * withholds. Both directions are asserted below; drop the control and the pin
+ * would stay green against a `clientFacingFailureText` that had stopped
+ * withholding anything at all.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { ObjectStackProtocolImplementation, clientFacingFailureText } from './protocol.js';
+
+/**
+ * A protocol over an engine whose every verb is a tripwire: this guard is the
+ * FIRST statement of `saveMetaItem`, so a refusal that touched the engine at
+ * all would mean the check had moved behind something with side effects.
+ */
+function makeProtocol() {
+ const findOne = vi.fn(async () => null);
+ const find = vi.fn(async () => [] as unknown[]);
+ const engine = {
+ registry: { getObject: () => undefined },
+ findOne,
+ find,
+ };
+ return { p: new ObjectStackProtocolImplementation(engine as any), findOne, find };
+}
+
+/** The refusal a rejected request produced, plus proof the engine was untouched. */
+async function refusalFor(request: Record<string, unknown>) {
+ const { p, findOne, find } = makeProtocol();
+ let answered: unknown;
+ try {
+ answered = await (p as any).saveMetaItem(request);
+ } catch (e) {
+ expect(findOne, 'the engine was reached before the refusal').not.toHaveBeenCalled();
+ expect(find, 'the engine was reached before the refusal').not.toHaveBeenCalled();
+ return e as Error & { code?: string; status?: number };
+ }
+ throw new Error(
+ `${JSON.stringify(request)} was ACCEPTED (answered ${JSON.stringify(answered)}) instead of refused`,
+ );
+}
+
+describe('#8818 — a save with no item declares the ADR-0112 envelope', () => {
+ // The three spellings that reach this guard. `item: null` is the one the
+ // REST route actually produces (from `{"item": null}` / `{"metadata":
+ // null}`); the other two are the same condition reached through the SDK
+ // and the protocol interface, where `item` is an optional parameter.
+ it.each<[string, Record<string, unknown>]>([
+ ['an explicitly null item (what the wire produces)', { type: 'app', name: 'a', item: null }],
+ ['an absent item', { type: 'app', name: 'a' }],
+ ['an explicitly undefined item', { type: 'app', name: 'a', item: undefined }],
+ ])('refuses %s with 400 INVALID_REQUEST', async (_label, request) => {
+ const err = await refusalFor(request);
+
+ // The envelope, not merely the throw. A bare `toThrow()` here would be
+ // permanently green: the UNFIXED guard threw too — that was the whole
+ // defect — so the throw carries no information and only the
+ // declaration does.
+ expect(err.code).toBe('INVALID_REQUEST');
+ expect(err.status).toBe(400);
+ });
+
+ it('names the remedy in the message, so the refusal is self-correcting', async () => {
+ const err = await refusalFor({ type: 'view', name: 'my_view', item: null });
+
+ expect(err.message).toContain("requires an 'item' body");
+ // The address the author got wrong is echoed back to them.
+ expect(err.message).toContain('view/my_view');
+ });
+
+ it('does not refuse a request that DOES carry an item', async () => {
+ // What a refusal is cheapest to break. Green in both directions on its
+ // own — it is a guard, not evidence — but an over-broad guard (say, one
+ // testing `'item' in request`) would turn it red.
+ const { p } = makeProtocol();
+ await expect(
+ (p as any).saveMetaItem({ type: 'app', name: 'a', item: { name: 'a' } }),
+ ).rejects.not.toMatchObject({ code: 'INVALID_REQUEST' });
+ });
+});
+
+describe('#8818 — the refusal now SURVIVES the ADR-0112 disclosure rule', () => {
+ it('is quoted back to the author instead of degrading to the fallback', async () => {
+ const err = await refusalFor({ type: 'app', name: 'test_app', item: null });
+
+ const shown = clientFacingFailureText(err, 'save failed');
+
+ // THE POINT OF THE CARD: a consumer applying the #8086/#8136 rule now
+ // shows the producer's own sentence.
+ expect(shown).not.toBe('save failed');
+ expect(shown).toBe(err.message);
+ expect(shown).toContain("requires an 'item' body");
+ });
+
+ it('CONTROL — the bare Error this guard used to throw is still withheld', () => {
+ // Byte-for-byte what `origin/main` threw at this site. Without this
+ // control the assertion above would also pass against a
+ // `clientFacingFailureText` that had stopped withholding ANYTHING,
+ // which is the regression that would silently re-open #8086.
+ const undeclared = new Error('Item data is required');
+
+ expect(clientFacingFailureText(undeclared, 'save failed')).toBe('save failed');
+ });
+});

packages/metadata-protocol/src/protocol.ts

@@ -11458,8 +11458,47 @@ export class ObjectStackProtocolImplementation implements
  }
 
  async saveMetaItem(request: { type: string, name: string, item?: any, organizationId?: string, parentVersion?: string | null, actor?: string, force?: boolean, mode?: 'draft' | 'publish', packageId?: string | null, source?: string }) {
+ // [#8818] The ADR-0112 envelope this refusal always owed. Every OTHER
+ // refusal in this method declares `code` AND `status`
+ // (`NOT_OVERRIDABLE`/403, `NOT_CREATABLE`/403, `ITEM_LOCKED`/403,
+ // `OBJECT_OVERLAY_PACKAGE_MISMATCH`/422, the org-scope and
+ // destructive-change refusals, the parent-version conflict/409); this
+ // one declared neither, and an undeclared throw is withheld by
+ // {@link clientFacingFailureText} (a positive list keyed on 4xx
+ // `status`) and rendered `500 INTERNAL_ERROR` by `handleRouteError` —
+ // a SERVER FAULT for what is purely the caller's mistake, telling the
+ // author less than the producer knew and inviting a pointless retry.
+ //
+ // MEASURED reachable from the wire — this is an AUTHORING refusal, not
+ // a programming-error guard. `PUT /api/v1/meta/:type/:name` unwraps the
+ // `{ item }` / `{ metadata }` envelope shapes before calling here, so
+ // `{"item": null}` and `{"metadata": null}` arrive as `item: null` and
+ // land exactly here (measured end to end against a live server: both
+ // answered `500 INTERNAL_ERROR` before this change). ⚠️ A missing,
+ // empty or literal-`null` BODY does NOT reach this guard: the route
+ // folds it to `{}`, which is truthy, and the per-type Zod parse below
+ // refuses it with `422 INVALID_METADATA` — so this guard's whole
+ // reachable population is the explicitly-null envelope.
+ //
+ // `INVALID_REQUEST`/400 rather than the `INVALID_METADATA`/422 the
+ // sibling refusals use, and NEITHER mints a code: both are already
+ // registered to this package in the ADR-0112 ledger (D3). The 422
+ // sites all describe a body that EXISTS and failed the per-type parse,
+ // and every one carries structured `issues`; here there is no body to
+ // validate and no issues to report, so a 422 would misdescribe the
+ // failure and break that convention. The structural twin is
+ // {@link rollbackMetaItem}'s own opening guard — same class, same
+ // position, a malformed REQUEST ENVELOPE rather than an off-spec
+ // document — which is `[invalid_request]`/400.
  if (!request.item) {
- throw new Error('Item data is required');
+ const err: any = new Error(
+ `[invalid_request] saveMetaItem requires an 'item' body for '${request.type}/${request.name}'. `
+ + `Send the metadata document as the request body, or wrap it as {"item": {...}} / {"metadata": {...}}. `
+ + `An explicitly null item is refused rather than persisted as an empty document.`,
+ );
+ err.code = 'INVALID_REQUEST';
+ err.status = 400;
+ throw err;
  }
  // #4432 — CANONICAL TYPE KEY. See {@link canonicalMetaType}.
  request = canonicalizeMetaRequestType(request);

packages/objectql/src/protocol-meta.test.ts

@@ -225,10 +225,19 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => {
  });
 
  describe('saveMetaItem', () => {
- it('should throw when item data is missing', async () => {
- await expect(
- protocol.saveMetaItem({ type: 'app', name: 'test_app' })
- ).rejects.toThrow('Item data is required');
+ // [#8818] WAS `rejects.toThrow('Item data is required')` — a bare
+ // message match that stayed green while the refusal declared no
+ // ADR-0112 envelope at all, so `clientFacingFailureText` withheld the
+ // sentence and the REST boundary served `500 INTERNAL_ERROR`. The
+ // envelope is the contract; assert it.
+ it('refuses a missing item with the ADR-0112 envelope (400 INVALID_REQUEST)', async () => {
+ const err: any = await protocol.saveMetaItem({ type: 'app', name: 'test_app' })
+ .then(() => { throw new Error('saveMetaItem ACCEPTED a request with no item'); })
+ .catch((e: any) => e);
+
+ expect(err.code).toBe('INVALID_REQUEST');
+ expect(err.status).toBe(400);
+ expect(err.message).toContain("requires an 'item' body");
  });
 
  it('writes the saved body through to the SchemaRegistry for non-object types (#4521)', async () => {