From a77d87d5f55c1445c77e9de1743b5ccea1f337da Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 15:37:05 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): publish refusals state a headline; the prose rides the declared issues[] channel Declare-then-trim (#10524): PublishPackageDraftsResponseSchema's failed[] element declares the issues[] the producer has emitted since #8333, and seedApplied declares the seed refusal's issues[]; then the #4463 gate's 422 message and seedRequestValidationError's message shrink to one-sentence headlines (count + path [rule|code] locators) instead of restating the prose the structured channel carries. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- .../protocol-publish-drafts-closure.test.ts | 9 +- .../protocol.batch-verb-driver-code.test.ts | 8 +- .../protocol.batch-verb-driver-text.test.ts | 24 ++- packages/metadata-protocol/src/protocol.ts | 66 +++++++- .../src/runtime-authoring-gate.ts | 27 +++- ...ackage-drafts-response-conformance.test.ts | 131 ++++++++++++++++ ...ublish-drafts-response-conformance.test.ts | 67 +++++++- packages/runtime/src/domains/packages.ts | 13 ++ packages/spec/src/api/protocol.test.ts | 143 ++++++++++++++++++ packages/spec/src/api/protocol.zod.ts | 45 +++++- 10 files changed, 510 insertions(+), 23 deletions(-) diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts index 2136344c9d..ceebef10d7 100644 --- a/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts +++ b/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts @@ -380,7 +380,14 @@ describe('publishPackageDrafts judges each draft against the BATCH closure (#103 const causal = res.failed.find((f) => f.name === 'customer_dashboard')!; expect(causal.code).toBe('INVALID_METADATA'); expect(causal.error).toMatch(/widget-dataset-unknown/); - expect(causal.error).toMatch(/no_such_dataset_xyz/); + // [#10524] `error` is a headline now (path + rule locators); the + // dataset NAME lives in the finding's message, once, on the + // structured channel the batch response declares (`failed[].issues`). + const unknownDs = (causal as any).issues.find( + (i: any) => i.rule === 'widget-dataset-unknown', + ); + expect(unknownDs.message).toMatch(/no_such_dataset_xyz/); + expect(causal.error).not.toContain(unknownDs.message); // ADR-0067 D2 — all-or-nothing: the healthy sibling is aborted, not // published around the refusal. expect(res.failed.find((f) => f.name === 'shyx_customer_ds')?.code).toBe('BATCH_ABORTED'); diff --git a/packages/metadata-protocol/src/protocol.batch-verb-driver-code.test.ts b/packages/metadata-protocol/src/protocol.batch-verb-driver-code.test.ts index abc81cc7e0..3dca35b01c 100644 --- a/packages/metadata-protocol/src/protocol.batch-verb-driver-code.test.ts +++ b/packages/metadata-protocol/src/protocol.batch-verb-driver-code.test.ts @@ -441,9 +441,12 @@ describe('[#8441] [GUARD] the Studio publish surface still gets the code it bran // WHICH DRAFT. expect(failure.type).toBe('flow'); expect(failure.name).toBe('leave_approval'); - // WHICH FIELD — the located path, in the human sentence (#8333's half). + // WHICH FIELD — the located path, in the human sentence (#8333's + // half). [#10524] The sentence is a HEADLINE: path and rule id stay + // in it, and the message prose lives once, in `issues[]` below. expect(failure.error).toContain('flows[0].nodes[1].config.approvers[0].value'); - expect(failure.error).toContain('does not parse as CEL'); + expect(failure.error).toContain('[approval-expression-invalid]'); + expect(failure.error).not.toContain('does not parse as CEL'); // …and the machine-readable halves the form highlights with. `code` is // THIS card's field: catalogued, so it passes through byte for byte. expect(failure.code).toBe('INVALID_METADATA'); @@ -451,6 +454,7 @@ describe('[#8441] [GUARD] the Studio publish surface still gets the code it bran expect(Array.isArray(failure.issues)).toBe(true); expect(failure.issues[0].path).toBe('flows[0].nodes[1].config.approvers[0].value'); expect(failure.issues[0].rule).toBe('approval-expression-invalid'); + expect(failure.issues[0].message).toMatch(/does not parse as CEL/); }); }); diff --git a/packages/metadata-protocol/src/protocol.batch-verb-driver-text.test.ts b/packages/metadata-protocol/src/protocol.batch-verb-driver-text.test.ts index 2fce7a7ece..75f69cd8e5 100644 --- a/packages/metadata-protocol/src/protocol.batch-verb-driver-text.test.ts +++ b/packages/metadata-protocol/src/protocol.batch-verb-driver-text.test.ts @@ -594,9 +594,16 @@ describe('[#8333] [GUARD] a spec-validation failure on the publish path still na // WHICH DRAFT. expect(failure.type).toBe('flow'); expect(failure.name).toBe('leave_approval'); - // WHICH FIELD — the located path, in the human sentence. + // WHICH FIELD — the located path, in the human sentence. [#10524] + // The sentence is a HEADLINE now: it keeps the path and the rule id + // (this pin's guarded property — the withhold must not blank WHICH + // FIELD of WHICH DRAFT) while the message prose lives once, in + // `issues[]` below, instead of being restated here — every console + // rendering both channels was showing each finding twice. expect(failure.error).toContain('flows[0].nodes[1].config.approvers[0].value'); - expect(failure.error).toContain('does not parse as CEL'); + expect(failure.error).toContain('[approval-expression-invalid]'); + expect(failure.error).not.toContain('does not parse as CEL'); + expect(failure.issues[0].message).toMatch(/does not parse as CEL/); // …and the machine-readable halves the Studio form highlights with. expect(failure.code).toBe('INVALID_METADATA'); expect(Array.isArray(failure.issues)).toBe(true); @@ -632,6 +639,19 @@ describe('[#8333] the seed request’s schema rejection DECLARES itself, so the // multi-line stringified `ZodError`, which is why this is evidence. expect(r.error).toContain('seeds.0.mode'); expect(r.error).not.toContain('"code":'); + + // [#10524] The message is a HEADLINE (count + `path [zod code]` + // locators); the curated per-key prose rides the receipt ONCE, + // structurally, on `issues[]` — which is what lets the sentence stop + // restating it without the author losing anything. Only the declared + // 422 threads this key; the driver-fault cases below stay issue-less. + expect(Array.isArray(r.issues)).toBe(true); + const modeIssue = r.issues!.find((i: any) => i.path === 'seeds.0.mode'); + expect(modeIssue).toBeDefined(); + expect(typeof modeIssue!.message).toBe('string'); + for (const i of r.issues!) { + expect(r.error).not.toContain(i.message); + } }); it('the unreadable-bodies guard is untouched — a different fact, a different sentence', async () => { diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 2f1fce3df3..88495c3d19 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -2046,8 +2046,10 @@ function clientFacingFailureCode(err: unknown): string | undefined { * * The author is also strictly better off: the old path stringified a whole * `ZodError`, so `seedApplied.error` was a multi-line JSON dump of raw zod - * internals. This is the curated summary {@link zodIssuesToMetadataIssues} - * already produces for every other authoring surface. + * internals. The curated findings {@link zodIssuesToMetadataIssues} produces + * for every other authoring surface ride this error's `issues` — surfaced on + * `seedApplied.issues` by the catches (#10524) — and the message is their + * one-sentence headline. * * [#8443] EXPORTED alongside {@link clientFacingFailureText}: the runtime * package-publish door parses the SAME `SeedLoaderRequestSchema` in its own @@ -2059,12 +2061,14 @@ function clientFacingFailureCode(err: unknown): string | undefined { */ export function seedRequestValidationError(zodIssues: unknown): Error { const issues = zodIssuesToMetadataIssues(zodIssues); - const summary = issues.slice(0, 3) - .map((i: { path: string; message: string }) => `${i.path || ''}: ${i.message}`) - .join('; '); + // [#10524] `message` is a HEADLINE — count plus `path [zod code]` + // locators — never a restatement of the issue prose: the same `issues` + // array rides the error structurally, and the catches that surface this + // refusal thread it onto `seedApplied.issues` beside the headline, so + // the author's curated per-key prose still arrives exactly once. const err = new Error( - `[invalid_metadata] the published seed bodies failed spec validation: ${summary}` - + (issues.length > 3 ? ` (+${issues.length - 3} more)` : ''), + `[invalid_metadata] the published seed bodies failed spec validation: ` + + metadataIssueHeadline(issues), ); (err as any).code = 'INVALID_METADATA'; (err as any).status = 422; @@ -2072,6 +2076,25 @@ export function seedRequestValidationError(zodIssues: unknown): Error { return err; } +/** + * [#10524] The one-sentence headline for a refusal whose per-path detail + * rides `issues[]` structurally: total count plus up to three + * `path [zod code]` locators. Restating the issue MESSAGES here is exactly + * the duplication #10524 removed — every console rendering both channels + * showed each finding twice — so the message names WHERE and HOW MANY and + * leaves the prose to the structured channel. The leading count subsumes the + * old `(+N more)` tail. The author-time gate composes its own analogue with + * `[rule]` locators (`runtime-authoring-gate.ts`), deliberately: rule ids + * and zod codes are different vocabularies and folding them into one helper + * would blur which one a reader is looking at. + */ +function metadataIssueHeadline(issues: MetadataIssueEntry[]): string { + const locators = issues.slice(0, 3) + .map((i) => `${i.path || ''}${i.code ? ` [${i.code}]` : ''}`) + .join('; '); + return `${issues.length} issue${issues.length === 1 ? '' : 's'} — ${locators}`; +} + /** * 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 @@ -13304,6 +13327,19 @@ 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('; '); @@ -14838,7 +14874,10 @@ export class ObjectStackProtocolImplementation implements private async applySeedBodies( bodies: unknown[], organizationId: string | null, - ): Promise<{ success: boolean; inserted: number; updated: number; error?: string; errors?: unknown[] }> { + ): Promise<{ + success: boolean; inserted: number; updated: number; error?: string; errors?: unknown[]; + issues?: Array<{ path: string; message: string; code?: string | undefined }>; + }> { try { const seeds = bodies.filter( (b: any) => b && typeof b.object === 'string' && Array.isArray(b.records), @@ -14905,6 +14944,17 @@ export class ObjectStackProtocolImplementation implements return { success: false, inserted: 0, updated: 0, error: clientFacingFailureText(e, 'seed apply failed'), + // [#10524] A DECLARED refusal's structured findings ride the + // receipt beside the headline `error` — the message is a + // one-sentence headline now, so this is where the per-key + // prose reaches the author. Guarded by the same declaration + // test as the text above: only the declared 422 + // (`seedRequestValidationError`) attaches `issues`; no driver + // error carries them (the #8441 measurement), so nothing + // undeclared is routed around the withhold. + ...(declaresClientRefusal(e) && Array.isArray(e?.issues) + ? { issues: e.issues } + : {}), }; } } diff --git a/packages/metadata-protocol/src/runtime-authoring-gate.ts b/packages/metadata-protocol/src/runtime-authoring-gate.ts index bdecfe1fde..11d73354fc 100644 --- a/packages/metadata-protocol/src/runtime-authoring-gate.ts +++ b/packages/metadata-protocol/src/runtime-authoring-gate.ts @@ -635,11 +635,30 @@ export function evaluateRuntimeAuthoringGate(args: { if (result.errors.length === 0 && localIssues.length === 0) return { error: null, advisories }; const issues = [...result.errors.map(toIssue), ...localIssues]; - const summary = issues + // [#10524] Two renderings of one array, two audiences — deliberately NOT + // one string: + // + // - `detail` is the WHOLE refusal — path, rule and message prose — and + // goes only where no structured channel exists: the operator's + // un-deduped hatch warn below (#4463 acceptance). + // - the thrown 422's `message` is `headline`: what failed, where, which + // rules, how many. Every wire face the message lands on carries the + // SAME `issues` array structurally (`error.details.issues` on the + // single-item 422, `failed[].issues` on the batch response), so + // restating the issue prose in the message made every console render + // each finding twice — the summary-then-bullets duplication this trim + // removes. The leading count subsumes the old `(+N more)` tail; the + // prose lives once, in `issues[]`. + const locators = issues .slice(0, 3) - .map((i) => `${i.path || i.where || ''}: [${i.rule}] ${i.message}`) + .map((i) => `${i.path || i.where || ''} [${i.rule}]`) .join('; '); - const detail = summary + (issues.length > 3 ? ` (+${issues.length - 3} more)` : ''); + const headline = `${issues.length} issue${issues.length === 1 ? '' : 's'} — ${locators}`; + const detail = issues + .slice(0, 3) + .map((i) => `${i.path || i.where || ''}: [${i.rule}] ${i.message}`) + .join('; ') + + (issues.length > 3 ? ` (+${issues.length - 3} more)` : ''); // The registry's own disclosure, plus the gate-local rule when it was // applicable to this type. `rulesRun` exists so a caller can tell "clean" // from "nothing ran"; a judgement that can refuse a write and never appears @@ -668,7 +687,7 @@ export function evaluateRuntimeAuthoringGate(args: { } const err = new Error( - `[invalid_metadata] ${args.type}/${args.name} failed author-time validation: ${detail}`, + `[invalid_metadata] ${args.type}/${args.name} failed author-time validation: ${headline}`, ); (err as any).code = 'INVALID_METADATA'; (err as any).status = 422; diff --git a/packages/objectql/src/publish-package-drafts-response-conformance.test.ts b/packages/objectql/src/publish-package-drafts-response-conformance.test.ts index e5a1464ae7..752c796ee9 100644 --- a/packages/objectql/src/publish-package-drafts-response-conformance.test.ts +++ b/packages/objectql/src/publish-package-drafts-response-conformance.test.ts @@ -506,3 +506,134 @@ describe('outcome discriminates the three publish exits (#10462)', () => { }); }); }); + +/** + * #10524 — declare-then-trim on the batch refusal face. + * + * The measured defect: the causal `failed[]` entry carried BOTH an `error` + * string that inlined the first three findings' prose AND the same findings + * structurally under `issues` — undeclared, so every declared parse stripped + * the structured half while every console rendered the prose twice. Step 1 + * declared `issues` on the element (spec #10524); step 2 trimmed the + * #4463 gate's message to a headline. These cases pin both halves against + * the REAL protocol, and each names its control kind. + * + * The refusal is real, not synthesized: the same Zod-valid / gate-invalid + * broken-CEL approval flow `protocol.runtime-authoring-gate.test.ts` and + * `protocol.batch-verb-driver-text.test.ts` use, so the three files agree on + * what "a broken flow" is. + */ +const brokenApprovalFlow = () => ({ + name: 'leave_approval', + label: 'Leave Approval', + type: 'autolaunched', + status: 'active', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'approve', + type: 'approval', + label: 'Approve', + config: { approvers: [{ type: 'expression', value: 'record.owner ==' }] }, + }, + ], + edges: [{ id: 'e1', source: 'start', target: 'approve' }], +}); + +describe('failed[].issues — the causal refusal\'s structured findings (#10524)', () => { + async function publishBrokenFlow() { + const p = await makeProtocol(); + // Drafts are never gated (#4463 D1), so the broken body stages + // cleanly; the promote inside the batch publish is what refuses. + await stageDrafts(p, [{ type: 'flow', name: 'leave_approval', item: brokenApprovalFlow() }]); + return (await p.publishPackageDrafts({ packageId: PKG })) as any; + } + + /** + * CONTROL WHOSE SUBJECT DOES NOT EXIST PRE-FIX — the step-1 declaration. + * The producer has emitted `issues` on the causal element since #8333; + * what was missing is this case, which is why the key rode the wire + * undeclared. Falsified by ablating the `issues` declaration on the + * `failed[]` element (a plain z.object STRIPS what it does not declare, + * so the deep-equal below reds); the ablation run — both legs rebuilt, + * dist state proven — is reported in the PR. + */ + it('a causal failed[] entry carries issues[] and the declared parse strips none of it', async () => { + const raw = await publishBrokenFlow(); + + expect(raw.success).toBe(false); + expect(raw.outcome).toBe('refused'); + const causal = raw.failed.find((f: any) => f.code === 'INVALID_METADATA'); + expect(causal).toMatchObject({ type: 'flow', name: 'leave_approval' }); + const issue = causal.issues.find((i: any) => i.rule === 'approval-expression-invalid'); + expect(issue, `issues: ${JSON.stringify(causal.issues)}`).toBeDefined(); + expect(issue.path).toBe('flows[0].nodes[1].config.approvers[0].value'); + expect(issue.message).toMatch(/does not parse as CEL/); + + expect(strippedKeys(raw)).toEqual([]); + const parsed = PublishPackageDraftsResponseSchema.parse(raw); + const parsedCausal = parsed.failed.find((f) => f.code === 'INVALID_METADATA'); + expect(parsedCausal?.issues).toEqual(causal.issues); + // The full six-key element shape — declaring the card's `{path, + // message}` guess instead would strip `rule`/`hint` right here. + expect(Object.keys(parsedCausal!.issues![0]!).sort()) + .toEqual(['hint', 'message', 'path', 'rule', 'severity', 'where']); + }); + + /** + * DEFECT CONTROL — red on the pre-fix tree by construction: the gate's + * message interpolated `path: [rule] message` for the first three issues + * verbatim, so `error` contained every message this loop checks. This is + * the card's duplication, asserted directly; no ablation needed. + */ + it('error is a headline: it restates none of the prose issues[] carries', async () => { + const raw = await publishBrokenFlow(); + const causal = raw.failed.find((f: any) => f.code === 'INVALID_METADATA'); + expect(causal.issues.length).toBeGreaterThan(0); + for (const i of causal.issues) { + expect(causal.error).not.toContain(i.message); + } + }); + + /** + * PRESERVED-BEHAVIOUR CONTROL — the triage acceptance criterion: a + * consumer that renders ONLY `error` (CLI, logs) still learns what + * failed, where, under which rule, and how many findings there are. + * Green post-fix by construction, so its power to fail is proven by + * mutating the fix (dropping the locators or the count from the + * headline reds the matching assertion) — the mutation run is reported + * in the PR. The `(+N more)` tail's decided fate: replaced by the + * leading total count, which subsumes it. + */ + it('a consumer rendering only error still learns what failed, where, and how many', async () => { + const raw = await publishBrokenFlow(); + const causal = raw.failed.find((f: any) => f.code === 'INVALID_METADATA'); + expect(causal.error).toContain('flow/leave_approval'); + expect(causal.error).toContain('failed author-time validation'); + // The located path stays in the sentence — the #8333 GUARD property, + // preserved through the trim… + expect(causal.error).toContain('flows[0].nodes[1].config.approvers[0].value'); + // …with the stable rule id beside it, and the total up front. + expect(causal.error).toContain('[approval-expression-invalid]'); + expect(causal.error).toMatch(/: 1 issue — /); + }); + + /** + * NAMED NON-EFFECT — BATCH_ABORTED siblings never carry `issues` (the + * refusal was not theirs), and the refusal face still parses unstripped + * beside them. + */ + it('BATCH_ABORTED siblings carry no issues, and the face still conforms', async () => { + const p = await makeProtocol(); + await stageDrafts(p, [ + { type: 'flow', name: 'leave_approval', item: brokenApprovalFlow() }, + { type: 'view', name: 'cases', item: viewBody('cases', 'Cases') }, + ]); + const raw: any = await p.publishPackageDrafts({ packageId: PKG }); + + const sibling = raw.failed.find((f: any) => f.code === 'BATCH_ABORTED'); + expect(sibling).toBeDefined(); + expect('issues' in sibling).toBe(false); + expect(strippedKeys(raw)).toEqual([]); + }); +}); diff --git a/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts b/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts index f621c335a7..284aff7e16 100644 --- a/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts +++ b/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts @@ -64,6 +64,13 @@ function makeDoor(opts: { failSaveMetaItem?: boolean; failTrigger?: boolean; apps?: Array>; + /** + * [#10524] Provide `getMetaItem` so the route-level seed back-fill gets + * past its services guard and actually reads seed bodies — the leg the + * seed-refusal case below needs. Absent by default, which the no-counter + * case relies on. + */ + getMetaItem?: (args: unknown) => Promise; } = {}) { const result = opts.result ?? { // `outcome` (#10462) is REQUIRED on every producer return — a double @@ -90,11 +97,15 @@ function makeDoor(opts: { const kernel: any = { getService: (name: string) => { if (name === 'protocol') { - // Deliberately NO `getMetaItem`: the route-level seed - // back-fill demands it and reports "required services + // Deliberately NO `getMetaItem` by default: the route-level + // seed back-fill demands it and reports "required services // unavailable" without counters — the measured union arm that - // makes the schema's counters optional. - return Promise.resolve({ publishPackageDrafts, getMetaItems, saveMetaItem }); + // makes the schema's counters optional. The seed-refusal case + // (#10524) opts one in to get past that guard. + return Promise.resolve({ + publishPackageDrafts, getMetaItems, saveMetaItem, + ...(opts.getMetaItem ? { getMetaItem: opts.getMetaItem } : {}), + }); } if (name === 'objectql') { return Promise.resolve({ @@ -193,6 +204,54 @@ describe('publish-drafts wire payload conforms to PublishPackageDraftsResponseSc expect(parsed.seedApplied?.updated).toBeUndefined(); }); + /** + * [#10524] The seed bodies' own schema refusal, through the REAL route + * catch: `applyPublishedSeeds` reads the body back, `SeedLoaderRequestSchema` + * refuses it, `seedRequestValidationError`'s declared 422 is thrown, and + * the route catch surfaces it as `seedApplied` — headline on `error`, the + * per-key prose ONCE, structurally, on the now-declared `issues[]`. This + * is the route-level half of the declare-then-trim pair; the in-batch + * producer's half is pinned in `protocol.batch-verb-driver-text.test.ts` + * (§4) against the real `applySeedBodies`. + */ + it('a seed-body schema refusal serves headline + structured issues on seedApplied, and it conforms (#10524)', async () => { + spyLogs(); + const { data } = await publishDrafts({ + result: { + success: true, outcome: 'published', publishedCount: 1, failedCount: 0, + published: [{ type: 'seed', name: 'bad_rows', version: 'sha256:cc33' }], failed: [], + // NO seedApplied — the route-level fallback applies the seed. + }, + apps: [], + // Passes the body-shape filter (`object` string, `records` array) + // so the loader-request parse is what refuses — same fixture family + // as `protocol.batch-verb-driver-text.test.ts` §4. + getMetaItem: async () => ({ + item: { object: 'acct', records: [{ x: 1 }], mode: 'not-a-real-mode' }, + }), + }); + + expect(strippedKeys(data)).toEqual([]); + const parsed = PublishPackageDraftsResponseSchema.parse(data); + expect(parsed.seedApplied?.success).toBe(false); + // The headline names the refusal class and the offending key… + expect(parsed.seedApplied?.error).toContain('[invalid_metadata]'); + expect(parsed.seedApplied?.error).toContain('failed spec validation'); + expect(parsed.seedApplied?.error).toContain('seeds.0.mode'); + // …and the prose lives ONCE, on the declared structured channel. + const issues = parsed.seedApplied?.issues; + expect(Array.isArray(issues)).toBe(true); + const modeIssue = issues!.find((i) => i.path === 'seeds.0.mode'); + expect(modeIssue).toBeDefined(); + expect(typeof modeIssue!.message).toBe('string'); + for (const i of issues!) { + expect(parsed.seedApplied?.error).not.toContain(i.message); + } + // Unstripped through the declared parse — the ablation of this + // declaration (reported in the PR) reds exactly this deep-equal. + expect(parsed.seedApplied?.issues).toEqual((data as any).seedApplied.issues); + }); + it('byte-stability across the route: nothing on the serving path parses this schema, so declaring it moved no bytes', async () => { const { data } = await publishDrafts({ apps: [] }); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 6838063292..a14fd85d59 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -426,6 +426,19 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin (result as any).seedApplied = { success: false, error: clientFacingFailureText(e, 'seed apply failed'), + // [#10524] The declared 422's structured + // findings ride beside the headline `error` — + // `seedRequestValidationError`'s message is a + // one-sentence headline now, and `issues` is + // where the per-key prose reaches the author. + // `issues` is structured authoring feedback + // only the declared refusal attaches; no + // driver error carries it (the #8441 + // measurement), so this routes nothing + // undeclared around the withhold above. + ...(Array.isArray((e as any)?.issues) + ? { issues: (e as any).issues } + : {}), }; } } diff --git a/packages/spec/src/api/protocol.test.ts b/packages/spec/src/api/protocol.test.ts index 98ab962b55..1b03156d32 100644 --- a/packages/spec/src/api/protocol.test.ts +++ b/packages/spec/src/api/protocol.test.ts @@ -1301,6 +1301,149 @@ describe('PublishPackageDraftsResponseSchema published[].advisories (#9343 — # }); }); +/** + * #10524 — `failed[].issues`, the structured findings behind a causal refusal, + * declared at last. The producer has emitted this key since #8333 + * (`...(Array.isArray(e?.issues) ? { issues: e.issues } : {})` on the causal + * element), but the element declared `type`/`name`/`error`/`code` only — so + * the key rode the wire undeclared, was stripped by every declared parse, and + * was invisible to the SDK type. Declaring it is step 1 of the card's + * declare-then-trim: only once the structured channel is declared may `error` + * shrink to a headline without the per-path detail becoming unreadable to + * typed consumers. + */ +describe('PublishPackageDraftsResponseSchema failed[].issues (#10524 — the causal refusal\'s findings)', () => { + const base = { + success: false, + outcome: 'refused', + publishedCount: 0, + failedCount: 1, + published: [], + }; + + /** A verbatim-shaped capture of the #4463 gate's refusal finding. */ + const gateIssue = { + rule: 'approval-expression-invalid', + path: 'flows[0].nodes[1].config.approvers[0].value', + where: 'flow "leave_approval" · node "approve"', + message: '\'record.owner ==\' does not parse as CEL.', + hint: 'Complete the comparison, e.g. `record.owner == current.id`.', + severity: 'error' as const, + }; + + const causal = (extra?: Record) => ({ + type: 'flow', name: 'leave_approval', + error: '[invalid_metadata] flow/leave_approval failed author-time validation: ' + + '1 issue — flows[0].nodes[1].config.approvers[0].value [approval-expression-invalid]', + code: 'INVALID_METADATA', + ...(extra ?? {}), + }); + + it('carries a causal refusal\'s issues[] through parse, unstripped', () => { + const parsed = PublishPackageDraftsResponseSchema.parse({ + ...base, + failed: [causal({ issues: [gateIssue] })], + }); + expect(parsed.failed[0]!.issues).toEqual([gateIssue]); + }); + + it('is OPTIONAL per element — BATCH_ABORTED siblings and finding-less refusals omit it, and parse fabricates nothing', () => { + const parsed = PublishPackageDraftsResponseSchema.parse({ ...base, failed: [causal()] }); + expect(parsed.failed[0]!.issues).toBeUndefined(); + expect('issues' in parsed.failed[0]!).toBe(false); + }); + + it('element shape is the ONE declared finding shape — a lossy element is refused, not narrowed', () => { + // `RuntimeAuthoringIssueSchema` by reference, not a local re-declaration — + // the 2xx advisories[] channels and the refusal channels stay one dialect + // (#4717). The card described the shape as `{ path, message }`; the real + // emitted shape is the six-key issue, and declaring the narrower guess + // would strip `rule`/`hint` at the parse door — the same silent-loss + // family this key's declaration closes. + const partial = { path: 'flows[0]', message: 'broken' }; + expect( + PublishPackageDraftsResponseSchema.safeParse({ + ...base, + failed: [causal({ issues: [partial] })], + }).success, + ).toBe(false); + }); + + it('rejects a non-array, so a single issue object cannot masquerade as the list', () => { + expect( + PublishPackageDraftsResponseSchema.safeParse({ + ...base, + failed: [causal({ issues: gateIssue })], + }).success, + ).toBe(false); + }); +}); + +/** + * #10524 — `seedApplied.issues`, the seed-body schema refusal's structured + * findings. `seedRequestValidationError` (#8443) has attached `issues` to its + * declared 422 all along, but both catches that surface that refusal onto + * `seedApplied` kept only the message — so trimming the message to a headline + * without this channel would have deleted the author's per-key detail from + * the wire entirely. Element shape is the `zodIssuesToMetadataIssues` entry + * (`path` / `message` / zod's own `code`), NOT `RuntimeAuthoringIssueSchema`: + * these findings are minted by a zod parse, not by the authoring-rule + * registry, and declaring the six-key shape here would refuse every real + * emission. + */ +describe('PublishPackageDraftsResponseSchema seedApplied.issues (#10524 — the seed refusal\'s findings)', () => { + const base = { + success: true, + outcome: 'published', + publishedCount: 1, + failedCount: 0, + published: [ + { type: 'seed', name: 'demo_rows', version: 'sha256:7aad99c8d969efb5067fff275fb3e5be7ec90f9cd610d41709fcddbf8c34b1f0' }, + ], + failed: [], + }; + + /** A verbatim-shaped capture of the loader-request parse refusal. */ + const seedIssue = { + path: 'seeds.0.mode', + message: 'Invalid enum value. Expected \'insert\' | \'upsert\' | \'ignore\', received \'not-a-real-mode\'', + code: 'invalid_enum_value', + }; + + const refusedApply = (issues?: unknown) => ({ + ...base, + seedApplied: { + success: false, + error: '[invalid_metadata] the published seed bodies failed spec validation: ' + + '1 issue — seeds.0.mode [invalid_enum_value]', + ...(issues !== undefined ? { issues } : {}), + }, + }); + + it('carries the refusal\'s issues[] through parse, unstripped', () => { + const parsed = PublishPackageDraftsResponseSchema.parse(refusedApply([seedIssue])); + expect(parsed.seedApplied!.issues).toEqual([seedIssue]); + }); + + it('zod\'s `code` is optional per entry — an entry without one still parses', () => { + const { code: _dropped, ...noCode } = seedIssue; + const parsed = PublishPackageDraftsResponseSchema.parse(refusedApply([noCode])); + expect(parsed.seedApplied!.issues).toEqual([noCode]); + }); + + it('is OPTIONAL — non-validation failures (driver faults, unreadable bodies) omit it, and parse fabricates nothing', () => { + const parsed = PublishPackageDraftsResponseSchema.parse(refusedApply()); + expect(parsed.seedApplied!.issues).toBeUndefined(); + expect('issues' in parsed.seedApplied!).toBe(false); + }); + + it('rejects a non-array, so a single issue object cannot masquerade as the list', () => { + expect( + PublishPackageDraftsResponseSchema.safeParse(refusedApply(seedIssue)).success, + ).toBe(false); + }); +}); + // Deliberately its own import line, matching the file's later blocks: this // group pins one change (#9726) and reads as one unit. import { diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index 4f3d9022ef..7557ab413a 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -885,13 +885,31 @@ export const PublishPackageDraftsResponseSchema = lazySchema(() => z.object({ name: z.string().describe('Item name.'), error: z.string().describe( 'What refused it. On a rollback, the causal item carries its real ' - + 'error and every sibling carries the all-or-nothing explanation.', + + 'error and every sibling carries the all-or-nothing explanation. ' + + 'A refusal that produced structured findings states a one-sentence ' + + 'HEADLINE here (what failed, where, which rules, how many); the ' + + 'per-path detail rides `issues[]` instead of being restated in this ' + + 'string (#10524 — consumers rendering both channels were showing ' + + 'every finding twice).', ), code: z.string().optional().describe( 'Machine code for the refusal class (SCREAMING_SNAKE, ADR-0112 ' + 'vocabulary) — e.g. a pre-flight violation code, or BATCH_ABORTED ' + 'on the non-causal items of a rolled-back batch.', ), + issues: z.array(RuntimeAuthoringIssueSchema).optional().describe( + 'The structured findings behind the refusal, when the refusing error ' + + 'carried them — today the #4463 author-time gate\'s ' + + 'INVALID_METADATA refusal on the causal item. The producer has ' + + 'emitted this key since #8333; declaring it (#10524) is what lets a ' + + 'typed consumer read it back, and what lets `error` stay a headline ' + + 'without losing the per-path detail. Same element shape as ' + + '`published[].advisories` and the single-item 422\'s `issues[]` ' + + '(#4717 — one dialect, declared once). Present ONLY on the causal ' + + 'item and only when the refusal produced structured findings; ' + + 'BATCH_ABORTED siblings never carry it. Absent means "this refusal ' + + 'carried no structured findings", never "no problems".', + ), })).describe( 'Items that did not publish. Because the batch is all-or-nothing ' + '(ADR-0067 D2), a non-empty list means NOTHING landed: ' @@ -914,12 +932,35 @@ export const PublishPackageDraftsResponseSchema = lazySchema(() => z.object({ ), error: z.string().optional().describe( 'Single failure message, present when the apply failed before the ' - + 'loader ran (including "no readable seed bodies").', + + 'loader ran (including "no readable seed bodies"). When the failure ' + + 'is the seed bodies\' own schema refusal, this is a one-sentence ' + + 'headline and the per-path detail rides `issues[]` (#10524).', ), errors: z.array(z.unknown()).optional().describe( 'Per-record failures reported by the seed loader, plus any seed-body ' + 'read failures. May be present and empty on a clean load.', ), + issues: z.array(z.object({ + path: z.string().describe( + 'Dotted config path inside the submitted seed set (`seeds.0.mode`), ' + + 'so an author can jump to the offending key. May be empty for a ' + + 'whole-request finding.', + ), + message: z.string().describe('What is wrong, in the schema\'s own words.'), + code: z.string().optional().describe( + 'Zod\'s own issue code, verbatim — deliberately NOT the ADR-0114 ' + + '`fields[]` vocabulary (the #5364 decision: this is a ' + + 'metadata-authoring diagnostic and its consumers read raw zod ' + + 'codes; aligning the vocabularies is a separate decision).', + ), + })).optional().describe( + 'Structured spec-validation findings behind `error`, present when the ' + + 'apply was refused by the seed bodies\' own schema — the declared ' + + '422 `seedRequestValidationError` mints (#8443). The per-path ' + + 'detail lives HERE, once; `error` stays a one-sentence headline ' + + '(#10524). Absent on non-validation failures (driver faults, ' + + 'unreadable bodies), whose whole story is `error` / `errors[]`.', + ), }).optional().describe( 'Aggregate outcome of materializing EVERY published `seed` body in one ' + 'multi-pass loader run (cross-seed references need the whole set). ' From 425890b451114bb70ba3b066f00bd3c741a82b11 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 15:54:08 +0000 Subject: [PATCH 2/2] chore: changeset + regenerated spec artifacts for #10524 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- .changeset/publish-error-headline-issues.md | 28 +++++++++++++++++++ content/docs/references/api/protocol.mdx | 2 +- ...07-unknown-key-strictness-ledger.counts.md | 2 +- 3 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 .changeset/publish-error-headline-issues.md diff --git a/.changeset/publish-error-headline-issues.md b/.changeset/publish-error-headline-issues.md new file mode 100644 index 0000000000..ceb7e0afca --- /dev/null +++ b/.changeset/publish-error-headline-issues.md @@ -0,0 +1,28 @@ +--- +'@objectstack/spec': minor +'@objectstack/metadata-protocol': patch +'@objectstack/runtime': patch +--- + +Publish refusals no longer render each validation finding twice (#10524) — declare-then-trim. + +**Declared (spec, additive):** `PublishPackageDraftsResponseSchema.failed[]` elements now +declare `issues[]` (the `RuntimeAuthoringIssueSchema` findings the producer has emitted +since #8333 but no declared parse could carry), and `seedApplied` declares `issues[]` +(`{ path, message, code? }`, the seed-body schema refusal's findings). Typed consumers — +the SDK's `PublishPackageDraftsResponse`, any `parse` through the schema — can now read +the structured findings back instead of having them silently stripped. + +**Trimmed (producers):** the #4463 author-time gate's 422 message and +`seedRequestValidationError`'s message are one-sentence headlines — total count plus up to +three `path [rule]` / `path [zod-code]` locators — instead of restating the issue prose +that `issues[]` carries on the same response. Consumers that render only `error` (CLI, +logs) keep what failed, where, under which rule, and how many; consumers that render both +channels stop repeating themselves. The old `(+N more)` tail is subsumed by the leading +count. Both catches that surface the seed refusal onto `seedApplied` now thread the +structured findings beside the headline. + +Error `code`/`status` vocabularies, `advisories`, the DESTRUCTIVE_CHANGE (409) message, +and `saveMetaItem`'s spec-validation 422 message are unchanged. Messages are not contract +(the machine-readable channels are `code` and `issues[]`), so this is not a breaking +change and registers no migration. diff --git a/content/docs/references/api/protocol.mdx b/content/docs/references/api/protocol.mdx index 7279a72733..3a11679176 100644 --- a/content/docs/references/api/protocol.mdx +++ b/content/docs/references/api/protocol.mdx @@ -1248,7 +1248,7 @@ List packages response | **publishedCount** | `integer` | ✅ | Number of drafts promoted to active — `published.length`. 0 on every refusal path (the batch is all-or-nothing, ADR-0067 D2). | | **failedCount** | `integer` | ✅ | Number of items that did not publish — `failed.length`. On a rollback this counts the WHOLE batch: the causal item plus every sibling marked BATCH_ABORTED. | | **published** | `{ type: string; name: string; version: string; advisories?: object[] }[]` | ✅ | Every draft promoted to active, in publish order. Empty on every refusal path. | -| **failed** | `{ type: string; name: string; error: string; code?: string }[]` | ✅ | Items that did not publish. Because the batch is all-or-nothing (ADR-0067 D2), a non-empty list means NOTHING landed: `published: []`, `publishedCount: 0`. | +| **failed** | `{ type: string; name: string; error: string; code?: string; … }[]` | ✅ | Items that did not publish. Because the batch is all-or-nothing (ADR-0067 D2), a non-empty list means NOTHING landed: `published: []`, `publishedCount: 0`. | | **seedApplied** | `{ success: boolean; inserted?: integer; updated?: integer; error?: string; … }` | optional | Aggregate outcome of materializing EVERY published `seed` body in one multi-pass loader run (cross-seed references need the whole set). Present ONLY when the batch published at least one seed. Two producers, one key: the batch itself self-applies (`applySeedBodies`), and the REST door back-fills the same key for custom protocols that do not — never both (an externalId-less seed would double-insert). Best-effort: a seed problem is surfaced here, never thrown. | | **materializeApplied** | `{ success: boolean; inserted: integer; updated: integer; failures: object[] }` | optional | ADR-0086 P2 — aggregate result of publish-time materializers across the batch (e.g. `permission` → `sys_permission_set`), including side-effect failures surfaced by the per-item effects loop. Present ONLY when at least one published item had a registered materializer or a side-effect failure. Best-effort, same contract as `seedApplied`. | | **probes** | `any` | optional | ADR-0038 L3 post-publish runtime probe report — one real read per published artifact (seeded objects have rows, views are readable, widget dataset selections execute). DELIBERATELY OPAQUE in this contract (#9406): the key is declared and carried through verbatim, but its inner shape is intentionally not modeled until a consumer needs a field of it. Present only when something was publishable; probes never fail the publish. | diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md b/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md index fa26c70714..a7a538038d 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md @@ -257,7 +257,7 @@ directory rather than per file. | Dir | Sites | |---|---| | `ai/` | 77 | -| `api/` | 406 | +| `api/` | 407 | | `cloud/` | 83 | | `identity/` | 32 | | `integration/` | 10 |