diff --git a/.changeset/format-zod-error-union-branches.md b/.changeset/format-zod-error-union-branches.md new file mode 100644 index 0000000000..c197bf8552 --- /dev/null +++ b/.changeset/format-zod-error-union-branches.md @@ -0,0 +1,64 @@ +--- +'@objectstack/spec': minor +--- + +`formatZodError` no longer cuts a union branch's rejection down to `Invalid input` (#4971). + +Zod raises exactly one issue for a failed `z.union` — `invalid_union`, whose own +`message` is the literal string `"Invalid input"` — and keeps each branch's real +issues one level down in `issue.errors[]`. `formatZodError` mapped `error.issues` +and never descended, so every rejection behind a union reached the author as: + +``` +Action ref (1 issue): + + ✗ (root): Invalid input +``` + +while the same class of mistake on a plain `strictObject` rendered its full +prescription. The prose was never lost — the REST error body and +`ZodError.message` carry the payload through. What dropped it is every consumer +that flattens to one line, and `formatZodError` is one: it is this package's +documented CLI formatter and what `defineStack` throws through, so an author +whose stack config has a union mistake reads it on every command that loads the +stack. Every `strictObject` that sits inside a union was affected; `ActionRef` / +`GuardRef` in `automation/state-machine.zod.ts` are the measured specimens, and +the surface grows with each #4001 strictness batch. + +The formatter now expands a failed union one level deeper: + +``` +Action ref (1 issue): + + ✗ (root): Invalid input + ✗ (root): Unrecognized key(s) on this action reference: `args`. Until #4001 … +``` + +**What gets printed, and what deliberately does not.** Printing every branch is +the failure this fix must not cause — a plain union of four strict members +reports one bad key once per member, which is why `view.zod.ts`'s +`submitBehavior` moved to `discriminatedUnion`. So branches are selected, not +dumped: + +- A branch that only says the value is the wrong **kind** at its root + (`expected string, received object`, from the `z.string()` member of + `z.union([z.string(), SomeObject])`) carries no prescription and is dropped. + When *every* branch is one — `z.union([z.string(), z.number()])` handed an + object — nothing is expanded and the output is byte-identical to before. +- Among the rest, the branch reporting the **fewest** issues wins: the member + the author was aiming at complains only about the stray key, while the others + also report a wrong discriminator and their own missing requireds. A + `unrecognized_keys` issue breaks a tie, declaration order breaks what remains. +- Branches that genuinely tie are all rendered (max 3), and identical verdicts + across them are printed once — so a single unknown key is reported once, never + once per branch. +- Nested unions expand recursively with absolute paths + (`✗ states.s.on.GO.actions.0: …`), bounded to three levels. + +The issue **count** in the header is unchanged: it stays `error.issues.length`, +so the CLI keeps agreeing with the structural consumers about how many things +are wrong — a union is one issue however many lines explain it. + +User-visible for anyone who prints `formatZodError` / `formatZodIssue` / +`safeParsePretty` output, including `defineStack`'s thrown message. No schema, +signature or accepted input changes. diff --git a/packages/spec/src/automation/state-machine.test.ts b/packages/spec/src/automation/state-machine.test.ts index b58de7d5d1..8ad0962380 100644 --- a/packages/spec/src/automation/state-machine.test.ts +++ b/packages/spec/src/automation/state-machine.test.ts @@ -282,20 +282,30 @@ describe('[#4001] ActionRef / GuardRef — strict inside a union', () => { }); // Anti-vacuity for the claim above: a PLAIN strictObject in this same file - // does surface its prose through the same formatter, so the union's - // `Invalid input` is a property of the union and not of the curation. If - // this control ever goes quiet too, the diagnosis changes completely. - it('CONTROL — a non-union shape renders its full prescription through formatZodError', () => { + // surfaces its prose through the same formatter, so anything the union + // renders differently is a property of the union and not of the curation. + // + // ⚠️ THIS PIN WAS FLIPPED BY #4971, exactly as 批 10 predicted it would be. + // It used to assert the second half was flattened away — `formatZodError` + // mapped `error.issues` and never descended into `invalid_union.errors`, so + // the curated rejection stopped at `✗ (root): Invalid input` on the CLI path + // while the REST body and `ZodError.message` carried it fine. The formatter + // now expands the union's most informative branch, and the two halves say + // the same thing again. What is still union-shaped is the extra `Invalid + // input` line above the prescription: zod raises one issue for the union, + // and that line is what says "no branch matched". + it('CONTROL — union and non-union shapes both render their prescription through formatZodError', () => { const plain = TransitionSchema.safeParse({ target: 't', guard: 'isX' }); expect(formatZodError(plain.error!)).toContain('`guard` → `cond`'); const union = ActionRefSchema.safeParse({ type: 'log', args: { a: 1 } }); - // Same formatter, same class of mistake, flattened to nothing usable. - // `formatZodError` maps `error.issues` and never descends into - // `invalid_union.errors` — filed as a finding, deliberately not fixed in a - // spec-strictness PR (it would change CLI output for every union). - expect(formatZodError(union.error!)).toContain('Invalid input'); - expect(formatZodError(union.error!)).not.toContain('this action reference'); + const formatted = formatZodError(union.error!); + expect(formatted).toContain('Invalid input'); + expect(formatted).toContain('this action reference'); + expect(formatted).toContain('`args`'); + // The string branch's "expected string, received object" is not a + // prescription and is not printed — see #4971's branch selection. + expect(formatted).not.toContain('expected string'); }); }); diff --git a/packages/spec/src/automation/state-machine.zod.ts b/packages/spec/src/automation/state-machine.zod.ts index 12a6197e29..f378709c07 100644 --- a/packages/spec/src/automation/state-machine.zod.ts +++ b/packages/spec/src/automation/state-machine.zod.ts @@ -80,25 +80,24 @@ const STATE_MACHINE_STRIP_HISTORY = * A union: the string form names a registered action, the object form * parameterises one. Only the OBJECT branch has keys to be strict about. * - * ⚠️ **The rejection is quieter here than on a plain shape, and that is a zod - * property, not a curation gap.** A failing branch does not raise its own - * issue to the top: the union raises ONE `invalid_union` issue whose - * `message` is the literal string `"Invalid input"`, with each branch's real - * issues nested one level down in `issue.errors[]`. Measured, both ways — - * `TransitionSchema` (a plain `strictObject`) renders its full prescription - * through `formatZodError`, while this schema renders `✗ (root): Invalid - * input` for the same class of mistake, with the prescription intact in the - * payload underneath. The nested message survives everywhere the issues are - * carried structurally (`ZodError.message`, the REST error body); it is the - * flatten-to-one-line consumers that drop it, and `formatZodError` is one — - * filed as a finding rather than fixed here, since teaching that shared - * formatter to descend changes CLI output for every union in the repo. + * ⚠️ **The rejection is shaped differently here than on a plain shape, and + * that is a zod property, not a curation gap.** A failing branch does not + * raise its own issue to the top: the union raises ONE `invalid_union` issue + * whose `message` is the literal string `"Invalid input"`, with each branch's + * real issues nested one level down in `issue.errors[]`. That payload survives + * everywhere the issues are carried structurally (`ZodError.message`, the REST + * error body); until #4971 the flatten-to-one-line consumers dropped it, and + * `formatZodError` — what `defineStack` throws through — was one, so this + * schema rendered a bare `✗ (root): Invalid input` where `TransitionSchema` (a plain + * `strictObject`) rendered its full prescription. `formatZodError` now expands + * the union's most informative branch, so both render the prescription; what + * remains union-shaped is the extra `Invalid input` line above it. * - * Strictness still earns its place: the alternative is not a better message, - * it is `params` misspelled as `args` **accepted in silence**, with the - * action running unparameterised. Rejection beats that even at "Invalid - * input". Both facts are pinned in `state-machine.test.ts` so neither the - * quietness nor the underlying prose can regress unnoticed. + * Strictness earned its place even before that fix: the alternative was never + * a better message, it is `params` misspelled as `args` **accepted in + * silence**, with the action running unparameterised. Rejection beat that even + * at "Invalid input". Both facts are pinned in `state-machine.test.ts` so + * neither the union's shape nor the underlying prose can regress unnoticed. */ export const ActionRefSchema = lazySchema(() => z.union([ z.string().describe('Action Name'), diff --git a/packages/spec/src/shared/error-map.test.ts b/packages/spec/src/shared/error-map.test.ts index 6b769f10a7..9ab90dc3f5 100644 --- a/packages/spec/src/shared/error-map.test.ts +++ b/packages/spec/src/shared/error-map.test.ts @@ -169,6 +169,164 @@ describe('formatZodError', () => { }); }); +// ─── [#4971] union branches ───────────────────────────────────────────────── +// +// Zod raises ONE `invalid_union` issue whose own `message` is the literal +// string `"Invalid input"`; every branch's real rejection sits one level down +// in `issue.errors[]`. Structural consumers (the REST error body, +// `ZodError.message`) carry that payload through — a flatten-to-one-line +// consumer does not, and `formatZodError` is exactly such a consumer: it is +// documented for CLI output and is what `defineStack` throws through, so every +// author loading a stack config reads it. Until #4971 the curated prose the +// #4001 campaign wrote for every strict shape behind a union was cut off +// before it reached them. (`os validate` / `os build` print through the CLI's +// OWN `formatZodErrors`, which still flattens the same way — #5341.) +// +// The whole risk of fixing it is the opposite failure: N branches × the same +// mistake = the same key reported N times, which is why `view.zod.ts`'s +// `submitBehavior` reached for `discriminatedUnion` in the first place. Both +// directions are pinned below. +describe('[#4971] formatZodError expands invalid_union branches', () => { + // The campaign's shape: a string form OR a strict object form. + const ACTION_REF = z.union([ + z.string(), + z.strictObject({ type: z.string(), params: z.record(z.string(), z.unknown()).optional() }), + ]); + + it('renders the failing branch prose under the union line', () => { + const result = ACTION_REF.safeParse({ type: 'log', args: { a: 1 } }); + expect(result.success).toBe(false); + + const formatted = formatZodError(result.error!); + // The union's own line is preserved — it is what says "no branch matched". + expect(formatted).toContain('✗ (root): Invalid input'); + // …and the branch's prescription now arrives with it, indented one level. + expect(formatted).toContain(' ✗ (root): Unrecognized key: "args"'); + }); + + it('drops the kind-mismatch branch that carries no prescription', () => { + const formatted = formatZodError(ACTION_REF.safeParse({ type: 'log', args: 1 }).error!); + // `expected string, received object` is the string branch complaining that + // the author did not write a string. They never meant to. + expect(formatted).not.toContain('expected string'); + }); + + it('resolves nested paths against the union, not relative to it', () => { + const schema = z.object({ actions: z.array(ACTION_REF) }); + const formatted = formatZodError( + schema.safeParse({ actions: [{ type: 'log', args: { a: 1 } }] }).error!, + ); + expect(formatted).toContain('✗ actions.0: Invalid input'); + expect(formatted).toContain('✗ actions.0: Unrecognized key: "args"'); + // Never the bare relative path a naive splice would print. + expect(formatted).not.toContain('✗ (root): Unrecognized key'); + }); + + it('expands a union nested inside a union', () => { + const schema = z.object({ on: z.union([z.string(), z.object({ actions: z.array(ACTION_REF) })]) }); + const formatted = formatZodError( + schema.safeParse({ on: { actions: [{ type: 'log', args: { a: 1 } }] } }).error!, + ); + expect(formatted).toContain(' ✗ on: Invalid input'); + expect(formatted).toContain(' ✗ on.actions.0: Invalid input'); + expect(formatted).toContain(' ✗ on.actions.0: Unrecognized key: "args"'); + }); + + // ⚠️ THE anti-regression. #4001 批 6c measured a plain `z.union` of four + // strict members reporting one bad key once per member; `submitBehavior` + // switched to `discriminatedUnion` because of it. Selecting the branch that + // complains LEAST is what keeps the expansion from reintroducing it: the + // member the author was aiming at reports only the stray key, while the + // others also report a wrong discriminator and their own missing requireds. + it('reports one unknown key ONCE, not once per branch', () => { + const union = z.union([ + z.strictObject({ kind: z.literal('a'), x: z.string() }), + z.strictObject({ kind: z.literal('b'), y: z.string() }), + z.strictObject({ kind: z.literal('c'), z: z.string() }), + ]); + const formatted = formatZodError(union.safeParse({ kind: 'a', x: 'ok', bogus: 1 }).error!); + + expect(formatted.match(/bogus/g)?.length).toBe(1); + // The two shapes the author was not writing stay out of the output. + expect(formatted).not.toContain('expected "b"'); + expect(formatted).not.toContain('expected "c"'); + }); + + it('de-duplicates a verdict every tied branch reaches', () => { + // Disjoint shapes, so no branch is "closer": each reports the same stray + // key plus its own missing required. Both are rendered — neither shape is + // more expected than the other — but the shared line is said once. + const union = z.union([ + z.strictObject({ a: z.string() }), + z.strictObject({ b: z.string() }), + ]); + const formatted = formatZodError(union.safeParse({ bogus: 1 }).error!); + + expect(formatted.match(/bogus/g)?.length).toBe(1); + expect(formatted).toContain('✗ a:'); + expect(formatted).toContain('✗ b:'); + }); + + it('caps a wide tie and says how many branches it did not print', () => { + // Five disjoint shapes, none closer than the others: every branch reports + // the same stray key plus its own missing required. Three are rendered and + // the remainder is stated rather than dropped in silence. + const union = z.union([ + z.strictObject({ a: z.string() }), + z.strictObject({ b: z.string() }), + z.strictObject({ c: z.string() }), + z.strictObject({ d: z.string() }), + z.strictObject({ e: z.string() }), + ]); + const formatted = formatZodError(union.safeParse({ bogus: 1 }).error!); + + expect(formatted).toContain('… and 2 more branches rejected this value'); + expect(formatted.match(/bogus/g)?.length).toBe(1); + expect(formatted).toContain('✗ c:'); + expect(formatted).not.toContain('✗ d:'); + }); + + // The conservative half of the contract: when NO branch has anything to say + // beyond "that is the wrong kind of value", the expansion adds N lines of + // noise for zero prescription. Primitive unions are the commonest union in + // the repo, so their output is deliberately left byte-identical to what it + // was before #4971. + it('leaves an all-kind-mismatch union exactly as it was', () => { + const formatted = formatZodError(z.union([z.string(), z.number()]).safeParse({}).error!); + expect(formatted).toBe('Validation failed (1 issue):\n\n ✗ (root): Invalid input'); + }); + + it('counts the union as the one issue zod raised', () => { + // The header must keep agreeing with `error.issues` / the REST body, or + // the CLI and the API would disagree about how many things are wrong. + const result = ACTION_REF.safeParse({ type: 'log', args: { a: 1 } }); + expect(result.error!.issues).toHaveLength(1); + expect(formatZodError(result.error!)).toContain('(1 issue)'); + }); + + it('stops expanding after three levels of nesting', () => { + // Bounded output: unions nest arbitrarily, the terminal does not. + const leaf = z.strictObject({ ok: z.string() }); + const l1 = z.union([z.string(), leaf]); + const l2 = z.union([z.string(), z.object({ n: l1 })]); + const l3 = z.union([z.string(), z.object({ n: l2 })]); + const l4 = z.union([z.string(), z.object({ n: l3 })]); + + const formatted = formatZodError( + l4.safeParse({ n: { n: { n: { ok: 'x', bogus: 1 } } } }).error!, + ); + const depths = formatted + .split('\n') + .filter((line) => line.includes('✗')) + .map((line) => line.length - line.trimStart().length); + // Three expansions below the top-level line, two spaces each: 2, 4, 6, 8. + expect(Math.max(...depths)).toBe(8); + // The deepest union is printed, but not expanded — so the innermost + // prescription is the one thing that does NOT reach the author here. + expect(formatted).not.toContain('bogus'); + }); +}); + describe('safeParsePretty', () => { it('should return success with data for valid input', () => { const schema = z.object({ name: z.string() }); diff --git a/packages/spec/src/shared/error-map.zod.ts b/packages/spec/src/shared/error-map.zod.ts index 2eb33ee956..3c6b855919 100644 --- a/packages/spec/src/shared/error-map.zod.ts +++ b/packages/spec/src/shared/error-map.zod.ts @@ -133,19 +133,173 @@ interface ZodIssueMinimal { path: PropertyKey[]; message: string; code?: string; + /** + * Only on `invalid_union`: one issue list **per union branch**, 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 here. + */ + errors?: readonly (readonly ZodIssueMinimal[])[]; +} + +/** One indent step of a formatted issue line. */ +const ISSUE_INDENT = ' '; + +/** + * How many levels of nested `invalid_union` are expanded below a top-level + * issue. Unions nest (a union member that is itself a union — `StateMachine → + * on.GO → actions[0]` is two levels in this repo today), and each level can + * render several branches, so the expansion is bounded rather than left to the + * shape of whatever the author typed. + */ +const UNION_EXPANSION_DEPTH_LIMIT = 3; + +/** How many equally-informative branches are rendered at one level. */ +const UNION_BRANCH_RENDER_LIMIT = 3; + +/** + * True when a branch only complains that the value is the wrong *kind* at the + * branch root — `expected string, received object` for the string member of + * `z.union([z.string(), SomeObject])`. + * + * Such a branch carries no prescription: the author never intended it, and + * printing it is the "N branches, N times the noise" failure that made + * `view.zod.ts`'s `submitBehavior` reach for `discriminatedUnion`. An empty + * branch (the `invalid_union` "matched multiple" variant carries `errors: []`) + * counts as uninformative too — `every` on an empty list is `true`. + */ +function isKindMismatchOnly(issues: readonly ZodIssueMinimal[]): boolean { + return issues.every( + (issue) => + issue.path.length === 0 && + (issue.code === 'invalid_type' || issue.code === 'invalid_value'), + ); +} + +/** True when a branch carries the #4001 campaign's unknown-key prescription. */ +function carriesUnknownKey(issues: readonly ZodIssueMinimal[]): boolean { + return issues.some((issue) => issue.code === 'unrecognized_keys'); } /** - * Format a single Zod issue into a human-readable line. + * Pick the branch(es) of a failed union whose issues actually explain the + * failure. + * + * Ranking, in order: + * + * 1. **Kind-mismatch-only branches are dropped entirely** (see + * {@link isKindMismatchOnly}). If *every* branch is one — a plain + * `z.union([z.string(), z.number()])` handed an object — nothing is + * selected and the union renders exactly as it always has. + * 2. **Fewest issues wins.** The branch the author was closest to hitting + * complains least: given `z.union([A, B, C])` of strict objects and one + * mistyped key, the intended member reports *only* that key while the other + * two also report a wrong discriminator and their own missing requireds. So + * "fewest" is what keeps a single unknown key from being reported once per + * branch. + * 3. **A branch carrying `unrecognized_keys` breaks a tie**, because that is + * where the curated prose lives. + * 4. Declaration order breaks what remains, so the output is deterministic. + * + * Branches that tie at the top are *all* rendered (capped): when two shapes + * explain the failure equally well, privileging the first one by accident of + * declaration order would be a lie about which shape was expected. + */ +function selectUnionBranches( + branches: readonly (readonly ZodIssueMinimal[])[], +): { selected: readonly (readonly ZodIssueMinimal[])[]; omitted: number } { + const informative = branches + .map((issues, index) => ({ issues, index })) + .filter((branch) => !isKindMismatchOnly(branch.issues)); + + if (informative.length === 0) return { selected: [], omitted: 0 }; + + const rank = (branch: { issues: readonly ZodIssueMinimal[] }): [number, number] => [ + branch.issues.length, + carriesUnknownKey(branch.issues) ? 0 : 1, + ]; + + const sorted = [...informative].sort((a, b) => { + const [aCount, aKeys] = rank(a); + const [bCount, bKeys] = rank(b); + return aCount - bCount || aKeys - bKeys || a.index - b.index; + }); + + const [bestCount, bestKeys] = rank(sorted[0]!); + const tied = sorted.filter((branch) => { + const [count, keys] = rank(branch); + return count === bestCount && keys === bestKeys; + }); + + return { + selected: tied.slice(0, UNION_BRANCH_RENDER_LIMIT).map((branch) => branch.issues), + omitted: Math.max(0, tied.length - UNION_BRANCH_RENDER_LIMIT), + }; +} + +/** Render a path array the way the CLI has always rendered it. */ +function renderPath(path: PropertyKey[]): string { + return path.length > 0 ? path.join('.') : '(root)'; +} + +/** + * Render one issue and — for `invalid_union` — the selected branches beneath + * it, one indent level deeper, with paths resolved against the union's own. + * + * `seen` de-duplicates leaf lines *within one top-level issue*: two branches + * that reject the same key with the same words say it once. Union lines + * themselves are never de-duplicated, since two same-path `"Invalid input"` + * lines can head genuinely different sub-trees. + */ +function renderIssue( + issue: ZodIssueMinimal, + parentPath: PropertyKey[], + depth: number, + seen: Set, +): string[] { + const path = [...parentPath, ...issue.path]; + const rendered = renderPath(path); + const branches = issue.code === 'invalid_union' ? (issue.errors ?? []) : []; + const expandable = branches.length > 0 && depth < UNION_EXPANSION_DEPTH_LIMIT; + + if (!expandable) { + const key = JSON.stringify([depth, rendered, issue.message]); + if (seen.has(key)) return []; + seen.add(key); + } + + const lines = [`${ISSUE_INDENT.repeat(depth + 1)}✗ ${rendered}: ${issue.message}`]; + if (!expandable) return lines; + + const { selected, omitted } = selectUnionBranches(branches); + for (const branch of selected) { + for (const nested of branch) { + lines.push(...renderIssue(nested, path, depth + 1, seen)); + } + } + if (selected.length > 0 && omitted > 0) { + lines.push( + `${ISSUE_INDENT.repeat(depth + 2)}… and ${omitted} more branch${omitted === 1 ? '' : 'es'} rejected this value`, + ); + } + return lines; +} + +/** + * Format a single Zod issue into human-readable line(s). + * + * One line for an ordinary issue. For an `invalid_union`, the union's own line + * (zod's literal `"Invalid input"`) is followed by the branch issues that + * explain the rejection, indented one level and carrying absolute paths — + * without this, every rejection behind a union arrives at the author as + * `✗ (root): Invalid input` with the prescription stranded in the payload + * (#4971). Branch selection is described on {@link selectUnionBranches}. * * @param issue - A single Zod issue - * @returns Formatted string with path and message + * @returns Formatted string; multi-line when a union is expanded */ export function formatZodIssue(issue: ZodIssueMinimal): string { - const path = issue.path.length > 0 - ? issue.path.join('.') - : '(root)'; - return ` ✗ ${path}: ${issue.message}`; + return renderIssue(issue, [], 0, new Set()).join('\n'); } /** @@ -176,6 +330,18 @@ export function formatZodIssue(issue: ZodIssueMinimal): string { * ✗ objects[0].fields.status.type: Invalid field type 'dropdown'. Did you mean 'select'? * ✗ views[0].object: Invalid identifier 'MyTasks'. Must be lowercase snake_case. * ``` + * + * A rejection behind a `z.union` is expanded one level deeper, so the branch's + * message reaches the author instead of zod's bare `"Invalid input"` (#4971): + * ``` + * Validation failed (1 issue): + * + * ✗ states.s.on.GO.actions.0: Invalid input + * ✗ states.s.on.GO.actions.0: Unrecognized key(s) on this action reference: `args`. … + * ``` + * The issue **count** stays the count of `error.issues` — the union is one + * issue no matter how many lines explain it, which keeps this header agreeing + * with the structural consumers (REST error bodies, `ZodError.message`). */ export function formatZodError(error: z.ZodError, label?: string): string { const count = error.issues.length;