From 9b2dc276d3f378b1aadf86b078cc1abd049934d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 18:24:45 +0000 Subject: [PATCH] fix(metadata-protocol): render the spec-validation 422 findings clause per write face (#10888) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `saveMetaItem`'s `422 INVALID_METADATA` restated its own findings in the message (`: ` for the first three, plus a `(+N more)` tail) while attaching the same array as `err.issues`. On the HTTP 422 both channels ride one response, so every console rendering both showed each finding twice. The clause is now rendered per face, reusing the `writeFace` channel #11015 established for the sibling 409. The `/meta` HTTP write doors — REST's `PUT /meta/:type/:name` and `PUT /meta/:type/:a/:b`, and the runtime dispatcher's `PUT /meta` — declare `writeFace: 'meta-envelope'` and get the #10524 headline: issue count plus up to three `path [zod code]` locators, the same grammar `seedRequestValidationError` and the author-time gate compose. `err.issues` is attached unchanged on every face. Silence keeps the full prose. The inventory re-derived for THIS gate finds four message-only faces, not one: `duplicatePackage`'s `failed[].error`, `migrateStoredMetadata`'s `rows[].reason`, and two out-of-package log faces that the 409's table eliminates by a constant and this gate cannot — `force` does not exempt a caller here, and `app` and `permission` both have registered schemas. Two of them live in other packages and could not declare a face without making the field caller-settable, so the destructive direction requires an explicit declaration and can never happen by omission. #10886's verdict is untouched: the duplicate face keeps the prescription in full, and its three #8333 GUARD pins stay green unmodified. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- .changeset/face-aware-invalid-metadata-422.md | 27 ++ ...col.destructive-409-face-inventory.test.ts | 32 +- ...nvalid-metadata-422-face-inventory.test.ts | 302 ++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 133 ++++++-- .../src/rest-4xx-message-truncation.test.ts | 28 +- packages/rest/src/rest-server.ts | 20 ++ packages/runtime/src/domains/meta.ts | 13 +- packages/runtime/src/http-dispatcher.test.ts | 13 +- scripts/engine-double-contract.pinned.json | 10 + 9 files changed, 548 insertions(+), 30 deletions(-) create mode 100644 .changeset/face-aware-invalid-metadata-422.md create mode 100644 packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts diff --git a/.changeset/face-aware-invalid-metadata-422.md b/.changeset/face-aware-invalid-metadata-422.md new file mode 100644 index 0000000000..faee09ed69 --- /dev/null +++ b/.changeset/face-aware-invalid-metadata-422.md @@ -0,0 +1,27 @@ +--- +"@objectstack/metadata-protocol": patch +"@objectstack/rest": patch +"@objectstack/runtime": patch +--- + +Render `saveMetaItem`'s `422 INVALID_METADATA` findings clause per write face + +The spec-validation refusal restated its own findings in the message +(`: ` for the first three, plus a `(+N more)` tail) while +attaching the same array as `issues`. On the HTTP 422 both channels ride one +response, so every console rendering both showed each finding twice. + +The clause is now rendered per face. The `/meta` HTTP write doors — REST's +`PUT /meta/:type/:name` and `PUT /meta/:type/:a/:b`, and the runtime +dispatcher's `PUT /meta` — declare that they carry the findings structurally +and get a one-sentence headline instead: the issue count plus up to three +`path [zod code]` locators, the same grammar the seed refusal and the +author-time gate already compose. `issues[]` is attached unchanged on every +face, so nothing is withheld from anyone. + +Faces that carry no structured channel keep the full prose, byte for byte — +`duplicatePackage`'s `failed[].error`, `migrateStoredMetadata`'s +`rows[].reason`, and the two out-of-package log faces, where this sentence is +the sole carrier of the author's prescription. Silence means "keep the prose": +a write door only ever drops the restatement by declaring itself, never by +omission. diff --git a/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts b/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts index 4871dc8fe3..dd8fd23fb5 100644 --- a/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts +++ b/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts @@ -207,7 +207,7 @@ function makeKernel(opts: { seed?: Row[] } = {}) { } /** The refusal this whole file is about, raised by the real producer. */ -async function destructiveRefusal(): Promise { +async function destructiveRefusal(writeFace?: string): Promise { const { protocol } = makeKernel({ seed: [objectRow('crm_task', ['a', 'b', 'c', 'd'])], }); @@ -216,6 +216,7 @@ async function destructiveRefusal(): Promise { type: 'object', name: 'crm_task', item: { name: 'crm_task', label: 'crm_task', fields: { a: { name: 'a', type: 'text' } } }, + ...(writeFace ? { writeFace } : {}), }); } catch (e: any) { return e; @@ -278,6 +279,35 @@ describe('[#10886] the 409 renders its findings into the message AND attaches th const wire = JSON.stringify(err.issues); expect(wire).not.toContain('force=true'); }); + + /** + * [#10888] The two switches that read `writeFace` answer DIFFERENT + * questions, and this pin holds them independent. + * + * That card made the sibling `422 INVALID_METADATA` findings clause + * face-aware too, and the `/meta` HTTP write doors now state + * `writeFace: 'meta-envelope'` on every `saveMetaItem` call — including + * the ones that land on THIS gate. `'meta-envelope'` is exactly the + * single-segment REST `PUT` among other doors, so `?force=true` is still + * its real remedy and this clause must not move. + * + * Without this pin the coupling is invisible: a later edit that folds the + * two switches together, or that gives `destructiveChangeRemedy` a case + * for the new face, would change a 409 remedy on the busiest write door + * in the product while every 422 test stayed green. + */ + it('[GUARD] a face declared for the 422 does NOT move the 409 remedy', async () => { + const err = await destructiveRefusal('meta-envelope'); + + expect(err.code).toBe('DESTRUCTIVE_CHANGE'); + expect(err.status).toBe(409); + expect(err.message).toContain(PUT_REMEDY); + expect(err.message).not.toContain(DUPLICATE_REMEDY_HEAD); + // …and the findings prose is still restated here, because this gate's + // sole-carrier verdict (#10886) is untouched by #10888: the 422's face + // split applies to the 422's clause only. + expect(err.message).toContain(FINDING_PROSE); + }); }); // ═══════════════════════════════════════════════════════════════════════════ diff --git a/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts b/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts new file mode 100644 index 0000000000..93e4845aaf --- /dev/null +++ b/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts @@ -0,0 +1,302 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #10888 — the face inventory for `saveMetaItem`'s spec-validation + * `422 INVALID_METADATA`, and the pins that hold its conclusion. + * + * ## The duplication that raised the card + * + * The refusal rendered its own findings into the message + * (`issues.slice(0, 3).map((i) => `${i.path}: ${i.message}`).join('; ')` plus a + * `(+N more)` tail) AND attached the same array as `err.issues`. On the HTTP + * 422 both channels ride one response, so every console rendering both showed + * each finding twice — the #10524 shape, on the save door. + * + * ## The conclusion: trim PER FACE, and let silence mean "keep the prose" + * + * A blanket trim was tried during #10524 and reverted: some faces put this + * sentence on a **200 response body** or in a **log**, where no structured + * channel exists and the sentence is the SOLE carrier of the author's + * prescription. #10886 reached the same verdict for the sibling 409. + * + * The maintainer ruling on #11017 (2026-08-22, option D) resolved it by + * reusing #11015/#11099's per-face rendering rather than by declaring a + * response contract for `duplicatePackage`: faces that already carry a + * structured `issues[]` drop the prose restatement; the duplicate face keeps + * it in full. + * + * ## The inventory, re-derived for THIS gate + * + * ⚠️ It is NOT the 409's inventory with a different verb name, and reusing that + * table would have been wrong in three rows. The 409 fires only when + * `!request.force` AND the folded type is `object`/`field` AND a row already + * exists AND the diff is non-empty. **This gate has none of those conditions**: + * it fires whenever `getMetadataTypeSchema()` resolves and the + * body fails `safeParse`. So `force: true` does not exempt a caller here, and + * neither does a literal `type` — `app` and `permission` both have registered + * schemas. + * + * | # | caller | type | reaches THIS gate | face | structured `issues[]` | + * |:--|:--|:--|:--|:--|:--| + * | 1 | `@objectstack/rest` `PUT /meta/:type/:name` | any | yes | `sendError` 422 body | **yes** — top-level `issues` | + * | 2 | `@objectstack/rest` `PUT /meta/:type/:a/:b` | any | yes | the same body | **yes** | + * | 3 | `@objectstack/runtime` dispatcher `PUT /meta` | any | yes | `errorFromThrown` → `details.issues` | **yes** | + * | 4 | `@objectstack/runtime` ADR-0045 visibility flip | `'app'` | **yes** — `app: AppSchema` | `unhideError` + log | ⛔ **no** | + * | 5 | `migrateStoredMetadata` | any | **yes** — `force` does not gate this check | `rows[].reason` on a report | ⛔ **no** | + * | 6 | `duplicatePackage` | `row.type` | yes | `failed[].error` on a **200** | ⛔ **no — sole carrier** | + * | 7 | `plugin-security` permission-set projection ×4 | `'permission'` | **yes** — `permission: PermissionSetSchema` | `logger.error` text | ⛔ **no** | + * + * Rows 4, 5 and 7 are the ones the 409's table eliminated and this one cannot. + * Row 7 matters most: its log sentence prescribes "make the record body + * spec-valid (**the error names the offending key**)" — a remedy written on the + * assumption that this clause names it. + * + * ## Why the polarity is "declare to trim", not "declare to keep" + * + * Rows 4 and 7 live in OTHER packages, reached through `(protocol as any)`. + * Neither can state a face without making `writeFace` a field an arbitrary + * caller sets. Under the opposite polarity — trim by default, message-only + * faces opt out — both would lose their prescription **silently**, and so would + * every write door added later by an author who never read this file. So + * silence renders the full prose, and only rows 1-3 declare + * `writeFace: 'meta-envelope'`. + * + * That the declaration is not client-settable is structural, not conventional: + * each door builds the `saveMetaItem` request object field by field from named + * `req` values and never spreads the body. + * + * ## Reverse verification — direction predicted BEFORE running + * + * Predicted: with `specValidationFindings`' `'meta-envelope'` case deleted (so + * every face falls to the prose branch), the `'meta-envelope'` pins below go + * RED and **nothing else in the package moves** — the three #8333 GUARD pins + * that hold the prescription (`protocol.batch-verb-driver-text.test.ts` P10 and + * the two in `protocol.save-union-issues.test.ts`) are asserting the DEFAULT + * face, which that ablation does not touch. Measured: exactly that. See the PR + * body for the run. + * + * Harness: the real repository write path over a stub engine — a change INSIDE + * `saveMetaItem` cannot use a harness that mocks `saveMetaItem`. The subject is + * imported as `./protocol.js`, a RELATIVE source specifier, so vitest resolves + * it to `src/protocol.ts` and no `dist/` is on the path; the ablation therefore + * needs no rebuild, and its RED result rules out a stale-artifact false green. + */ +import { describe, expect, it } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + state: string; + metadata: string; +} + +const keyOf = (w: Record) => + `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`; + +function makeProtocol() { + const rows = new Map(); + let nextId = 0; + const engine: any = { + async findOne() { return null; }, + async find() { return []; }, + async insert(table: string, data: Record) { + if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; + nextId += 1; + const row = { id: `r_${nextId}`, ...(data as any) } as Row; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(_t: string, data: Record, opts?: Record) { + assertEngineUpdateDispatch(data, opts); + return { id: null }; + }, + async delete(_t: string, opts?: Record) { + assertEngineDeleteDispatch(opts); + return { deleted: 0 }; + }, + registry: { registerItem: () => {}, registerObject: () => {} }, + }; + const protocol: any = new ObjectStackProtocolImplementation(engine, () => new Map()); + return { protocol, rows }; +} + +/** + * The issue's own repro body: a list view whose `summary` carries a typo'd key. + * `ViewMetadataSchema` is a top-level union, so this is also the root-level + * union case the card flagged as the headline's worst input. + */ +const typoView = () => ({ + name: 'task_list', + object: 'task', + type: 'list', + label: 'Tasks', + columns: [{ field: 'title', summary: { type: 'sum', fieldd: 'amount' } }], +}); + +async function refusal(protocol: any, writeFace?: string): Promise { + try { + await protocol.saveMetaItem({ + type: 'view', + name: 'task_list', + item: typoView(), + ...(writeFace ? { writeFace } : {}), + }); + } catch (e: any) { + return e; + } + throw new Error('expected saveMetaItem to refuse the invalid body'); +} + +/** The #4001 curated prescription the three #8333 GUARD pins hold. */ +const PRESCRIPTION = 'Unrecognized key(s) on this view container'; + +// ═══════════════════════════════════════════════════════════════════════════ +// 1. The structured channel is unconditional — the face decides only the prose +// ═══════════════════════════════════════════════════════════════════════════ + +describe('[#10888] `issues[]` is attached identically on every face', () => { + it('the declared face and the default face carry the SAME findings', async () => { + const { protocol } = makeProtocol(); + const plain = await refusal(protocol); + const envelope = await refusal(protocol, 'meta-envelope'); + + expect(plain.code).toBe('INVALID_METADATA'); + expect(envelope.code).toBe('INVALID_METADATA'); + expect(plain.status).toBe(422); + expect(envelope.status).toBe(422); + + // ⭐ The load-bearing non-effect: trimming the SENTENCE withholds + // nothing, because this array is what the sentence was restating. + expect(envelope.issues).toEqual(plain.issues); + expect(envelope.issues.length).toBeGreaterThan(1); + }); + + it('an invalid body is still refused, and still persists nothing', async () => { + const { protocol, rows } = makeProtocol(); + await refusal(protocol, 'meta-envelope'); + + // The accept set is unchanged by this card — it edits wording only. + expect(rows.size).toBe(0); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// 2. Rows 1-3 — the declared face drops the restatement +// ═══════════════════════════════════════════════════════════════════════════ + +describe('[#10888] `meta-envelope` renders the headline, not the prose', () => { + it('no issue message is restated in the sentence', async () => { + const { protocol } = makeProtocol(); + const err = await refusal(protocol, 'meta-envelope'); + + // The duplication, gone: not one finding appears twice on this face. + for (const i of err.issues) { + expect(err.message).not.toContain(i.message); + } + expect(err.message).not.toContain(PRESCRIPTION); + }); + + it('the headline still names HOW MANY and WHERE', async () => { + const { protocol } = makeProtocol(); + const err = await refusal(protocol, 'meta-envelope'); + + expect(err.message).toContain('[invalid_metadata] view/task_list failed spec validation: '); + expect(err.message).toContain(`${err.issues.length} issue`); + // The same grammar `seedRequestValidationError` composes — count plus + // `path [zod code]` locators — so one mistake reads the same whichever + // authoring door reported it. + expect(err.message).toMatch(/\d+ issues? — /); + expect(err.message).toContain('[invalid_union]'); + }); + + /** + * The card asked whether the headline degrades unacceptably on a + * root-level union failure, where every locator's path is empty and the + * key names live only in `issues[]`. MEASURED here rather than assumed, + * because the answer decides whether the headline grammar needs work. + * + * It renders ` [invalid_union]; [unrecognized_keys]` — the + * `path || ''` fallback predates this card and survives it, so the + * locator is never blank. Degraded but not lossy: this face is, by the + * definition of the face, one where `issues[]` rides along carrying the key + * names. Hoisting them into the locator is an improvement to the shared + * headline grammar (`metadataIssueHeadline`, which the seed refusal and the + * author-time gate also compose) and deliberately NOT done here — one + * message's wording is not the place to change three doors' grammar. + */ + it('a root-level union failure still locates itself as ``, never blank', async () => { + const { protocol } = makeProtocol(); + const err = await refusal(protocol, 'meta-envelope'); + + expect(err.message).toContain(' [invalid_union]'); + expect(err.message).not.toMatch(/: {2}\[/); + expect(err.message).not.toContain(' [invalid_union]; ['); + + // …and the key names the headline does not carry are on the channel + // that made the trim safe in the first place. + const unknownKey = err.issues.find((i: any) => i.code === 'unrecognized_keys'); + expect(unknownKey).toBeDefined(); + expect(unknownKey.message).toContain('`type`'); + expect(unknownKey.message).toContain('`columns`'); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// 3. Rows 4-7 — the message-only faces keep the prescription, in full +// ═══════════════════════════════════════════════════════════════════════════ + +describe('[#10888 · GUARD] a face that carries no `issues[]` keeps the whole sentence', () => { + /** + * ⛔ The pin that must never be made green by deleting what it protects. + * If a future trim widens to this branch, the author on rows 4-7 is told a + * body is invalid and never told which key — the exact regression #10524's + * trial-trim was reverted for. + */ + it('the DEFAULT face restates the findings, byte for byte', async () => { + const { protocol } = makeProtocol(); + const err = await refusal(protocol); + + const expected = err.issues.slice(0, 3) + .map((i: { path: string; message: string }) => `${i.path || ''}: ${i.message}`) + .join('; ') + + (err.issues.length > 3 ? ` (+${err.issues.length - 3} more)` : ''); + + expect(err.message).toBe( + `[invalid_metadata] view/task_list failed spec validation: ${expected}`, + ); + // The #4001 self-correcting prescription, whole. + expect(err.message).toContain(PRESCRIPTION); + }); + + it('row 6 — the duplicate face is explicitly on the keep side', async () => { + const { protocol } = makeProtocol(); + const duplicate = await refusal(protocol, 'package-duplicate'); + const plain = await refusal(protocol); + + // #10886's verdict, unchanged: `failed[].error` is the sole carrier. + // (The end-to-end pin through `duplicatePackage` itself is P10 in + // `protocol.batch-verb-driver-text.test.ts`, still green, untouched.) + expect(duplicate.message).toBe(plain.message); + expect(duplicate.message).toContain(PRESCRIPTION); + expect(duplicate.message).toContain('defineView('); + }); + + /** + * The polarity itself, pinned. An unrecognised face is not a configuration + * error to be reported — it is a door whose author did not declare a + * structured channel, and the only safe reading of that is "assume there + * isn't one". + */ + it('an UNKNOWN face falls to the prose branch, not to the headline', async () => { + const { protocol } = makeProtocol(); + const unknown = await refusal(protocol, 'some-future-door'); + const plain = await refusal(protocol); + + expect(unknown.message).toBe(plain.message); + expect(unknown.message).toContain(PRESCRIPTION); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index f30411eeb4..9b8f03c258 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -2095,6 +2095,77 @@ function metadataIssueHeadline(issues: MetadataIssueEntry[]): string { return `${issues.length} issue${issues.length === 1 ? '' : 's'} — ${locators}`; } +/** + * [#10888] The `422 INVALID_METADATA` findings clause, rendered PER FACE. + * + * The refusal is raised in one place ({@link + * ObjectStackProtocolImplementation.saveMetaItem}'s overlay spec check) and + * quoted onto whatever response the caller's catch builds, so one clause is + * read on every face at once — and the faces do not agree on whether the + * structured `issues[]` array reaches the consumer beside it. Where it does, + * restating the prose here is the #10524 duplication exactly: every console + * rendering both channels shows each finding twice. Where it does NOT, this + * clause is the SOLE carrier and trimming it deletes the author's + * prescription from the wire outright. + * + * ## The polarity is the load-bearing decision, not the wording + * + * Silence renders the FULL prose — byte-identical to what every face carried + * before this card. Only a face that positively declares it carries the + * structured channel gets the headline. That direction is deliberate and is + * the opposite of a convenience default: + * + * - the destructive direction (deleting prose) requires an explicit, + * reviewable declaration at a call site, so it can never happen by + * omission; + * - a new write door added later, or one whose author never read this + * comment, keeps the complete sentence. The failure mode of forgetting is + * a redundant sentence on a console, not an author who is told a body is + * invalid and never told which key. + * + * ⛔ Do not invert this to "trim by default, message-only faces opt out". + * Measured on `origin/main` for THIS gate (see + * `protocol.invalid-metadata-422-face-inventory.test.ts`): four of the seven + * callers are message-only, and two of them live in OTHER packages + * (`@objectstack/runtime`'s ADR-0045 visibility flip, `plugin-security`'s + * permission-set projection) where no face can be stated without making the + * field caller-settable. Under the inverted polarity those two lose their + * prescription silently — and `plugin-security`'s remedy sentence + * ("make the record body spec-valid (the error names the offending key)") + * is written on the assumption that this clause names it. + * + * ⚠️ Unlike the 409's {@link destructiveChangeRemedy}, whose faces differ on + * WHAT REMEDY EXISTS, the faces here differ only on WHETHER A SECOND CHANNEL + * CARRIES THE SAME FACTS. Nothing is withheld from anyone: `err.issues` is + * attached identically on every face, and the headline still names how many + * findings there are and where they are. + */ +function specValidationFindings( + face: MetadataWriteFace | undefined, + issues: MetadataIssueEntry[], +): string { + switch (face) { + case 'meta-envelope': + // The door answers with an ADR-0112 error envelope that carries + // `issues[]` beside the message (`@objectstack/rest`'s + // `sendError` threads a top-level `issues`; `@objectstack/runtime`'s + // dispatcher threads `details.issues`). The prose lives there, + // once. Same headline grammar the seed refusal and the author-time + // gate compose — count plus `path [zod code]` locators. + return metadataIssueHeadline(issues); + default: + // Byte-identical to the pre-#10888 clause: the first three findings + // as `: `, then a `(+N more)` tail. Read by + // `duplicatePackage`'s `failed[].error`, `migrateStoredMetadata`'s + // `rows[].reason`, and the two out-of-package log faces — none of + // which carry `issues[]`. + return issues.slice(0, 3) + .map((i: { path: string; message: string }) => `${i.path || ''}: ${i.message}`) + .join('; ') + + (issues.length > 3 ? ` (+${issues.length - 3} more)` : ''); + } +} + /** * A batch row that names no record id for an operation that needs one — a * caller error, so it carries VALIDATION_FAILED / 400 rather than falling @@ -3242,8 +3313,31 @@ function detectDestructiveObjectChanges(prev: any, next: any): Array<{ * guessed at. Adding a face value here is one of the two candidate repairs it * weighs. */ +/** + * [#11015 / #10888] Which write door a `saveMetaItem` refusal is being + * rendered FOR. Stated by the SERVER — either by the protocol's own internal + * call ({@link ObjectStackProtocolImplementation.duplicatePackage}) or by the + * in-process HTTP boundary that owns the response envelope — never by a remote + * caller: no write door spreads a request body into the `saveMetaItem` request + * object, so there is no path for a client to smuggle a face in (pinned in + * both face-inventory suites). + * + * Two refusals read this, and they read it for DIFFERENT questions: + * + * - {@link destructiveChangeRemedy} (409) — which remedy actually exists on + * this door; + * - {@link specValidationFindings} (422) — whether a structured `issues[]` + * channel reaches the consumer beside the message. + * + * A door therefore answers both questions by naming itself once, and neither + * switch may assume the other's default. `'meta-envelope'` deliberately keeps + * the 409's `?force=true` wording (it is the single-segment REST `PUT`'s + * genuine remedy) while changing the 422's clause. + */ +type MetadataWriteFace = 'package-duplicate' | 'meta-envelope'; + function destructiveChangeRemedy( - face: 'package-duplicate' | undefined, + face: MetadataWriteFace | undefined, name: string, ): string { switch (face) { @@ -12905,7 +12999,7 @@ 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, writeFace?: 'package-duplicate' }) { + 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, writeFace?: MetadataWriteFace }) { // [#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, @@ -13441,25 +13535,24 @@ export class ObjectStackProtocolImplementation implements const parsed = schema.safeParse(request.item); if (!parsed.success) { const issues = zodIssuesToMetadataIssues(parsed.error.issues); - // [#10524] Deliberately NOT trimmed to the headline the - // author-time gate and `seedRequestValidationError` now - // compose, although this is the same duplication shape on - // the 422 envelope face (message prose + `details.issues`). - // Measured during that card: this message is quoted on - // faces where it is the SOLE carrier — `duplicatePackage`'s - // `failed[].error` threads no `issues`, and three #8333 - // GUARD pins hold the author's prescription ("Unrecognized - // key(s) …", the `defineView(` spelling) to it. Trimming - // here without first declaring a structured channel on - // those faces deletes the prescription from the wire — - // the declare-then-trim order, violated. Filed as its own - // card; see the #10524 PR for the measurement. - const summary = issues.slice(0, 3) - .map((i: { path: string; message: string }) => `${i.path || ''}: ${i.message}`) - .join('; '); + // [#10524 → #10888] The findings clause is rendered PER + // FACE — see {@link specValidationFindings}. #10524's + // headline is applied on the faces that carry the same + // findings structurally beside the message (the `/meta` + // HTTP write doors, which declare `'meta-envelope'`), and + // the full prose is kept everywhere else, because on those + // faces this sentence is the SOLE carrier of the author's + // prescription — `duplicatePackage`'s `failed[].error` + // above all, whose three #8333 GUARD pins hold + // "Unrecognized key(s) …" and the `defineView(` spelling to + // it. A blanket trim here was tried during #10524 and + // reverted for exactly that reason; the face split is what + // lets the duplication go without the prescription going + // with it. `err.issues` below is unconditional — the split + // decides only what the SENTENCE repeats. const err = new Error( - `[invalid_metadata] ${request.type}/${request.name} failed spec validation: ${summary}` - + (issues.length > 3 ? ` (+${issues.length - 3} more)` : '') + `[invalid_metadata] ${request.type}/${request.name} failed spec validation: ` + + specValidationFindings(request.writeFace, issues) ); (err as any).code = 'INVALID_METADATA'; (err as any).status = 422; diff --git a/packages/rest/src/rest-4xx-message-truncation.test.ts b/packages/rest/src/rest-4xx-message-truncation.test.ts index c7279a1eb4..0e647957ef 100644 --- a/packages/rest/src/rest-4xx-message-truncation.test.ts +++ b/packages/rest/src/rest-4xx-message-truncation.test.ts @@ -244,16 +244,30 @@ async function callRoute(rest: any, method: string, path: string, req: Record: ` joined by `; `, behind an - * `[invalid_metadata] / failed spec validation: ` prefix, with a - * `(+N more)` suffix for the remainder. + * Built the way `metadata-protocol`'s `saveMetaItem` builds it ON A + * MESSAGE-ONLY FACE: the first THREE issues summarised as `: ` + * joined by `; `, behind an `[invalid_metadata] / failed spec + * validation: ` prefix, with a `(+N more)` suffix for the remainder. + * + * ⚠️ [#10888] That is no longer the form THIS route receives in production, and + * the distinction is worth stating because the docblock used to imply otherwise. + * `saveMetaItem` renders its findings clause per face: the `/meta` write doors + * declare `writeFace: 'meta-envelope'` and get a short headline (count plus + * `path [zod code]` locators), because `sendError` already carries the per-key + * prose structurally in `issues[]` beside the message. The long form above still + * exists, on the faces where the sentence is the SOLE carrier — + * `duplicatePackage`'s `failed[].error`, `migrateStoredMetadata`'s + * `rows[].reason` — but those are 200-response DATA and never reach `sendError`. + * + * So the error is CONSTRUCTED here rather than provoked, and that is the point: + * the subject under test is `sendError`'s bound, not the metadata door's + * wording. The bound must truncate any over-long 4xx that reaches it, whatever + * produced it, and it must keep the structured half intact while doing so. A + * realistic long 4xx is the right input for that even when the metadata face + * has stopped being one of its producers. * * Worth recording how close this family runs to the line: the same fixture with * three issues and no suffix measured 492 characters — under the bound by 8. - * A metadata save is not an exotic path and a five-issue rejection is not an - * exotic mistake, so this family straddles the cliff exactly as #5423 suspected - * the near-miss filter refusals (#5240 at ~469, #5327 at ~454) do. */ function invalidMetadataError() { const issues = [ diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 6383c77eaa..ca9c5079f4 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -5474,6 +5474,16 @@ export class RestServer { name: req.params.name, item, organizationId, + // [#10888] This door answers with an ADR-0112 error + // envelope that carries the refusal's `issues[]` + // structurally beside the message (`sendError` threads a + // top-level `issues`), so `saveMetaItem`'s 422 renders + // its findings as a headline here instead of restating + // the per-key prose a console would then show twice. + // Server-stated: this object is built field by field + // from named `req` values and never spreads the body, so + // a client cannot smuggle a face in. + writeFace: 'meta-envelope', ...(environmentId ? { environmentId } : {}), ...(parentVersion !== undefined ? { parentVersion } : {}), ...(actor ? { actor } : {}), @@ -6592,6 +6602,16 @@ export class RestServer { name: compoundName, item: req.body, organizationId, + // [#10888] This door answers with an ADR-0112 error + // envelope that carries the refusal's `issues[]` + // structurally beside the message (`sendError` threads a + // top-level `issues`), so `saveMetaItem`'s 422 renders + // its findings as a headline here instead of restating + // the per-key prose a console would then show twice. + // Server-stated: this object is built field by field + // from named `req` values and never spreads the body, so + // a client cannot smuggle a face in. + writeFace: 'meta-envelope', ...(environmentId ? { environmentId } : {}), ...(parentVersion !== undefined ? { parentVersion } : {}), ...(actor ? { actor } : {}), diff --git a/packages/runtime/src/domains/meta.ts b/packages/runtime/src/domains/meta.ts index bc61f30f69..3da595adca 100644 --- a/packages/runtime/src/domains/meta.ts +++ b/packages/runtime/src/domains/meta.ts @@ -414,7 +414,18 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin // the REST `/meta` write doors run the same one. const activeOrganizationId = await deps.resolveActiveOrganizationId(_context); const organizationId = organizationIdForMetaWrite(type, activeOrganizationId); - const result = await protocol.saveMetaItem({ type, name, item, organizationId, ...(packageId ? { packageId } : {}) }); + // [#10888] Server-stated face: this branch answers through + // `deps.errorFromThrown`, which carries the refusal's + // `issues[]` in `details` (see the `details.issues` pin in + // `http-dispatcher.test.ts`), so `saveMetaItem`'s 422 renders + // a headline rather than restating the per-key prose that + // already rides the envelope structurally. Not client-settable: + // the request object names each field explicitly. + const result = await protocol.saveMetaItem({ + type, name, item, organizationId, + writeFace: 'meta-envelope', + ...(packageId ? { packageId } : {}), + }); return { handled: true, response: deps.success(result) }; } catch (e: any) { // Preserve the 422 + structured spec-validation `issues` so diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index fb5ac075f6..6b84153739 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -136,7 +136,15 @@ describe('HttpDispatcher', () => { expect(mockProtocol.saveMetaItem).toHaveBeenCalledWith({ type: 'objects', name: 'my_obj', - item: body + item: body, + // [#10888] Server-stated, and asserted here rather than relaxed + // to `objectContaining`: this door's whole claim to the face is + // that it answers through `errorFromThrown`, which carries a + // refusal's `issues[]` in `details` (pinned below). If the face + // ever stops being stated, `saveMetaItem`'s 422 silently goes + // back to restating prose the envelope already carries — an + // exact-match assertion is what makes that visible. + writeFace: 'meta-envelope', }); expect(result.response?.body).toEqual({ success: true, @@ -159,6 +167,9 @@ describe('HttpDispatcher', () => { type: 'lead', name: 'views/all_leads', item: body, + // [#10888] The compound-name door states the same face — it is + // the same handler and the same envelope. + writeFace: 'meta-envelope', }); }); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 2cce7bbeb9..65b286a626 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -356,6 +356,16 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol.legacy-overlay-delete.test.ts", "verb": "delete",