diff --git a/.changeset/adr-0114-field-error-catalog.md b/.changeset/adr-0114-field-error-catalog.md new file mode 100644 index 0000000000..23c8082abd --- /dev/null +++ b/.changeset/adr-0114-field-error-catalog.md @@ -0,0 +1,53 @@ +--- +"@objectstack/spec": minor +"@objectstack/rest": minor +"@objectstack/objectql": minor +--- + +feat(spec,rest,objectql)!: a closed field-level error catalog, and Zod stops leaking onto the wire (#3977) + +Settles the vocabulary ADR-0112 D6 deferred, per [ADR-0114](https://github.com/objectstack-ai/objectstack/blob/main/docs/adr/0114-field-level-error-code-catalog.md). + +**`FieldErrorCode` — a closed, lowercase catalog.** 27 members covering what the +six emitters already emit. `FieldErrorSchema.code` tightens from `z.string()` to +this enum, so a validation body's per-field codes are validated for the first time. +`FieldValidationError.code` (objectql) and `FieldCoerceError.code` (rest) stop +being a hand-listed union and a bare `string` respectively and reference the +catalog, so the three cannot drift apart. + +Lowercase is deliberate, not an oversight against ADR-0112's SCREAMING_SNAKE: a +top-level code names the condition the *request* hit, while a field-level code +names the *constraint* the value violated — and constraints are declared in the +metadata's own snake_case, so `max_length` the code and `max_length: 50` the +property are the same word on purpose. + +**Zod issue codes no longer reach the wire (wire-visible).** Routes that validate +with Zod passed its vocabulary straight through, so `fields[]` spoke a different +language depending on which route served it, and `too_small` was ambiguous between +a short string, a small number and a short array. `zodIssuesToFields` now maps +using Zod's `origin`/`format`: + +| Was | Now | +|:---|:---| +| `too_small` | `min_length` / `min_value` / `min_items` | +| `too_big` | `max_length` / `max_value` / `max_items` | +| `invalid_format` | `invalid_email` / `invalid_url` / `invalid_format` | +| `invalid_value` | `invalid_option` | +| `unrecognized_keys` | `unknown_field` | +| `invalid_union`, `invalid_element`, `invalid_key` | `invalid_shape` | + +**A missing required property now reports `required`, not `invalid_type`.** Zod +spells "absent" as a type mismatch against `undefined`, so passing it through made +a form mark a *missing* input as the wrong *type*. The two are indistinguishable on +the issue alone, so the mapper takes the parsed input as an optional argument and +walks the issue path; a caller that cannot supply it keeps `invalid_type` rather +than guessing. + +**`unknown_param` → `unknown_field`.** `ActionParamIssue.code` references the +catalog instead of its own literal union; the `param` key beside it already says +what was addressed. + +**Not changed:** `EnhancedApiErrorSchema.fieldErrors` keeps its name even though +every producer emits `fields`. Retiring an authorable key needs a tombstone plus a +migration (ADR-0104's contract guard), so it lands on its own — the property now +carries a banner saying which name the wire uses. diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index bf34d99c17..8af2431f69 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -443,15 +443,48 @@ interface EnhancedApiError { ```typescript interface FieldError { field: string; // Field path (supports dot notation) - code: string; // Field-level error code — a SEPARATE vocabulary from - // error.code (`required`, `max_length`, `invalid_email`, …); - // its own catalog is #3977 (ADR-0112 D6) + code: FieldErrorCode; // Which CONSTRAINT the value violated — a closed, + // lowercase catalog, separate from error.code (ADR-0114) message: string; // Human-readable error for this field value?: unknown; // The invalid value (if safe to include) constraint?: unknown; // The constraint that was violated (e.g., max length) } ``` +### Field-level codes + +`FieldError.code` is its **own** closed vocabulary, and it is lowercase where +`error.code` is SCREAMING. That is deliberate (ADR-0114): a top-level code names +the condition the *request* hit, while a field-level code names the *constraint* +the value violated — and constraints are declared in the metadata's own +snake_case, so the code and the schema property are the same word. + +``` +{ required: true } → code 'required' +{ max_length: 50 } → code 'max_length' +{ min_value: 0 } → code 'min_value' +``` + +| Group | Codes | +|:---|:---| +| Presence and shape | `required`, `invalid_type`, `invalid_shape`, `unknown_field` | +| Per-type parse | `invalid_boolean`, `invalid_number`, `invalid_date`, `invalid_time`, `invalid_email`, `invalid_url`, `invalid_phone`, `invalid_json`, `invalid_format` | +| Bounded ranges | `min_length`, `max_length`, `min_value`, `max_value`, `min_items`, `max_items` | +| Closed sets and references | `invalid_option`, `invalid_value`, `reference_not_found`, `reference_ambiguous` | +| Declarative rules | `rule_violation`, `json_schema_violation`, `invalid_initial_state`, `invalid_transition` | + +Branch on `code` to decide *how* to mark an input; show `message` to the user. +Routes that parse with Zod map its issue codes into this catalog rather than +passing them through, so `fields[]` speaks one vocabulary whichever route served +it. + + +The declared envelope still calls this array `fieldErrors`, while every producer +emits **`fields`**. Read `fields`. The rename is decided but deferred — retiring +an authorable key needs a tombstone plus a migration, so it lands on its own +(ADR-0114 D4). + + --- ## Client-Side Error Handling diff --git a/content/docs/api/error-handling-client.mdx b/content/docs/api/error-handling-client.mdx index 041600fd86..8b38fb306e 100644 --- a/content/docs/api/error-handling-client.mdx +++ b/content/docs/api/error-handling-client.mdx @@ -19,6 +19,8 @@ Every error response from ObjectStack follows this shape: {/* os:check */} ```typescript +import type { FieldErrorCode } from '@objectstack/spec/api'; + interface ErrorResponse { success: false; // Always false for error responses error: { @@ -29,9 +31,15 @@ interface ErrorResponse { retryable?: boolean; // Whether the request can be retried retryStrategy?: string; retryAfter?: number; // Seconds to wait before retrying (rate limits) + // ⚠️ Declared as `fieldErrors`; every producer emits `fields`. Read `fields`. + // (ADR-0114 D4 — the rename needs a tombstone + migration, so it lands on + // its own.) fieldErrors?: Array<{ field: string; // Field path (supports dot notation) - code: string; // Per-field error code (e.g. "INVALID_FORMAT") + // Which CONSTRAINT the value violated — a lowercase `FieldErrorCode` + // (`required`, `max_length`, `invalid_email`, …), NOT a top-level code. + // The two vocabularies are separate on purpose: see ADR-0114. + code: FieldErrorCode; message: string; // Detail message value?: unknown; // The invalid value that was provided constraint?: unknown; diff --git a/content/docs/api/error-handling-server.mdx b/content/docs/api/error-handling-server.mdx index 0902b2d5b5..825ae8e144 100644 --- a/content/docs/api/error-handling-server.mdx +++ b/content/docs/api/error-handling-server.mdx @@ -48,7 +48,10 @@ class TaskAlreadyCompletedError extends AppError { 'RESOURCE_CONFLICT', `Task ${taskId} is already completed and cannot be modified`, 409, - [{ field: 'status', code: 'lock_conflict', message: 'Task is in terminal state' }], + // No per-field entry: a terminal-state conflict is a condition of the + // RECORD, not a constraint some value violated, and the top-level + // RESOURCE_CONFLICT already says so. `fields[]` is for "this value is + // wrong", which is why its codes name constraints. ); } } @@ -59,14 +62,16 @@ class InvalidStatusTransitionError extends AppError { 'INVALID_FIELD', `Cannot transition from '${from}' to '${to}'`, 400, - [{ field: 'status', code: 'invalid_field', message: `Invalid transition: ${from} → ${to}` }], + // `invalid_transition` — the catalog has a member for exactly this, and it + // is more useful to a form than a generic "this field is invalid". + [{ field: 'status', code: 'invalid_transition', message: `Invalid transition: ${from} → ${to}` }], ); } } ``` -**Error Codes:** The top-level `code` is SCREAMING_SNAKE (`StandardErrorCode` or a ledger-registered code). Per-field `code` values are a **separate, lowercase vocabulary** — the validators emit `required`, `max_length`, `invalid_email`, … and its catalog is being settled in [#3977](https://github.com/objectstack-ai/objectstack/issues/3977) (ADR-0112 D6). Use the `category` and `httpStatus` that match — `ErrorHttpStatusMap` in `@objectstack/spec/api` maps each category to its HTTP status. +**Error Codes:** The top-level `code` is SCREAMING_SNAKE (`StandardErrorCode` or a ledger-registered code). Per-field `code` values are a **separate, lowercase catalog** — `FieldErrorCode`, closed, naming the constraint the value violated (`required`, `max_length`, `invalid_transition`, …). See the [Error Catalog](/docs/api/error-catalog#field-level-codes); the two vocabularies and why they differ are ADR-0114. Use the `category` and `httpStatus` that match — `ErrorHttpStatusMap` in `@objectstack/spec/api` maps each category to its HTTP status. --- diff --git a/content/docs/references/api/errors.mdx b/content/docs/references/api/errors.mdx index 01049f378f..451719b2d9 100644 --- a/content/docs/references/api/errors.mdx +++ b/content/docs/references/api/errors.mdx @@ -30,8 +30,8 @@ Industry alignment: Google Cloud Errors, AWS Error Codes, Stripe API Errors ## TypeScript Usage ```typescript -import { EnhancedApiError, ErrorCategory, ErrorResponse, FieldError, RetryStrategy, StandardErrorCode } from '@objectstack/spec/api'; -import type { EnhancedApiError, ErrorCategory, ErrorResponse, FieldError, RetryStrategy, StandardErrorCode } from '@objectstack/spec/api'; +import { EnhancedApiError, ErrorCategory, ErrorResponse, FieldError, FieldErrorCode, RetryStrategy, StandardErrorCode } from '@objectstack/spec/api'; +import type { EnhancedApiError, ErrorCategory, ErrorResponse, FieldError, FieldErrorCode, RetryStrategy, StandardErrorCode } from '@objectstack/spec/api'; // Validate data const result = EnhancedApiError.parse(data); @@ -53,7 +53,7 @@ const result = EnhancedApiError.parse(data); | **retryStrategy** | `Enum<'no_retry' \| 'retry_immediate' \| 'retry_backoff' \| 'retry_after'>` | optional | Recommended retry strategy | | **retryAfter** | `number` | optional | Seconds to wait before retrying | | **details** | `any` | optional | Additional error context | -| **fieldErrors** | `{ field: string; code: string; message: string; value?: any; … }[]` | optional | Field-specific validation errors | +| **fieldErrors** | `{ field: string; code: Enum<'required' \| 'invalid_type' \| 'invalid_shape' \| 'unknown_field' \| 'invalid_boolean' \| 'invalid_number' \| 'invalid_date' \| 'invalid_time' \| 'invalid_email' \| 'invalid_url' \| 'invalid_phone' \| 'invalid_json' \| 'invalid_format' \| 'min_length' \| 'max_length' \| 'min_value' \| 'max_value' \| 'min_items' \| 'max_items' \| 'invalid_option' \| 'invalid_value' \| 'reference_not_found' \| 'reference_ambiguous' \| 'rule_violation' \| 'json_schema_violation' \| 'invalid_initial_state' \| 'invalid_transition'>; message: string; value?: any; … }[]` | optional | Field-specific validation errors (wire name is `fields` — see ADR-0114 D4) | | **timestamp** | `string` | optional | When the error occurred | | **requestId** | `string` | optional | Request ID for tracking | | **traceId** | `string` | optional | Distributed trace ID | @@ -100,12 +100,47 @@ const result = EnhancedApiError.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **field** | `string` | ✅ | Field path (supports dot notation) | -| **code** | `string` | ✅ | Error code for this field (field-level vocabulary — see #3977) | +| **code** | `Enum<'required' \| 'invalid_type' \| 'invalid_shape' \| 'unknown_field' \| 'invalid_boolean' \| 'invalid_number' \| 'invalid_date' \| 'invalid_time' \| 'invalid_email' \| 'invalid_url' \| 'invalid_phone' \| 'invalid_json' \| 'invalid_format' \| 'min_length' \| 'max_length' \| 'min_value' \| 'max_value' \| 'min_items' \| 'max_items' \| 'invalid_option' \| 'invalid_value' \| 'reference_not_found' \| 'reference_ambiguous' \| 'rule_violation' \| 'json_schema_violation' \| 'invalid_initial_state' \| 'invalid_transition'>` | ✅ | Which constraint the value violated (field-level catalog, ADR-0114) | | **message** | `string` | ✅ | Human-readable error message | | **value** | `any` | optional | The invalid value that was provided | | **constraint** | `any` | optional | The constraint that was violated (e.g., max length) | +--- + +## FieldErrorCode + +### Allowed Values + +* `required` +* `invalid_type` +* `invalid_shape` +* `unknown_field` +* `invalid_boolean` +* `invalid_number` +* `invalid_date` +* `invalid_time` +* `invalid_email` +* `invalid_url` +* `invalid_phone` +* `invalid_json` +* `invalid_format` +* `min_length` +* `max_length` +* `min_value` +* `max_value` +* `min_items` +* `max_items` +* `invalid_option` +* `invalid_value` +* `reference_not_found` +* `reference_ambiguous` +* `rule_violation` +* `json_schema_violation` +* `invalid_initial_state` +* `invalid_transition` + + --- ## RetryStrategy diff --git a/docs/adr/0114-field-level-error-code-catalog.md b/docs/adr/0114-field-level-error-code-catalog.md new file mode 100644 index 0000000000..a180237749 --- /dev/null +++ b/docs/adr/0114-field-level-error-code-catalog.md @@ -0,0 +1,108 @@ +# ADR-0114: Field-level error codes name the violated constraint — a closed lowercase catalog, and Zod stops leaking onto the wire + +**Status**: Accepted (2026-07-30) +**Deciders**: ObjectStack Protocol Architects +**Builds on**: [ADR-0112](./0112-error-code-vocabulary-and-ledger.md) (D6 deferred exactly this decision, and its catalog+ledger shape is the model reused here), [ADR-0049](./0049-no-unenforced-security-properties.md) (declare-and-enforce — the wire `fields[]` element currently has no schema at all), [ADR-0078](./0078-no-silently-inert-metadata.md) (no silently inert declarations — `EnhancedApiErrorSchema.fieldErrors` is declared and never emitted), [ADR-0104](./0104-field-runtime-value-shape-contract.md) (the silently-stripped-key line its contract guard enforces — why D4's rename is deferred rather than done here) +**Consumers**: `@objectstack/spec` (`api/errors.zod.ts`, `ui/action-params.zod.ts`), `@objectstack/objectql` (`validation/record-validator.ts`, `validation/rule-validator.ts`), `@objectstack/rest` (`import-coerce.ts`, `import-runner.ts`, `zodIssuesToFields`), `@objectstack/plugin-sharing` (`rule-criteria.ts`), `@objectstack/runtime` (`validation-failure.ts`), `@objectstack/client`, objectui (`react/src/utils/error-message.ts` — the only console consumer) +**Surfaced by**: [#3977](https://github.com/objectstack-ai/objectstack/issues/3977), split out of [#3841](https://github.com/objectstack-ai/objectstack/issues/3841) by ADR-0112 D6. + +--- + +## TL;DR + +A field-level code names **which constraint the value violated**, and constraints are already named in the metadata's own snake_case vocabulary — `required: true`, `max_length: 50`, `min_value: 0`. So the field vocabulary stays **lowercase snake_case**, deliberately *not* following ADR-0112's SCREAMING_SNAKE, because here the code is a constraint name and the correspondence with the schema property is the point. + +It becomes a **closed catalog** (`FieldErrorCode` in spec, 27 members) that `FieldErrorSchema.code` validates against, and Zod's issue codes get **mapped at the `zodIssuesToFields` boundary** instead of leaking library internals onto the wire. The `fields`-vs-`fieldErrors` naming split is decided (`fields` is right) but **not executed**: it is an authorable-key retirement, so ADR-0104's tombstone-plus-migration machinery applies and gets its own change. + +## Context + +ADR-0112 settled the top-level `error.code` vocabulary and explicitly refused to decide this one (D6), on the grounds that field-level codes have different emitters (validators) and different consumers (form UIs). `FieldErrorSchema.code` was widened to `z.string()` there so the spec would stop declaring an enum nothing complied with. This ADR is the deferred decision. + +### The vocabulary is more coherent than #3977's framing + +#3977 describes "four vocabularies with no schema." The harvest says: **six emitters, 24 distinct codes, and the overlaps are semantically consistent.** + +| Emitter | Codes | +|:---|:---| +| `record-validator.ts` | `required`, `invalid_type`, `invalid_boolean`, `invalid_date`, `invalid_time`, `invalid_email`, `invalid_url`, `invalid_phone`, `invalid_number`, `invalid_option`, `min_length`, `max_length`, `min_value`, `max_value` | +| `rule-validator.ts` | `required`, `invalid_option`, `invalid_format`, `invalid_json`, `json_schema_violation`, `invalid_initial_state`, `invalid_transition`, `rule_violation` | +| `import-coerce.ts` | `invalid_boolean`, `invalid_date`, `invalid_number`, `invalid_option`, `min_length`, `max_length`, `min_value`, `max_value`, `reference_not_found`, `reference_ambiguous` | +| `import-runner.ts`, `rule-criteria.ts` | `required` | +| `ui/action-params.zod.ts` | `required`, `unknown_param`, `invalid_shape` | + +`required` means the same thing in all five places that emit it; so do `max_length`, `min_value`, `invalid_option`. This is a de-facto standard that grew consistently, not four dialects that happen to share a wire position. What is missing is a **schema**, not a decision about which dialect wins. + +The genuine outlier is the fourth item in #3977's list: `StandardErrorCode`'s never-complied-with members (`VALUE_TOO_LONG` vs the emitted `max_length`, `MISSING_REQUIRED_FIELD` vs the emitted `required`). ADR-0112 resolved the immediate lie by widening `FieldErrorSchema.code`; this ADR leaves those members where they are. + +That leaves a **known wart, recorded rather than fixed**: the top-level catalog still carries six field-shaped members — `INVALID_FIELD`, `MISSING_REQUIRED_FIELD`, `INVALID_FORMAT`, `VALUE_TOO_LONG`, `VALUE_TOO_SHORT`, `VALUE_OUT_OF_RANGE`. As *top-level* codes they answer "why did this request fail" with "some value was too long", which is answerable but not useful — the useful version is `VALIDATION_ERROR` plus a `fields[]` entry naming the value and its constraint. They stay because removing a member from the top-level catalog is a breaking change to a vocabulary two batches just stabilised, and because `VALIDATION_ERROR` already covers the honest top-level answer. The consequence to live with: `invalid_format` exists at both levels (as `INVALID_FORMAT` above), which the catalog test admits as the single declared overlap rather than papering over. Retiring the six belongs to whoever next revisits the top-level catalog. + +### Nothing branches on the value + +#3977 asks to survey consumers before deciding casing, because "form UIs may already string-match." They do not: + +- **objectui** has exactly one field-error consumer, `extractFieldErrors` in `react/src/utils/error-message.ts`. It reads `field` and `message`, and touches `code` only as the last fallback in `firstString(rec.message, rec.error, rec.code)`. Its own comment says an untranslated enum in the UI reads as a bug. No branch, no match. +- **The framework** has no product-code branch on a field code at all — every match is in a test asserting an emitter's output. + +So the casing choice is unconstrained by migration cost, and can be made on principle. + +### Zod leaks its internals, ambiguously + +`zodIssuesToFields` (`rest-server.ts`) passes Zod's issue code straight through with `String(i?.code ?? 'invalid')`. Two consequences: + +1. The wire carries Zod's vocabulary (`too_small`, `too_big`, `unrecognized_keys`, `invalid_value`) on the same field position as the validators' — so a client cannot know which vocabulary it is reading without knowing which route served it. +2. `too_small` is **ambiguous on its own**: it covers a short string, a small number, and a short array. #3977 assumed this was a mapping problem with no clean answer. It has one — Zod v4 issues carry `origin` (`'string' | 'number' | 'array' | …`) and, for format failures, `format` (`'email' | 'url' | 'regex' | …`). Those disambiguate every case: + +``` +too_small + origin=string → min_length too_big + origin=string → max_length +too_small + origin=number → min_value too_big + origin=number → max_value +too_small + origin=array → min_items too_big + origin=array → max_items +invalid_format + format=email → invalid_email invalid_format + format=url → invalid_url +invalid_value → invalid_option unrecognized_keys → unknown_field +invalid_type (see below) → required | invalid_type +``` + +That last row is a real bug, not a tidy-up: Zod reports a missing required property as `invalid_type` (expected string, received undefined). Passed through verbatim, a form marks a *missing* input as a *type* error. + +It is also the one case `origin`/`format` cannot settle. A v4 issue carries `expected` and a message but **not the offending value**, so a missing property and a wrong-typed one are byte-identical on the issue — same `code`, same `expected`, same keys. The only other signal is the message text ("received undefined"), and depending on Zod's phrasing for a wire contract is precisely the leak this decision removes. So the discriminator is the **parsed input**, walked to the issue's `path`: the mapper takes it as an optional argument, and a caller that cannot supply it gets the accurate-but-less-specific `invalid_type` rather than a guess. + +### `fieldErrors` is declared and never emitted + +The wire carries `fields` — `runtime/src/validation-failure.ts`, all six emitters, `@objectstack/client`, and objectui's extractor all say `fields`. `EnhancedApiErrorSchema` declares `fieldErrors`, which nothing emits and nothing reads. That is ADR-0078's silently-inert declaration, on the error envelope. + +## Decision + +**D1 — Field-level codes stay lowercase `snake_case`, and this is not an exception to ADR-0112 but a consequence of what they name.** ADR-0112 D8 makes machine constants SCREAMING because a top-level code names a *condition the request hit* — an API-level fact, catalogued across services. A field-level code names the *constraint the value violated*, and constraints are declared in the metadata's own snake_case vocabulary: `required` ↔ `required: true`, `max_length` ↔ `max_length: 50`, `min_value` ↔ `min_value: 0`. The code and the schema property are the same word on purpose, and SCREAMING would break that correspondence to buy consistency with a vocabulary these codes are deliberately not part of (D6). Prime Directive #3's snake_case-for-data-values applies. + +**D2 — One closed catalog, `FieldErrorCode`, and `FieldErrorSchema.code` validates against it.** 27 members: the 24 harvested plus `min_items` / `max_items` / `unknown_field`, which the Zod mapping in D3 needs and which the validators will grow into. No ledger tier, unlike ADR-0112: field codes describe constraint kinds, which are a property of the *type system* and therefore closed by nature — a service does not get to invent one, it gets to add one to the catalog. `unknown_param` (action-params) folds into `unknown_field`; the param/field distinction lives in the surrounding record's key, not in the code. + +**D3 — Zod is mapped at the boundary, never passed through.** `zodIssuesToFields` translates using `origin` / `format` per the table above, plus the parsed input for the `invalid_type` split. An unmapped Zod code becomes `invalid_value` (a catalog member) rather than leaking. The mapping is tested by driving **real** `safeParse` calls, not by hand-written issue fixtures — which is how the `input` problem above surfaced at all: the first draft read `issue.input`, and a real parse showed that branch could never fire. + +**D4 — The wire's `fields` is the right name, and the rename is deferred with its cost written down.** `EnhancedApiErrorSchema.fieldErrors` should become `fields`; nothing has ever emitted `fieldErrors`, so the declaration points away from reality. But it is an **authorable key**, and the contract guard in `build-schemas.ts` (#3733, ADR-0104) rightly blocks a bare rename: these schemas are not `.strict()`, so Zod silently strips an unknown key, and an author or producer still writing the old name would get a clean parse and a value that never takes effect. Retiring it properly needs a `retiredKey()` tombstone carrying the fix, a D2 conversion (and D3 chain step) so `os migrate meta` can rewrite consumers, and a major changeset with the FROM → TO mapping. + +That is a change of its own, not a rider on this one. What lands here is the half that makes a validation body **assertable** — the element schema, with `code` closed — and the property keeps its name plus a banner naming the mismatch and the machinery its retirement needs. Declaring the wrong name loudly beats renaming it quietly. + +**D5 — What this catalog does NOT govern, restated from ADR-0112.** The three neighbouring vocabularies stay put: persisted columns (D6b), diagnostics inside a 200 (D6c), and the top-level catalog. The line from ADR-0112 holds — *the field catalog governs the code that names which constraint a value violated*. `check-error-code-casing.mjs` already recognises the field-addressed shape structurally, so this catalog needs no new guard exemptions; what it needs, and now gets, is a schema. + +## Alternatives rejected + +**SCREAMING_SNAKE for consistency with ADR-0112.** Defensible on "one convention for machine constants," and free of migration cost since nothing branches. Rejected because it severs the code-to-constraint-name correspondence that makes `max_length` self-documenting against `max_length: 50`, and because ADR-0112 D6 already ruled that this is a different vocabulary — inheriting its casing rule would quietly re-merge what D6 separated. Consistency with the *metadata* vocabulary is the more load-bearing consistency here. + +**Keep `z.string()` and document the convention.** Cheapest, and no worse than today. Rejected as exactly the state ADR-0112 D4 refused for the top level: an undeclared vocabulary reopens the moment someone types a new code, and the conformance suites have nothing to assert. The Zod passthrough proves the failure mode is live, not hypothetical. + +**Map Zod codes into the catalog *and* keep them as ledger-style extensions.** Rejected: it makes the wire carry two vocabularies again, which is the whole problem. Zod is an implementation detail of one route's parsing, and an implementation detail has no business being a wire contract. + +**Rename the wire's `fields` to the declared `fieldErrors`.** Rejected on ADR-0112's own reasoning: the wire is the harder thing to move, and here it is also the *more used* thing — six emitters, the client, and the console versus zero emitters of `fieldErrors`. + +## Consequences + +- `FieldErrorSchema.code` tightens from `z.string()` to `FieldErrorCode`, and ADR-0112's banner comment comes off. The conformance suites that parse error bodies gain a value check for free. +- The wire `fields[]` element has a schema for the first time, so a validation response can be asserted structurally rather than by duck-typing. +- Zod-served routes change their field codes (`too_small` → `min_length`, and a missing property stops reporting as a type error). This is a wire-visible fix; nothing in-repo or in the console branches on the old values. +- `EnhancedApiErrorSchema.fieldErrors` keeps a name the wire does not use, now with a banner saying so. That is a deliberate debt: loud and documented beats a quiet rename that strips an author's key. +- Adding a constraint kind now means adding a catalog member, which is the intended friction: a new *kind* of constraint is a type-system change and deserves a line in the spec. + +## Rollout + +One PR — the emitters already speak the chosen vocabulary, so the change is the catalog, the schema tightening, and the Zod mapping. No sweep, no batches, no consumer migration. + +Deferred to its own change (D4): retiring `fieldErrors` for `fields`, which needs the ADR-0104 sequence — tombstone, D2 conversion + D3 chain step, major changeset. diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index f63ae2f81d..4bd42217e3 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -38,6 +38,7 @@ import { FILE_REFERENCE_TYPES, STRUCTURED_JSON_TYPES, } from '@objectstack/spec/data'; +import type { FieldErrorCode } from '@objectstack/spec/api'; // Lifecycle columns the engine always owns and the client never supplies. These // are skipped by NAME because they are not author-declared business fields. @@ -76,28 +77,17 @@ const PHONE_RE = /^[+()\-\s\d.]{5,}$/; export interface FieldValidationError { field: string; - code: - | 'required' - | 'min_length' - | 'max_length' - | 'min_value' - | 'max_value' - | 'invalid_email' - | 'invalid_url' - | 'invalid_phone' - | 'invalid_number' - | 'invalid_boolean' - | 'invalid_date' - | 'invalid_time' - | 'invalid_option' - | 'invalid_type' - // Object-level validation rules (ADR-0020, see rule-validator.ts) - | 'invalid_transition' - | 'invalid_initial_state' - | 'rule_violation' - | 'invalid_format' - | 'invalid_json' - | 'json_schema_violation'; + /** + * Which constraint the value violated — the spec's field-level catalog + * (ADR-0114), not a union maintained here. + * + * This was a hand-listed literal union, which is the shape that drifts + * silently: adding a validator case meant remembering to widen it, and a + * consumer's `switch` over it went non-exhaustive in a package the change never + * touched. The catalog is the single list, and `FieldErrorSchema.code` validates + * against it on the way out. + */ + code: FieldErrorCode; message: string; /** Allowed values for select/multiselect, when applicable. */ options?: string[]; diff --git a/packages/rest/src/import-coerce.ts b/packages/rest/src/import-coerce.ts index cecc83c972..4839b6a27e 100644 --- a/packages/rest/src/import-coerce.ts +++ b/packages/rest/src/import-coerce.ts @@ -41,6 +41,7 @@ import { REFERENCE_VALUE_TYPES, isMultiValueField as specIsMultiValueField, } from '@objectstack/spec/data'; +import type { FieldErrorCode } from '@objectstack/spec/api'; /** * Field types whose stored value points at another record (id). The spec's @@ -112,7 +113,13 @@ export interface CoerceContext { /** A per-field coercion failure, shaped like the engine's validation errors. */ export interface FieldCoerceError { field: string; - code: string; + /** + * Which constraint the value violated — the spec's field-level catalog + * (ADR-0114). Was a bare `string`, so a typo here reached the wire and the + * "shaped like the engine's validation errors" claim above was a comment rather + * than a type. + */ + code: FieldErrorCode; message: string; } diff --git a/packages/rest/src/rest-bulk-path-object.test.ts b/packages/rest/src/rest-bulk-path-object.test.ts index ae5d128d79..e85690199a 100644 --- a/packages/rest/src/rest-bulk-path-object.test.ts +++ b/packages/rest/src/rest-bulk-path-object.test.ts @@ -134,8 +134,14 @@ describe('updateMany ingress validation (#3933)', () => { expect(res.body.code).toBe('VALIDATION_FAILED'); // The documented data-surface envelope (`fields[]`, wire-format §7), not a // second per-route shape. + // + // `required`, not `invalid_type` (ADR-0114 D3): `id` is ABSENT here, and Zod + // spells absent as a type mismatch against `undefined`. This assertion used to + // pin that passthrough, so it pinned a form marking a MISSING input as the + // wrong TYPE — the message below still says "received undefined", which is + // what the old code was reporting as a type error. expect(res.body.fields).toEqual([ - { field: 'records.0.id', code: 'invalid_type', message: expect.any(String) }, + { field: 'records.0.id', code: 'required', message: expect.any(String) }, ]); expect(updateManyData).not.toHaveBeenCalled(); }); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 033f96cbc9..cd784c0229 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -9,6 +9,7 @@ import { allowPerfDisclosure, isPerfDisclosurePrincipal } from '@objectstack/obs import { RouteManager } from './route-manager.js'; import { RestServerConfig, RestApiConfig, CrudEndpointsConfig, MetadataEndpointsConfig, BatchEndpointsConfig, RouteGenerationConfig } from '@objectstack/spec/api'; import { DataProtocol, MetadataProtocol } from '@objectstack/spec/api'; +import type { FieldErrorCode } from '@objectstack/spec/api'; import { PUBLIC_FORM_SERVER_MANAGED_FIELDS } from '@objectstack/spec/security'; import { PLURAL_TO_SINGULAR } from '@objectstack/spec/shared'; import type { DroppedFieldsEvent } from '@objectstack/spec/data'; @@ -77,6 +78,78 @@ const TRANSLATABLE_META_TYPES = new Set(['view', 'action', 'object', 'app', 'das * not permitted …" — trips the `'' … not` substring check and * returns a misleading 404. */ +/** + * A Zod issue → the field-level catalog (ADR-0114 D3). + * + * Zod's issue codes are Zod's API, not ours, and this used to pass them straight + * through. Two things were wrong with that. The wire carried two vocabularies on + * one position — `too_small` from a route that parses with Zod, `min_length` from + * the validators — so a client could not read a field code without knowing which + * route served it. And Zod's own codes are ambiguous alone: `too_small` covers a + * short string, a small number AND a short array. + * + * `origin` and `format` disambiguate every case, so the mapping is total rather + * than best-effort. The one row that fixes a user-visible bug rather than tidying + * a name: Zod reports a MISSING required property as `invalid_type` (expected + * string, received undefined), so passing it through marked a missing input as a + * type error. + */ +function zodIssueToFieldCode(issue: any, input?: unknown, inputProvided = false): FieldErrorCode { + const origin = issue?.origin; + switch (issue?.code) { + case 'too_small': + return origin === 'number' || origin === 'bigint' || origin === 'date' ? 'min_value' + : origin === 'array' || origin === 'set' ? 'min_items' + : 'min_length'; + case 'too_big': + return origin === 'number' || origin === 'bigint' || origin === 'date' ? 'max_value' + : origin === 'array' || origin === 'set' ? 'max_items' + : 'max_length'; + case 'invalid_format': + return issue?.format === 'email' ? 'invalid_email' + : issue?.format === 'url' ? 'invalid_url' + : 'invalid_format'; + case 'invalid_type': { + // Zod spells "absent" as a type mismatch against `undefined`, so a + // MISSING required property arrives here rather than as its own code. + // The issue itself cannot tell the two apart — v4 carries `expected` + // and a message but not the offending value — so the only honest + // discriminator is the parsed input, walked to `path`. Without it we + // keep `invalid_type`: reading "received undefined" out of the message + // would make the wire contract depend on Zod's phrasing, which is the + // leak this mapping exists to stop. + if (!inputProvided) return 'invalid_type'; + return valueAtPath(input, issue?.path) === undefined ? 'required' : 'invalid_type'; + } + case 'invalid_value': + // A closed set (`z.enum`, `z.literal`) the value is not a member of. + return 'invalid_option'; + case 'unrecognized_keys': + return 'unknown_field'; + case 'invalid_union': + case 'invalid_element': + case 'invalid_key': + return 'invalid_shape'; + case 'not_multiple_of': + case 'custom': + default: + // A catalog member, not a leak: an unmapped Zod code still lands on a + // code the client can read, and `message` carries the specifics. + return 'invalid_value'; + } +} + +/** Walk a Zod issue `path` into the value that was parsed. */ +function valueAtPath(input: unknown, path: unknown): unknown { + if (!Array.isArray(path)) return undefined; + let cur: any = input; + for (const seg of path) { + if (cur === null || cur === undefined) return undefined; + cur = cur[seg as any]; + } + return cur; +} + /** * Zod issues → the data surface's `fields[]` validation envelope * (`{ field, code, message }`, docs/api/wire-format §7). @@ -85,13 +158,18 @@ const TRANSLATABLE_META_TYPES = new Set(['view', 'action', 'object', 'app', 'das * a validator-thrown `VALIDATION_FAILED` does through {@link mapDataError} * (#3918) — otherwise a client keying on `fields` has to learn a second shape * per route, and `code: 'VALIDATION_FAILED'` stops meaning one thing on the - * wire. + * wire. Since ADR-0114 that sameness covers the `code` VALUE too, not just the + * shape: see {@link zodIssueToFieldCode}. */ -export function zodIssuesToFields(issues: unknown): Array<{ field: string; code: string; message: string }> { +export function zodIssuesToFields( + issues: unknown, + ...input: [] | [unknown] +): Array<{ field: string; code: FieldErrorCode; message: string }> { if (!Array.isArray(issues)) return []; + const inputProvided = input.length > 0; return issues.map((i: any) => ({ field: Array.isArray(i?.path) ? i.path.join('.') : String(i?.path ?? ''), - code: String(i?.code ?? 'invalid'), + code: zodIssueToFieldCode(i, input[0], inputProvided), message: String(i?.message ?? 'Invalid value'), })); } @@ -7003,15 +7081,13 @@ export class RestServer { // becoming the execution context on a deployment where none // resolves (anonymous-reachable `requireAuth: false`). const { UpdateManyDataRequestSchema } = await import('@objectstack/spec/api'); - const parsedUpdate = (UpdateManyDataRequestSchema as any).safeParse({ - ...(req.body ?? {}), - object: req.params.object, - }); + const updateManyInput = { ...(req.body ?? {}), object: req.params.object }; + const parsedUpdate = (UpdateManyDataRequestSchema as any).safeParse(updateManyInput); if (!parsedUpdate.success) { res.status(400).json({ error: 'Invalid updateMany request', code: 'VALIDATION_FAILED', - fields: zodIssuesToFields(parsedUpdate.error?.issues), + fields: zodIssuesToFields(parsedUpdate.error?.issues, updateManyInput), object: req.params?.object, }); return; @@ -7067,15 +7143,13 @@ export class RestServer { // so a body `object` would move the delete to an object // whose exposure policy was never checked. const { DeleteManyDataRequestSchema } = await import('@objectstack/spec/api'); - const parsed = (DeleteManyDataRequestSchema as any).safeParse({ - ...(req.body ?? {}), - object: req.params.object, - }); + const deleteManyInput = { ...(req.body ?? {}), object: req.params.object }; + const parsed = (DeleteManyDataRequestSchema as any).safeParse(deleteManyInput); if (!parsed.success) { res.status(400).json({ error: 'Invalid deleteMany request', code: 'VALIDATION_FAILED', - fields: zodIssuesToFields(parsed.error?.issues), + fields: zodIssuesToFields(parsed.error?.issues, deleteManyInput), object: req.params?.object, }); return; diff --git a/packages/rest/src/zod-field-codes.test.ts b/packages/rest/src/zod-field-codes.test.ts new file mode 100644 index 0000000000..0941de3e5f --- /dev/null +++ b/packages/rest/src/zod-field-codes.test.ts @@ -0,0 +1,126 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { z } from 'zod'; +import { FieldErrorCode } from '@objectstack/spec/api'; +import { zodIssuesToFields } from './rest-server'; + +/** + * Zod issue → field-level catalog (ADR-0114 D3). + * + * Every case here drives a REAL `safeParse` rather than hand-writing an issue + * object. That is the point of the file: the mapping reads `origin` and `format`, + * which are Zod's internals, so a test built on an ASSUMED issue shape would keep + * passing after a Zod upgrade quietly changed one — the wire would fall back to + * `invalid_value` for everything while every assertion stayed green. + * + * Writing it that way is also how the `input` handling got found: the first draft + * of the mapping read `issue.input` to tell a missing property from a wrong-typed + * one, and a real parse showed v4 issues do not carry it — so that branch could + * never have fired. + */ +describe('zodIssuesToFields', () => { + /** + * Parse `value` against `schema` and return the mapped field entries, passing + * the parsed input the way the routes do — required-vs-wrong-type needs it, + * because a Zod v4 issue does not carry the offending value. + */ + const fieldsFor = (schema: z.ZodType, value: unknown) => { + const r = schema.safeParse(value); + expect(r.success, 'the fixture must actually fail to parse').toBe(false); + return zodIssuesToFields((r as { error: { issues: unknown[] } }).error.issues, value); + }; + + /** The same, without the input — the conservative path. */ + const fieldsForBlind = (schema: z.ZodType, value: unknown) => { + const r = schema.safeParse(value); + return zodIssuesToFields((r as { error: { issues: unknown[] } }).error.issues); + }; + + it('disambiguates too_small by what was too small', () => { + // The ambiguity #3977 flagged as having no clean answer: one Zod code, three + // meanings. `origin` carries the answer. + expect(fieldsFor(z.object({ a: z.string().min(3) }), { a: 'x' })[0].code).toBe('min_length'); + expect(fieldsFor(z.object({ a: z.number().min(5) }), { a: 1 })[0].code).toBe('min_value'); + expect(fieldsFor(z.object({ a: z.array(z.string()).min(2) }), { a: ['one'] })[0].code).toBe('min_items'); + }); + + it('disambiguates too_big the same way', () => { + expect(fieldsFor(z.object({ a: z.string().max(2) }), { a: 'toolong' })[0].code).toBe('max_length'); + expect(fieldsFor(z.object({ a: z.number().max(1) }), { a: 9 })[0].code).toBe('max_value'); + expect(fieldsFor(z.object({ a: z.array(z.string()).max(1) }), { a: ['a', 'b'] })[0].code).toBe('max_items'); + }); + + it('maps a missing property to required, not to a type error', () => { + // The user-visible bug, not a tidy-up: Zod reports an absent required + // property as `invalid_type` (expected string, received undefined). Passed + // through, a form marked a MISSING input as the wrong TYPE. + expect(fieldsFor(z.object({ a: z.string() }), {})[0].code).toBe('required'); + }); + + it('still reports a genuine type mismatch as invalid_type', () => { + expect(fieldsFor(z.object({ a: z.string() }), { a: 42 })[0].code).toBe('invalid_type'); + }); + + it('keeps invalid_type when the input is not supplied, rather than guessing', () => { + // Missing and wrong-type are INDISTINGUISHABLE on the issue alone — v4 gives + // both the same `code`, the same `expected`, and no value. The only other + // signal is the message text ("received undefined"), and depending on Zod's + // phrasing for a wire contract is the leak this mapping removes. So a caller + // that cannot supply the input gets the accurate-but-less-specific code. + expect(fieldsForBlind(z.object({ a: z.string() }), {})[0].code).toBe('invalid_type'); + expect(fieldsForBlind(z.object({ a: z.string() }), { a: 42 })[0].code).toBe('invalid_type'); + }); + + it('maps declared formats to their own members', () => { + expect(fieldsFor(z.object({ a: z.string().email() }), { a: 'no' })[0].code).toBe('invalid_email'); + expect(fieldsFor(z.object({ a: z.string().url() }), { a: 'no' })[0].code).toBe('invalid_url'); + // A pattern with no dedicated member falls back to the generic format code. + expect(fieldsFor(z.object({ a: z.string().regex(/^\d+$/) }), { a: 'abc' })[0].code).toBe('invalid_format'); + }); + + it('maps a closed set to invalid_option', () => { + expect(fieldsFor(z.object({ a: z.enum(['x', 'y']) }), { a: 'z' })[0].code).toBe('invalid_option'); + }); + + it('maps an unexpected key to unknown_field', () => { + expect(fieldsFor(z.object({ a: z.string() }).strict(), { a: 'ok', extra: 1 })[0].code).toBe('unknown_field'); + }); + + it('never emits a code outside the catalog, whatever Zod produced', () => { + // The invariant that matters most: an unmapped Zod code must land on a member + // rather than leak. Exercised across a deliberately varied schema so a Zod + // upgrade that introduces a NEW issue code fails here rather than on the wire. + const schema = z.object({ + s: z.string().min(2).max(4), + n: z.number().int().positive().multipleOf(3), + e: z.enum(['a', 'b']), + arr: z.array(z.number()).min(1), + nested: z.object({ deep: z.string().email() }), + u: z.union([z.string(), z.number()]), + d: z.string().refine(() => false, 'always fails'), + }).strict(); + const fields = fieldsFor(schema, { + s: 'toolongvalue', n: -2.5, e: 'nope', arr: [], nested: { deep: 'bad' }, + u: { neither: true }, d: 'x', surprise: 1, + }); + expect(fields.length).toBeGreaterThan(5); + for (const f of fields) { + expect(() => FieldErrorCode.parse(f.code), `'${f.code}' is not a catalog member`).not.toThrow(); + } + }); + + it('keeps the dotted path and the server message', () => { + const [entry] = fieldsFor(z.object({ nested: z.object({ deep: z.string() }) }), { nested: { deep: 1 } }); + expect(entry.field).toBe('nested.deep'); + // `message` is what a UI shows verbatim, so the mapping must not touch it. + expect(entry.message).toBeTruthy(); + expect(entry.message).not.toBe(entry.code); + }); + + it('tolerates a non-array argument', () => { + for (const junk of [null, undefined, {}, 'issues', 0]) { + expect(zodIssuesToFields(junk)).toEqual([]); + } + }); +}); diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 10b9a430cf..b59a466918 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -2509,6 +2509,7 @@ "ExportRequest (type)", "ExportRequestSchema (const)", "FieldError (type)", + "FieldErrorCode (type)", "FieldErrorSchema (const)", "FieldMappingEntry (type)", "FieldMappingEntrySchema (const)", diff --git a/packages/spec/json-schema.manifest.json b/packages/spec/json-schema.manifest.json index 736579f773..1faebb7fad 100644 --- a/packages/spec/json-schema.manifest.json +++ b/packages/spec/json-schema.manifest.json @@ -220,6 +220,7 @@ "api/ExportJobSummary", "api/ExportRequest", "api/FieldError", + "api/FieldErrorCode", "api/FieldMappingEntry", "api/FileDownloadUrlResponse", "api/FileTypeValidation", diff --git a/packages/spec/src/api/errors.test.ts b/packages/spec/src/api/errors.test.ts index bf8f3f6118..b15a2c2bfe 100644 --- a/packages/spec/src/api/errors.test.ts +++ b/packages/spec/src/api/errors.test.ts @@ -4,6 +4,7 @@ import { StandardErrorCode, RetryStrategy, FieldErrorSchema, + FieldErrorCode, EnhancedApiErrorSchema, ErrorResponseSchema, ErrorHttpStatusMap, @@ -57,8 +58,8 @@ describe('RetryStrategy', () => { }); describe('FieldErrorSchema', () => { - // Field-level codes are the validators' own lowercase vocabulary (ADR-0112 - // D6, #3977) — these literals mirror what record-validator actually emits. + // Field-level codes are the validators' own lowercase vocabulary, catalogued + // by ADR-0114 — these literals mirror what record-validator actually emits. it('should accept basic field error', () => { const error = FieldErrorSchema.parse({ field: 'email', @@ -115,10 +116,14 @@ describe('EnhancedApiErrorSchema', () => { retryable: false, retryStrategy: 'no_retry', details: { count: 2 }, + // Still `fieldErrors` — the wire says `fields`, but retiring an authorable + // key needs ADR-0104's tombstone machinery, so ADR-0114 D4 defers it. The + // code below IS fixed: the field-level catalog's lowercase `invalid_email`, + // where this block used to assert a top-level SCREAMING member. fieldErrors: [ { field: 'email', - code: 'INVALID_FORMAT', + code: 'invalid_email', message: 'Invalid email format', }, ], @@ -272,3 +277,60 @@ describe('HttpStatusErrorCodeMap / standardErrorCodeForHttpStatus (#3842)', () = expect(standardErrorCodeForHttpStatus(ErrorHttpStatusMap['not_found'])).toBe('RESOURCE_NOT_FOUND'); }); }); + +/** + * The field-level catalog (ADR-0114). What is load-bearing here is not that the + * members parse — it is the two invariants that keep this vocabulary from drifting + * back into the top-level one, and the correspondence that justifies its casing. + */ +describe('FieldErrorCode', () => { + const members = FieldErrorCode.options; + + it('is lowercase snake_case throughout — the opposite of StandardErrorCode', () => { + // ADR-0114 D1: these name a violated CONSTRAINT, and constraints are declared + // in the metadata's own snake_case. A SCREAMING member here means someone + // reached for the top-level catalog's convention by reflex. + for (const m of members) { + expect(m, `${m} must be lowercase snake_case`).toMatch(/^[a-z][a-z0-9_]*$/); + } + }); + + it('names the constraint property it reports on', () => { + // The whole argument for D1's casing: the code IS the schema property name. + // If these ever diverge, the field vocabulary has lost its reason to be + // lowercase and the decision should be revisited rather than patched. + for (const constraint of ['required', 'max_length', 'min_length', 'max_value', 'min_value'] as const) { + expect(members).toContain(constraint); + } + }); + + it('overlaps the top-level catalog only where the overlap is declared', () => { + // Two vocabularies on two structural levels (ADR-0112 D6). A name in both is + // not automatically wrong — the top-level code says why the REQUEST failed, + // the field-level one says which value and which constraint — but it must be + // deliberate. An unlisted overlap means someone added a field member by + // copying a top-level one, which is the reflex the casing test above guards + // from the other side. + // + // `invalid_format` is the only case, and it exists because the top-level + // catalog still carries field-shaped members it inherited (`INVALID_FORMAT`, + // `VALUE_TOO_LONG`, `MISSING_REQUIRED_FIELD`, …) — see ADR-0114's note on + // them. Their field-level counterparts mostly have better names here + // (`max_length` over `VALUE_TOO_LONG`); this one happens to coincide. + const DECLARED_OVERLAPS = new Set(['invalid_format']); + const top = new Set(StandardErrorCode.options.map((c) => c.toLowerCase())); + for (const m of members) { + if (DECLARED_OVERLAPS.has(m)) continue; + expect(top.has(m), `${m} collides with a StandardErrorCode member`).toBe(false); + } + }); + + it('rejects a code from the vocabulary it replaced', () => { + // The pre-ADR-0114 leak: Zod's own issue codes reaching the wire. + for (const zodCode of ['too_small', 'too_big', 'unrecognized_keys', 'invalid_union']) { + expect(() => FieldErrorCode.parse(zodCode)).toThrow(); + } + // …and a top-level code, which is what the schema used to declare. + expect(() => FieldErrorCode.parse('VALIDATION_ERROR')).toThrow(); + }); +}); diff --git a/packages/spec/src/api/errors.zod.ts b/packages/spec/src/api/errors.zod.ts index d40133f8ef..234f37b487 100644 --- a/packages/spec/src/api/errors.zod.ts +++ b/packages/spec/src/api/errors.zod.ts @@ -208,19 +208,80 @@ export const RetryStrategy = z.enum([ export type RetryStrategy = z.infer; +/** + * Which constraint a single value violated (ADR-0114 D2). + * + * A **closed** catalog, and deliberately lowercase `snake_case` where the + * top-level `StandardErrorCode` is SCREAMING (ADR-0114 D1): a top-level code + * names a condition the *request* hit, while these name the *constraint* — and + * constraints are already declared in the metadata's own vocabulary, so the code + * and the schema property are the same word on purpose: + * + * { required: true } → code 'required' + * { max_length: 50 } → code 'max_length' + * { min_value: 0 } → code 'min_value' + * + * Closed with no ledger tier, unlike the top-level catalog: a constraint *kind* + * is a property of the type system, so a service does not get to invent one — it + * adds a member here. That friction is the point. + * + * The `field` key of the surrounding record says what was addressed (a column, a + * dotted path, an action param), so there is no per-surface variant of a code: + * an unknown action param and an unknown column are both `unknown_field`. + */ +export const FieldErrorCode = z.enum([ + // presence and shape + 'required', // no value where one is mandatory + 'invalid_type', // a value of the wrong primitive type + 'invalid_shape', // an object/array whose structure does not fit + 'unknown_field', // a key the target does not declare + // per-type parse failures + 'invalid_boolean', + 'invalid_number', + 'invalid_date', + 'invalid_time', + 'invalid_email', + 'invalid_url', + 'invalid_phone', + 'invalid_json', + 'invalid_format', // a declared pattern/format the value does not match + // bounded ranges — the property names they mirror + 'min_length', + 'max_length', + 'min_value', + 'max_value', + 'min_items', + 'max_items', + // closed sets and references + 'invalid_option', // not a member of the field's declared options + 'invalid_value', // rejected for a reason no other member names + 'reference_not_found', // a lookup target that does not exist + 'reference_ambiguous', // a lookup that matched more than one record + // declarative rules layered above the field's own type + 'rule_violation', // a validation rule said no + 'json_schema_violation', // a declared JSON Schema said no + 'invalid_initial_state', // state machine: not a legal starting state + 'invalid_transition', // state machine: not a legal move from here +]); + +export type FieldErrorCode = z.infer; + /** * Field Error Schema * Detailed error for a specific field */ export const FieldErrorSchema = lazySchema(() => z.object({ field: z.string().describe('Field path (supports dot notation)'), - // ⚠️ DELIBERATELY WIDE (ADR-0112 D6, #3977): field-level codes are a separate - // vocabulary from top-level `error.code` and were never `StandardErrorCode` in - // practice — the validators emit `required` / `max_length` / `invalid_email` / - // …, import coercion adds its own, and one route leaks raw Zod issue codes. - // Declaring the enum here was a lie the wire never honoured. #3977 owns the - // real field-level catalog; when it lands, this tightens to that enum. - code: z.string().describe('Error code for this field (field-level vocabulary — see #3977)'), + /** + * Which constraint the value violated — a `FieldErrorCode` (ADR-0114 D2). + * + * Closed on purpose. This was `z.string()` between ADR-0112 (which widened it, + * because declaring `StandardErrorCode` here was a lie the wire never + * honoured) and ADR-0114 (which gave the field level its own catalog). One + * route used to leak raw Zod issue codes through this position; they are now + * mapped at the boundary (`zodIssuesToFields`, ADR-0114 D3). + */ + code: FieldErrorCode.describe('Which constraint the value violated (field-level catalog, ADR-0114)'), message: z.string().describe('Human-readable error message'), value: z.unknown().optional().describe('The invalid value that was provided'), constraint: z.unknown().optional().describe('The constraint that was violated (e.g., max length)'), @@ -287,7 +348,21 @@ export const EnhancedApiErrorSchema = lazySchema(() => z.object({ retryStrategy: RetryStrategy.optional().describe('Recommended retry strategy'), retryAfter: z.number().optional().describe('Seconds to wait before retrying'), details: z.unknown().optional().describe('Additional error context'), - fieldErrors: z.array(FieldErrorSchema).optional().describe('Field-specific validation errors'), + /** + * One entry per offending value. + * + * ⚠️ NAME MISMATCH, deliberately left standing (ADR-0114 D4). The wire carries + * `fields` — the validators, import coercion, `validation-failure.ts`, + * `@objectstack/client` and the console's extractor all say `fields`, and + * nothing has ever emitted `fieldErrors`. Renaming it here is the right end + * state, but it is an authorable-key retirement: ADR-0104's contract guard + * requires a `retiredKey()` tombstone, a D2 conversion so `os migrate meta` can + * rewrite consumers, and a major changeset carrying the FROM → TO mapping. + * That machinery is a change of its own, not a rider on this one — ADR-0114 + * lands the ELEMENT schema (which is what makes a validation body assertable) + * and defers the rename with its cost written down. + */ + fieldErrors: z.array(FieldErrorSchema).optional().describe('Field-specific validation errors (wire name is `fields` — see ADR-0114 D4)'), timestamp: z.string().datetime().optional().describe('When the error occurred'), requestId: z.string().optional().describe('Request ID for tracking'), traceId: z.string().optional().describe('Distributed trace ID'), diff --git a/packages/spec/src/ui/action-params.test.ts b/packages/spec/src/ui/action-params.test.ts index 6cbffce8d0..1fd6b25477 100644 --- a/packages/spec/src/ui/action-params.test.ts +++ b/packages/spec/src/ui/action-params.test.ts @@ -57,7 +57,9 @@ describe('validateActionParams (ADR-0104 D2)', () => { it('flags unknown params, but allows the dispatcher built-in keys', () => { const resolved: ResolvedActionParam[] = [{ name: 'title', type: 'text' }]; const issues = validateActionParams(resolved, { title: 'x', bogus: 1, recordId: 'r1', objectName: 'o' }); - expect(codes(issues)).toEqual(['unknown_param']); + // `unknown_param` folded into the catalog's `unknown_field` (ADR-0114 D2) — + // the `param` key beside it already says what was addressed. + expect(codes(issues)).toEqual(['unknown_field']); expect(issues[0].param).toBe('bogus'); expect(ACTION_PARAM_BUILTIN_KEYS).toContain('recordId'); expect(ACTION_PARAM_BUILTIN_KEYS).toContain('objectName'); diff --git a/packages/spec/src/ui/action-params.zod.ts b/packages/spec/src/ui/action-params.zod.ts index 89d16622b3..da17cad3a2 100644 --- a/packages/spec/src/ui/action-params.zod.ts +++ b/packages/spec/src/ui/action-params.zod.ts @@ -18,6 +18,7 @@ */ import { valueSchemaFor } from '../data/field-value.zod'; +import type { FieldErrorCode } from '../api/errors.zod'; /** * A declared action param resolved to its effective value-shape inputs. A @@ -41,7 +42,17 @@ export interface ResolvedActionParam { export interface ActionParamIssue { /** The offending param key. */ param: string; - code: 'required' | 'invalid_shape' | 'unknown_param'; + /** + * Which constraint the value violated, from the field-level catalog + * (ADR-0114). Typed as `FieldErrorCode` rather than a local literal union so + * an action param and a record field cannot drift into two vocabularies for + * the same three conditions — `required` and `invalid_shape` were already + * shared verbatim before the catalog existed. + * + * `unknown_param` folded into `unknown_field` (ADR-0114 D2): the `param` key + * beside it already says what was addressed, so the code did not need to. + */ + code: FieldErrorCode; message: string; } @@ -101,7 +112,7 @@ export function validateActionParams( if (declared.has(key) || allow.has(key)) continue; issues.push({ param: key, - code: 'unknown_param', + code: 'unknown_field', message: `Unknown action param "${key}" — not declared on this action`, }); } diff --git a/scripts/check-error-code-casing.mjs b/scripts/check-error-code-casing.mjs index c2898e7487..fde29695bd 100644 --- a/scripts/check-error-code-casing.mjs +++ b/scripts/check-error-code-casing.mjs @@ -36,7 +36,9 @@ * one of those families still has to say which family it joins: * * - **D6 — field/param-addressed** (`{ field, code }`, `{ param, code }`): - * validator vocabularies, #3977's scope. + * the field-level catalog, which ADR-0114 closed and made lowercase on purpose + * (a field code names the violated CONSTRAINT, and constraints are declared in + * the metadata's own snake_case). * - **D6b — persisted**: `sys_metadata_audit.code` is audit history; old rows * keep their spelling forever and the column also holds `ok`. * - **D6c — diagnostics**: probe/diff records that ship as payload of a 200. @@ -61,12 +63,12 @@ const SCAN_ROOTS = ['packages']; */ const EXEMPT_FILES = new Map([ // D6 — field-addressed validator vocabularies (#3977) - ['packages/objectql/src/validation/record-validator.ts', 'D6 field-level validator codes'], + ['packages/objectql/src/validation/record-validator.ts', 'D6/ADR-0114 field-level catalog codes'], ['packages/objectql/src/validation/rule-validator.ts', 'D6 field-level validator codes'], ['packages/rest/src/import-coerce.ts', 'D6 field-level import coercion codes'], ['packages/rest/src/import-runner.ts', 'D6 field-level import row codes'], ['packages/plugins/plugin-sharing/src/rule-criteria.ts', 'D6 field-level; top-level code is VALIDATION_FAILED'], - ['packages/spec/src/ui/action-params.zod.ts', 'D6 param-addressed action-param issues'], + ['packages/spec/src/ui/action-params.zod.ts', 'D6/ADR-0114 param-addressed issues'], // D6b — persisted audit column ['packages/metadata-core/src/objects/sys-metadata-audit.object.ts', 'D6b persisted audit vocabulary'], ['packages/spec/src/api/errors.test.ts', 'D6 FieldError tests spell field-level codes'],