diff --git a/.changeset/metadata-422-container-issue-descent.md b/.changeset/metadata-422-container-issue-descent.md new file mode 100644 index 0000000000..1bb76d300c --- /dev/null +++ b/.changeset/metadata-422-container-issue-descent.md @@ -0,0 +1,60 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata): the `422 INVALID_METADATA` envelope descends `invalid_key` / `invalid_element`, so a rejected record key arrives with the rule it broke (#8783) + +Zod raises a `z.record` / `z.map` **key** rejection as `invalid_key` and a +`z.map` **element** rejection as `invalid_element`, and in both cases the +issue's own `message` is a bare wrapper — `"Invalid key in record"` — with the +real diagnosis one level down in `issue.issues`. That is structurally the +`invalid_union` shape #4971 named: the prescription is produced and then +dropped by a walk that reads only the top level. + +Both `packages/spec` walks learned to descend those codes in #5389. +`zodIssuesToMetadataIssues` — the walk behind `saveMetaItem`'s 422 (#5364) and +the read path's diagnostics (#5598) — expanded `invalid_union` only, so it +stopped at the wrapper. Three walks over one `safeParse`, two of them reaching +the prescription and the Studio-facing one not. + +**It was reachable from ordinary authored metadata, not synthetic.** +`ObjectSchema.fields` is a record whose KEY schema carries the snake_case rule +(`spec/src/data/object.zod.ts`), and `object` is in the builtin +`getMetadataTypeSchema` registry. So the commonest authoring mistake on the +most-authored metadata type — writing `firstName` for a field key, which is +exactly what an agent coming from JS naming writes — produced: + +``` +{ path: 'fields.firstName', code: 'invalid_key', message: 'Invalid key in record' } +``` + +The author was told a key was invalid and never told what a valid one looks +like, so the next move was to guess. The declared message existed and was +correct; it just did not reach anyone. Now the same save answers: + +``` +{ path: 'fields.firstName', code: 'invalid_key', message: 'Invalid key in record' } +{ path: 'fields.firstName', code: 'invalid_format', message: 'Field names must be lowercase snake_case (e.g., "first_name", …)' } +``` + +**Additive, and matched to the walks that already worked** rather than chosen. +The other two were measured over the card's own repro first: `formatZodIssue` +prints the wrapper line then the indented detail, and `zodIssuesToFields` emits +the `invalid_shape` wrapper entry then the detail entry. So the wrapper stays at +index 0 — it is the only entry naming the slot the client sent, and Studio's +designer keys on it — and the detail joins it on the same path. No entry that +shipped before is removed or renumbered. + +**Targeted, not a widened walk.** Only the two container codes open the descent; +an `issues` array hanging off any other code is still ignored, `invalid_union` +still expands through the unchanged ranking, and the nesting bound now covers +both descents at the same depth of 3. Container issues are deliberately *not* +ranked the way union branches are: a union's branches are competing candidates, +while a container has one inner schema, so every issue it raised is a true +statement about the value. + +The verdict is unchanged in every case — this moves what a refusal *says*, never +whether it is one. `union-branch-policy.cross-package-parity.test.ts` gains a §5 +comparing the container descent across all three walks; its §1 (the policy is +not publicly exported from `@objectstack/spec`, so this package must run its own +copy) is untouched, and no export was added. diff --git a/packages/metadata-protocol/src/protocol.container-issue-descent.test.ts b/packages/metadata-protocol/src/protocol.container-issue-descent.test.ts new file mode 100644 index 0000000000..048ba5fdaf --- /dev/null +++ b/packages/metadata-protocol/src/protocol.container-issue-descent.test.ts @@ -0,0 +1,363 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #8783 — `saveMetaItem`'s `422 INVALID_METADATA` descends the CONTAINER issue + * codes, so a rejected record KEY arrives with the rule it broke. + * + * ## The defect + * + * Zod raises `invalid_key` (a `z.record`/`z.map` key schema rejected a key) and + * `invalid_element` (a `z.map` value schema rejected a value under a + * non-`PropertyKey` key) with a bare wrapper as the issue's OWN message — + * `"Invalid key in record"` — and the real diagnosis one level down in + * `issue.issues`. That is structurally the `invalid_union` shape #4971 named: + * the prescription is produced and then dropped by a walk that reads only the + * top level. + * + * Both `packages/spec` walks learned to descend those codes in #5389. + * `zodIssuesToMetadataIssues` — the walk behind this 422 (#5364) and the read + * path's diagnostics (#5598) — expanded `invalid_union` only, so it stopped at + * the wrapper. + * + * ## Why it mattered rather than being dormant drift + * + * `ObjectSchema.fields` is a record whose KEY schema carries the snake_case + * rule (`spec/src/data/object.zod.ts`), and `object` is in the builtin + * `getMetadataTypeSchema` registry. So the commonest authoring mistake on the + * most-authored metadata type — writing `firstName` for a field key — went + * `saveMetaItem` → 422 → this walk and reached Studio as + * `{path: 'fields.firstName', code: 'invalid_key', message: 'Invalid key in + * record'}`, with *"Field names must be lowercase snake_case"* stranded one + * level below. {@link https://github.com/objectstack-ai/objectstack/issues/8783} + * + * ## What these pins assert, and what they deliberately do not + * + * The descent is ADDITIVE — the wrapper entry stays and the detail joins it — + * because that is what the other two walks were **measured** to do over the + * card's own repro before this changed, not because additive is nicer: + * + * ``` + * prose (formatZodIssue) ✗ m.ab: Invalid key in record + * ✗ m.ab: Too small: expected string to have >=4 characters + * wire (zodIssuesToFields) [{field: 'm.ab', code: 'invalid_shape', …wrapper}, + * {field: 'm.ab', code: 'min_length', …detail }] + * ``` + * + * The three-walk agreement itself is pinned across the package boundary in + * `union-branch-policy.cross-package-parity.test.ts` §5. This file pins the + * envelope THIS package serves, plus the controls that show the change is a + * targeted second descent rather than a widened walk: an `invalid_union` still + * expanding exactly as it did, a plain issue still passing through untouched, + * and an `issues` payload on any other code still ignored. + */ +import { describe, expect, it } from 'vitest'; +import { z } from 'zod'; +// [#5619] The producer's OWN write-verb dispatch decisions, so the fake engine +// cannot accept a call ObjectQL would refuse. From `@objectstack/metadata-core` +// and not `@objectstack/objectql`, which depends on this package. +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { getMetadataTypeSchema } from '@objectstack/spec/kernel'; +import { ObjectStackProtocolImplementation, zodIssuesToMetadataIssues } 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'}`; + +/** The engine surface the repository write path touches (as #5364's harness). */ +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 }; +} + +async function rejection(promise: Promise): Promise { + try { + await promise; + } catch (err) { + return err; + } + throw new Error('expected the save to be rejected, but it resolved'); +} + +/** The card's measured synthetic repro, verbatim. */ +const syntheticSchema = z.object({ m: z.record(z.string().min(4), z.string()) }); +const syntheticValue = { m: { ab: 'x' } }; + +/** + * The real one: an authored object whose field key is not snake_case. + * + * `sharingModel` is present so the POSITIVE control below reaches a successful + * save — ADR-0090 D1's author-time gate refuses a custom object that declares + * no OWD, one layer AFTER the schema parse this card is about. It changes + * nothing for the rejection fixtures, which never get that far. + */ +const authoredObject = (fieldKey: string) => ({ + name: 'contact', + label: 'Contact', + sharingModel: 'private', + fields: { + [fieldKey]: { name: fieldKey, label: 'First Name', type: 'text' }, + }, +}); + +const SNAKE_CASE_RULE = 'Field names must be lowercase snake_case'; + +function issuesOf(schema: z.ZodTypeAny, value: unknown): readonly unknown[] { + const result = schema.safeParse(value); + expect(result.success, 'the fixture must actually fail to parse').toBe(false); + return (result as { error: { issues: readonly unknown[] } }).error.issues; +} + +describe('#8783 zod strands a container rejection one level down (the defect, pinned)', () => { + it('the synthetic repro raises ONE issue whose own message says nothing useful', () => { + // The reverse verification for this change, stated as a fact about zod + // rather than as a code revert: this is exactly what a walk reading + // only the top level had to work with. Take the container descent back + // out of `collectMetadataIssues` and every assertion below that reads a + // second entry goes red, because `issue.issues` is the only place the + // prescription exists. + const issues = issuesOf(syntheticSchema, syntheticValue) as any[]; + + expect(issues).toHaveLength(1); + expect(issues[0]).toMatchObject({ code: 'invalid_key', message: 'Invalid key in record' }); + expect(issues[0].path).toEqual(['m', 'ab']); + expect(issues[0].issues[0].message).toContain('expected string to have >=4 characters'); + }); + + it('the REAL repro does the same to the snake_case rule on `object.fields`', () => { + const schema = getMetadataTypeSchema('object')! as any; + const issues = issuesOf(schema, authoredObject('firstName')) as any[]; + + expect(issues).toHaveLength(1); + expect(issues[0]).toMatchObject({ code: 'invalid_key', message: 'Invalid key in record' }); + expect(issues[0].path).toEqual(['fields', 'firstName']); + // The declared prescription exists and is correct — it was simply not + // reachable from the top-level issue. + expect(issues[0].issues[0].message).toContain(SNAKE_CASE_RULE); + }); +}); + +describe('#8783 the descent: the prescription reaches the envelope', () => { + it('the card\'s synthetic repro surfaces the detail, not just the wrapper', () => { + const out = zodIssuesToMetadataIssues(issuesOf(syntheticSchema, syntheticValue)); + + expect(out).toEqual([ + // Additive: the wrapper is entry 0, unmoved, exactly as it shipped. + { path: 'm.ab', message: 'Invalid key in record', code: 'invalid_key' }, + { + path: 'm.ab', + message: 'Too small: expected string to have >=4 characters', + code: 'too_small', + }, + ]); + }); + + it('the descended entry\'s path is resolved against the container\'s own', () => { + // Container issues carry paths RELATIVE to the wrapper, the same trap + // #5014 paid for on union branches: the inner issue's own path is `[]`, + // and an unresolved reading would report the document root. + const raw = issuesOf(syntheticSchema, syntheticValue) as any[]; + expect(raw[0].issues[0].path).toEqual([]); + + expect(zodIssuesToMetadataIssues(raw)[1]!.path).toBe('m.ab'); + }); + + it('`invalid_element` descends too — the other half of the code set', () => { + // `z.map`'s VALUE schema rejecting under a non-PropertyKey key is the + // only way to reach this code, so the fixture needs an object key. + const schema = z.map(z.object({ k: z.string() }), z.string().min(4)); + const issues = issuesOf(schema, new Map([[{ k: 'a' }, 'x']])) as any[]; + + expect(issues[0].code).toBe('invalid_element'); + const out = zodIssuesToMetadataIssues(issues); + expect(out).toHaveLength(2); + expect(out[1]!.message).toContain('expected string to have >=4 characters'); + }); +}); + +describe('#8783 the 422 an author actually receives', () => { + it('a non-snake_case field key: the rule reaches the author through the envelope', async () => { + const { protocol, rows } = makeProtocol(); + + const err = await rejection( + protocol.saveMetaItem({ type: 'object', name: 'contact', item: authoredObject('firstName') }), + ); + + // The ADR-0112 envelope, both halves. + expect(err.code).toBe('INVALID_METADATA'); + expect(err.status).toBe(422); + // Load-bearing: the verdict is unchanged. #8783 moves what the refusal + // SAYS, never whether it is one — nothing is persisted either way. + expect(rows.size).toBe(0); + + // The entry that shipped before this change, unmoved at index 0: it is + // the only one naming the slot, and Studio's designer keys on it. + expect(err.issues[0]).toEqual({ + path: 'fields.firstName', + message: 'Invalid key in record', + code: 'invalid_key', + }); + + // …and the sentence that says what a valid key looks like now rides + // along, on the same slot, which is the whole card. + expect(err.issues).toHaveLength(2); + expect(err.issues[1]!.path).toBe('fields.firstName'); + expect(err.issues[1]!.message).toContain(SNAKE_CASE_RULE); + expect(err.issues[1]!.message).toContain('first_name'); + expect(err.issues[1]!.code).toBe('invalid_format'); + }); + + it('the same object with a snake_case key still saves — no rejection is invented', async () => { + const { protocol, rows } = makeProtocol(); + + const result = await protocol.saveMetaItem({ + type: 'object', + name: 'contact', + item: authoredObject('first_name'), + }); + + expect(result.success).toBe(true); + expect(rows.size).toBe(1); + }); +}); + +describe('#8783 controls — a second descent, not a widened walk', () => { + const union = (errors: unknown[][], path: unknown[] = []) => + ({ code: 'invalid_union', message: 'Invalid input', path, errors }); + + it('a plain non-container issue still passes through byte-identical', () => { + expect(zodIssuesToMetadataIssues([ + { code: 'invalid_type', message: 'Required', path: ['label'] }, + ])).toEqual([ + { path: 'label', message: 'Required', code: 'invalid_type' }, + ]); + }); + + it('an `invalid_union` still expands by the ranking, unchanged', () => { + // Verbatim the #5364 pin: fewest issues wins, `unrecognized_keys` + // breaks the tie. Container descent must not have perturbed it. + const out = zodIssuesToMetadataIssues([union([ + [{ code: 'invalid_value', message: 'wrong discriminator', path: ['kind'] }], + [{ code: 'unrecognized_keys', message: 'Unrecognized key(s): `nmae`', path: [] }], + [ + { code: 'invalid_value', message: 'wrong discriminator', path: ['kind'] }, + { code: 'invalid_type', message: 'Required', path: ['title'] }, + ], + ])]); + + expect(out).toEqual([ + { path: '', message: 'Invalid input', code: 'invalid_union' }, + { path: '', message: 'Unrecognized key(s): `nmae`', code: 'unrecognized_keys' }, + ]); + }); + + it('⛔ an `issues` payload on any OTHER code is NOT descended', () => { + // The targeting assertion. Zod hangs `issues` on the container codes; + // a walk that descended whatever nests would emit the inner entry here + // too, and would be the "widened indiscriminately" outcome the card + // ruled out. Only the code set opens the door. + const out = zodIssuesToMetadataIssues([{ + code: 'custom', + message: 'Invalid input', + path: ['x'], + issues: [{ code: 'too_small', message: 'inner detail', path: [] }], + }]); + + expect(out).toEqual([{ path: 'x', message: 'Invalid input', code: 'custom' }]); + }); + + it('a container issue with no payload is the wrapper alone, as before', () => { + expect(zodIssuesToMetadataIssues([ + { code: 'invalid_key', message: 'Invalid key in record', path: ['m', 'ab'] }, + ])).toEqual([ + { path: 'm.ab', message: 'Invalid key in record', code: 'invalid_key' }, + ]); + // …and an empty list behaves the same, rather than descending nothing + // and losing the de-duplication the non-expandable branch applies. + expect(zodIssuesToMetadataIssues([ + { code: 'invalid_key', message: 'Invalid key in record', path: ['m'], issues: [] }, + ])).toEqual([ + { path: 'm', message: 'Invalid key in record', code: 'invalid_key' }, + ]); + }); + + it('a union BELOW a container is expanded by the ranking, not emitted whole', () => { + // The two descents compose, and each level applies its own rule: the + // container emits every issue it has, the union below it selects. + const out = zodIssuesToMetadataIssues([{ + code: 'invalid_key', + message: 'Invalid key in record', + path: ['m', 'ab'], + issues: [union([ + [{ code: 'invalid_type', message: 'expected string', path: [] }], + [{ code: 'unrecognized_keys', message: 'Unrecognized key(s): `q`', path: [] }], + ])], + }]); + + expect(out.map((e) => [e.path, e.code])).toEqual([ + ['m.ab', 'invalid_key'], + ['m.ab', 'invalid_union'], + ['m.ab', 'unrecognized_keys'], + ]); + }); + + it('the nesting bound covers the container descent as well', () => { + // Four nested containers: three levels are expanded and the fourth is + // emitted as a head only, so `leaf` is never reached — the same bound + // the union descent has always applied, now shared. + const container = (path: unknown[], inner: unknown): unknown => ({ + code: 'invalid_key', message: 'Invalid key in record', path, issues: [inner], + }); + const leaf = { code: 'too_small', message: 'leaf', path: [] }; + const out = zodIssuesToMetadataIssues([ + container(['a'], container(['b'], container(['c'], container(['d'], leaf)))), + ]); + + expect(out.map((e) => e.path)).toEqual(['a', 'a.b', 'a.b.c', 'a.b.c.d']); + expect(out.some((e) => e.message === 'leaf')).toBe(false); + }); + + it('two containers rejecting the same key with the same words say it once', () => { + // The `seen` de-duplication is per top-level issue and applies to the + // descended leaves, exactly as it does under a union. + const detail = () => [{ code: 'too_small', message: 'too short', path: [] }]; + const out = zodIssuesToMetadataIssues([{ + code: 'invalid_key', + message: 'Invalid key in record', + path: ['m', 'ab'], + issues: [...detail(), ...detail()], + }]); + + expect(out).toHaveLength(2); + expect(out[1]!.message).toBe('too short'); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 3fce5f3c78..7b300832ee 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -503,8 +503,8 @@ interface MetadataIssueEntry { } /** - * How many levels of nested `invalid_union` are expanded below a top-level - * issue, and how many equally-informative branches are emitted at one level. + * How many levels of nested issues are expanded below a top-level issue, and + * how many equally-informative branches are emitted at one level. * * Both bounds — and the whole selection policy below — are the ones * `formatZodError` landed for the CLI/spec side of this defect (#4971, @@ -516,10 +516,55 @@ interface MetadataIssueEntry { * same, or one mistake gets three different prescriptions depending on whether * the author published from the terminal, POSTed to the data API, or saved from * Studio (#5364). + * + * [#8783] The depth bound is named for what it bounds — nesting — and not for + * `invalid_union`, because it now bounds the container descent below as well. + * The spec pair renamed the same constant for the same reason when #8318 + * extracted it (`NESTED_EXPANSION_DEPTH_LIMIT` in + * `spec/src/shared/union-branch-policy.ts`); the value is the same 3, and it + * has to be, for the reason in the paragraph above. */ -const UNION_EXPANSION_DEPTH_LIMIT = 3; +const NESTED_EXPANSION_DEPTH_LIMIT = 3; const UNION_BRANCH_EMIT_LIMIT = 3; +/** + * [#5389/#8783] The issue codes that hang their real diagnosis on + * `issue.issues` rather than on `invalid_union`'s `issue.errors`. + * + * Zod raises these when a CONTAINER's inner schema rejects a key or an element + * that cannot be addressed by a path segment: + * + * - `invalid_key` — `z.record(K, V)`'s **key** schema rejected a key, and + * `z.map(K, V)`'s key schema rejected a non-`PropertyKey` key; + * - `invalid_element` — `z.map(K, V)`'s **value** schema rejected the value + * under a non-`PropertyKey` key. + * + * In both cases the issue's own `message` is a bare wrapper (`"Invalid key in + * record"`) and everything the author needs sits one level down, exactly as + * `invalid_union` hides a branch's prescription in `errors` (#4971). Without + * the descent this envelope stopped at the wrapper while the other two walks + * descended, so the single most-authored metadata type's commonest authoring + * mistake — `ObjectSchema.fields` is a record whose KEY schema carries the + * snake_case rule (`spec/src/data/object.zod.ts`) — reached Studio as + * `{path: 'fields.firstName', code: 'invalid_key', message: 'Invalid key in + * record'}` with the sentence naming the rule dropped (#8783). + * + * ⚠️ These issues are NOT ranked, and that is the one structural difference + * from the union descent below. A union's branches are competing *candidates*, + * so they are selected between; a container has one inner schema, so every + * issue it produced is a true statement about the value and dropping any would + * be dropping a real diagnosis. Hence the plain loop rather than + * {@link selectUnionBranches}. + * + * ⛔ Deliberately a local declaration, not an import. `spec`'s + * `CONTAINER_ISSUE_CODES` is package-internal by decision (the #4001 export + * pitfall) and reaching for it would be #8660's option 2 — a public-surface + * decision this change does not make. `union-branch-policy.cross-package-parity.test.ts` + * §1 is the tripwire that keeps that decision explicit, and §5 there pins this + * set's *behaviour* against both spec walks. + */ +const CONTAINER_ISSUE_CODES: ReadonlySet = new Set(['invalid_key', 'invalid_element']); + /** * A zod issue, as much of it as the expansion reads. * @@ -527,12 +572,20 @@ const UNION_BRANCH_EMIT_LIMIT = 3; * with each branch's paths RELATIVE to the union issue's own path. Zod raises a * single `invalid_union` issue whose own `message` is the literal * `"Invalid input"`, so everything a failing branch has to say lives down there. + * + * `issues` is the container equivalent (#8783), carried by the + * {@link CONTAINER_ISSUE_CODES}: the one issue list the failing key/element + * schema produced, with paths relative to this issue's own — the same + * relationship `errors` has, spelled with a different property name and without + * the per-branch nesting, because a container has one inner schema rather than + * N alternatives. */ interface ZodIssueLike { path?: unknown; message?: unknown; code?: unknown; errors?: unknown; + issues?: unknown; } /** A zod issue path, normalised to the array zod always produces. */ @@ -620,16 +673,28 @@ function selectUnionBranches( * to it, which is the trap #5014 paid for: a branch issue's `path` names a slot * inside the union member, not inside the document. * - * The union's entry is kept rather than replaced: it is the only entry naming - * the slot the client sent, existing consumers already read it, and when every - * branch is uninformative it is still the whole answer. So the expansion is - * strictly ADDITIVE — no entry that shipped before this changed is gone or - * renumbered, only newly accompanied. + * [#8783] A {@link CONTAINER_ISSUE_CODES} issue descends the same way and for + * the same reason, into `issue.issues` instead of `issue.errors`, with two + * differences: nothing is ranked (a container has one inner schema, so every + * issue it raised is true — see the note on the code set), and the descent is + * only taken when the issue's own code says the payload is a container's. That + * second half is what keeps this targeted: an `issues` array hanging off any + * other code is left alone, so the walk did not become "descend whatever + * nests". + * + * The wrapper's entry is kept rather than replaced — for the union and the + * container alike: it is the only entry naming the slot the client sent, + * existing consumers already read it, and when nothing below is informative it + * is still the whole answer. So the expansion is strictly ADDITIVE — no entry + * that shipped before this changed is gone or renumbered, only newly + * accompanied. Both spec walks were measured to do the same before #8783 copied + * them (`formatZodIssue` prints the wrapper line then the indented detail; + * `zodIssuesToFields` emits the `invalid_shape` wrapper then the detail entry). * * `seen` de-duplicates entries *within one top-level issue*: two branches that - * reject the same key with the same words say it once. Union entries themselves - * are exempt, since two same-path `"Invalid input"` entries can head genuinely - * different sub-trees. + * reject the same key with the same words say it once. Expanded entries — union + * and container heads alike — are exempt, since two same-path wrapper entries + * can head genuinely different sub-trees. * * Deliberate divergence from the spec-side renderer: where it prints a trailing * "… and N more branches rejected this value", this emits nothing. That line is @@ -651,7 +716,13 @@ function collectMetadataIssues( (branch): branch is ZodIssueLike[] => Array.isArray(branch), ) : []; - const expandable = branches.length > 0 && depth < UNION_EXPANSION_DEPTH_LIMIT; + const contained: readonly ZodIssueLike[] = + typeof issue?.code === 'string' && CONTAINER_ISSUE_CODES.has(issue.code) + && Array.isArray(issue?.issues) + ? (issue.issues as ZodIssueLike[]) + : []; + const expandable = (branches.length > 0 || contained.length > 0) + && depth < NESTED_EXPANSION_DEPTH_LIMIT; const entry: MetadataIssueEntry = { path: path.join('.'), @@ -667,10 +738,18 @@ function collectMetadataIssues( out.push(entry); if (!expandable) return; - for (const branch of selectUnionBranches(branches)) { - for (const nested of branch) { - collectMetadataIssues(nested, path, depth + 1, seen, out); + if (branches.length > 0) { + for (const branch of selectUnionBranches(branches)) { + for (const nested of branch) { + collectMetadataIssues(nested, path, depth + 1, seen, out); + } } + return; + } + + // Unranked, and every one of them: see {@link CONTAINER_ISSUE_CODES}. + for (const nested of contained) { + collectMetadataIssues(nested, path, depth + 1, seen, out); } } @@ -692,8 +771,19 @@ function collectMetadataIssues( * `formatZodErrors` (#5341) — lost prescriptions; this one lost field * localisation itself. * + * A rejection inside a CONTAINER is expanded for the same reason (#8783): zod + * hangs a `z.record`/`z.map` key or element rejection on + * {@link CONTAINER_ISSUE_CODES} with the bare wrapper `"Invalid key in record"` + * as its own message and the real rule one level down in `issue.issues`. Both + * spec walks have descended those since #5389 while this one stopped at the + * wrapper, so an author who typed `firstName` for a field key was told a key + * was invalid and never told what a valid one looks like — the declared + * snake_case prescription exists, is correct, and simply did not reach the + * author. + * * Branch selection is described on {@link selectUnionBranches} and is identical - * to the other copies by construction. + * to the other copies by construction; the container descent is described on + * {@link CONTAINER_ISSUE_CODES}. */ export function zodIssuesToMetadataIssues(issues: unknown): MetadataIssueEntry[] { if (!Array.isArray(issues)) return []; diff --git a/packages/metadata-protocol/src/union-branch-policy.cross-package-parity.test.ts b/packages/metadata-protocol/src/union-branch-policy.cross-package-parity.test.ts index 8fa1494435..fce756039f 100644 --- a/packages/metadata-protocol/src/union-branch-policy.cross-package-parity.test.ts +++ b/packages/metadata-protocol/src/union-branch-policy.cross-package-parity.test.ts @@ -67,13 +67,24 @@ * will NOT redden this file. Rebuild `@objectstack/spec` first, or the run is * reporting on the previous build. * + * ## Container descent — the exclusion this file recorded, now discharged + * + * Until #8783 the bullet below read as a deliberate exclusion: the spec walks + * descended `invalid_key` / `invalid_element` (#5389) and this package's copy + * expanded `invalid_union` only, so the corpus stayed inside `invalid_union` + * where all three copies made the same claim. That difference in REACH was + * filed as its own card and fixed there: `collectMetadataIssues` now descends + * the same two codes, so §5 below compares them and the exclusion is gone + * rather than merely still true. + * + * §5 is kept SEPARATE from §2's corpus rather than folded into it, because §3 + * asserts that every §2 fixture really produces an `invalid_union` — the + * vacuity guard that keeps that corpus about branch ranking. A container + * fixture there would either break that guard or force it to be loosened, which + * would cost more than the shared list saves. + * * ## What is deliberately NOT compared * - * - **Container descent** (`invalid_key` / `invalid_element`, #5389). The spec - * walks descend those; this package's copy expands `invalid_union` only. That - * is a difference in REACH, not in the ranking, and it is not what #5014 bound - * together — so the corpus stays inside `invalid_union`, where all three - * copies make the same claim. Widening it is a separate card. * - **The `code` vocabulary.** This package emits zod's raw code, the wire emits * the ADR-0114 catalog code (#5364 records why they are not aligned). §4 * asserts that divergence in place rather than normalising it away, so it @@ -403,3 +414,61 @@ describe('#8660 §4 the deliberate asymmetries, asserted rather than normalised .toEqual(['invalid_shape', 'required']); }); }); + +/** + * [#8783] The container descent, compared the same way — the exclusion the + * header used to record, now an assertion. + * + * The three walks agree on REACH as well as on ranking: a `z.record`/`z.map` + * key or element rejection is emitted as the wrapper zod puts on the issue + * itself, followed by the issues the inner schema produced, on the wrapper's + * own slot. Nothing here is ranked — a container has one inner schema, so every + * issue it raised is true — which is why these fixtures live outside §2's + * corpus and its `invalid_union` vacuity guard. + */ +describe('#8783 §5 the container descent, in all three walks', () => { + const CONTAINER_FIXTURES: readonly Fixture[] = [ + { + rule: 'a rejected record KEY carries the key schema\'s prescription, on the key\'s own slot', + schema: z.object({ m: z.record(z.string().min(4), z.string()) }), + value: { m: { ab: 'x' } }, + expectedPaths: ['m.ab', 'm.ab'], + }, + { + rule: 'a rejected map ELEMENT descends the same way', + schema: z.object({ m: z.map(z.object({ k: z.string() }), z.string().min(4)) }), + value: { m: new Map([[{ k: 'a' }, 'x']]) }, + expectedPaths: ['m', 'm'], + }, + { + rule: 'every issue the inner schema raised is emitted — containers are not ranked', + schema: z.object({ m: z.record(z.string().min(4).regex(/^[a-z]+$/), z.string()) }), + value: { m: { A1: 'x' } }, + expectedPaths: ['m.A1', 'm.A1', 'm.A1'], + }, + ]; + + it.each(CONTAINER_FIXTURES.map((f) => [f.rule, f] as const))('%s', (_rule, fixture) => { + const issues = issuesFor(fixture.schema, fixture.value); + + // The fixture really is a container issue, so a future zod change that + // reshaped it could not leave this section green and vacuous. + expect((issues[0] as { code?: unknown }).code) + .toBe(fixture.rule.includes('ELEMENT') ? 'invalid_element' : 'invalid_key'); + + const mine = metadataPairs(issues); + expect(mine.map((v) => v.path)).toEqual([...fixture.expectedPaths]); + expect(mine).toEqual(wirePairs(issues, fixture.value)); + expect(mine).toEqual(prosePairs(issues)); + }); + + it('the descent is additive in all three — the wrapper is kept, never replaced', () => { + const fixture = CONTAINER_FIXTURES[0]!; + const issues = issuesFor(fixture.schema, fixture.value); + + for (const pairs of [metadataPairs(issues), wirePairs(issues, fixture.value), prosePairs(issues)]) { + expect(pairs[0]!.message).toBe('Invalid key in record'); + expect(pairs[1]!.message).toContain('expected string to have >=4 characters'); + } + }); +});