Skip to content

Commit 3b8e2b1

Browse files
fix(runtime): two hand-built refusals answer FieldErrorCode members (unknown_field, invalid_format), not Zod issue codes (#24683)
Fixes #24597 Clause-②: no Basis for the Clause-② line: the two refusals' field codes move onto members the published `FieldErrorCode` catalog already declares (`unknown_field`, `invalid_format`). No accept set moves, no status or `error.code` moves, and no catalog member is added; `packages/spec` is not touched. ADR-0114 D2 declares `FieldErrorCode` (`packages/spec/src/api/errors.zod.ts`) a **closed** catalog: "a service does not get to invent one — it adds a member here". Two refusals in `packages/runtime` build their `details.fields[]` rows by hand and wrote Zod's issue codes there. `validationFailure(message, fields)` types `fields` as `unknown[]` and `ApiErrorSchema` types `details` loosely, so neither tsc nor the envelope schema saw it. This PR moves both rows onto catalog members, takes a census of every hand-built `details.fields[]` writer in `packages/runtime`, and pins both sites plus a closed-set check over the domain's hand-built refusals. ## Before and after, per site Measured through the real `HttpDispatcher` handler (`handleAutomation` / `handleSecurity`), on `origin/main` `bfc1539fe` (before) and this branch (after). A field-by-field diff of the two envelopes shows exactly one path changing per site, `body.error.details.fields[0].code`. Status, `error.code`, `error.message`, `httpStatus` and the row's `field` and `message` are byte-identical. **1. Run-lifecycle body refusal** — `packages/runtime/src/domains/automation.ts` `refuseInvalidRunLifecycleBody`. Request: `POST approval_flow/runs/run_7/cancel` (and `restore-suspension`), body `{"foo":1}`, platform-operator caller. Before: ```json { "status": 400, "body": { "success": false, "error": { "code": "VALIDATION_FAILED", "httpStatus": 400, "message": "Unknown key `foo` — the cancel body accepts `reason`", "details": { "fields": [ { "field": "foo", "code": "unrecognized_keys", "message": "not a cancel body key — the cancel body accepts `reason`" } ] } } } } ``` After: the same envelope with `"code": "unknown_field"` on the row. `restore-suspension` is the same with its own door name in both messages, `unrecognized_keys` → `unknown_field`. `unknown_field` is what the sibling `resume` arm on the same run already answers for an unknown key. **2. Permission-set clone door, name rule** — `packages/runtime/src/domains/permission-set-clone.ts` `readCloneBody`. Request: `POST /permission-sets/crm_sales_rep/clone`, body `{"name":"My Copy","label":"X"}`, `manage_metadata` holder. Before: ```json { "status": 400, "body": { "success": false, "error": { "code": "VALIDATION_FAILED", "httpStatus": 400, "message": "'My Copy' is not a valid permission-set name — use lowercase snake_case: start with a lowercase letter, then lowercase letters, digits and underscores, at least 2 characters (e.g. 'sales_rep_copy').", "details": { "fields": [ { "field": "name", "code": "invalid_string", "message": "Identifier must be lowercase snake_case, starting with a letter, and may contain only letters, numbers, and underscores (e.g., \"user_profile\")" } ] } } } } ``` After: the same envelope with `"code": "invalid_format"` ("a declared pattern/format the value does not match"). What changes on the wire, stated in the changeset too: a client that switched on `details.fields[].code` sees `unknown_field` where it saw `unrecognized_keys`, and `invalid_format` where it saw `invalid_string`. Consumer read: `git grep` for either literal in `packages/client`, `packages/client-react`, `packages/cli` and in `../objectui` (`79cb8c0`) finds only Zod-issue reads (`issue.code === 'unrecognized_keys'` on a local Zod parse), none on these doors' wire rows. ## Census — every hand-built `details.fields[]` writer in `packages/runtime` Method: a TypeScript-AST walk of `packages/runtime/src/**/*.ts` (tests excluded) for every object literal carrying both `field` and `code`, plus every call to `validationFailure`, `fieldsFromZodIssues`, `validationFailureDetails` and `invalidQueryParam`. 31 literal rows, 48 helper calls, at this branch's head. Each row checked against `FieldErrorCode`; line numbers are this branch's. **Hand-built rows (31)** | Site | Door | `field` | `code` | Catalog member? | Closed-set check | |---|---|---|---|---|---| | `domains/activation-gate.ts:226` | activation body (catalog + action activation doors) | `(body)` | `invalid_type` | yes | driven | | `domains/activation-gate.ts:235` | ″ | each unknown key | `unknown_field` | yes | driven | | `domains/activation-gate.ts:244` | ″ | `enabled` | `invalid_type` | yes | driven | | `domains/analytics.ts:66` | `POST /analytics/query` | `filters` | `unknown_field` | yes | driven | | `domains/automation.ts:939` | run-lifecycle body (cancel / restore-suspension) | `(body)` | `invalid_type` | yes | driven, both doors | | `domains/automation.ts:962` | ″ | each unknown key | ~~`unrecognized_keys`~~ → `unknown_field` | **was no — fixed here** | driven, both doors | | `domains/automation.ts:980` | ″ | `reason` | `invalid_type` | yes | driven, both doors | | `domains/automation.ts:1498` | `flowDefinitionRefusal` fallback (engine refusal with no status, no issues) | `(body)` | `invalid_value` | yes | driven (via `POST /`) | | `domains/automation.ts:2699` | `POST /` (createFlow) | `(body)` | `invalid_type` | yes | driven | | `domains/automation.ts:2704` | ″ | `name` | `required` / `invalid_type` | yes, both arms | driven, both arms | | `domains/automation.ts:2870` | `POST /:name/toggle` | `(body)` | `invalid_type` | yes | driven | | `domains/automation.ts:2881` | ″ | each unknown key | `unknown_field` | yes | driven | | `domains/automation.ts:2886` | ″ | `enabled` | `invalid_type` | yes | driven | | `domains/automation.ts:2951` | `POST /:name/clone` (flow clone) | `(body)` | `invalid_type` | yes | driven | | `domains/automation.ts:2960` | ″ | each unknown key | `unknown_field` | yes | driven | | `domains/automation.ts:2974` | ″ | `name` | `required` / `invalid_type` | yes, both arms | driven, both arms | | `domains/automation.ts:2989` | ″ | `label` | `required` / `invalid_type` | yes, both arms | driven, both arms | | `domains/automation.ts:3153` | `POST /:name/runs/:runId/resume` | `(body)` | `invalid_type` | yes | driven | | `domains/automation.ts:3195` | ″ | each unknown key | `unknown_field` | yes | driven | | `domains/automation.ts:3240` | ″ | `inputs` / `variables` / `output` | `invalid_type` | yes | driven (`inputs`) | | `domains/automation.ts:3248` | ″ | `branchLabel` | `invalid_type` | yes | driven | | `domains/automation.ts:3286` | ″ | `inputs` (both spellings sent) | `invalid_value` | yes | driven | | `domains/automation.ts:3291` | ″ | `variables` (both spellings sent) | `invalid_value` | yes | driven | | `domains/automation.ts:3676` | `PUT /:name` (updateFlow) | `(body)` | `invalid_type` | yes | driven | | `domains/permission-set-clone.ts:170` | `POST /security/permission-sets/:name/clone` | `(body)` | `invalid_type` | yes | driven | | `domains/permission-set-clone.ts:177` | ″ | each unknown key | `unknown_field` | yes | driven | | `domains/permission-set-clone.ts:185` | ″ | `name` | `required` / `invalid_type` | yes, both arms | driven, both arms | | `domains/permission-set-clone.ts:197` | ″ | `name` (metadata name rule) | ~~`invalid_string`~~ → `invalid_format` | **was no — fixed here** | driven | | `domains/permission-set-clone.ts:205` | ″ | `label` | `required` / `invalid_type` | yes, both arms | driven, both arms | | `endpoint-executor.ts:386` | object endpoint, missing `?id` | `id` | `required` | yes | outside `src/domains/`; already pinned exactly in `endpoint-executor.test.ts` | | `query-param.ts:81` | `invalidQueryParam(param, code, …)` | the parameter | the `code` argument | yes, by construction | outside `src/domains/`; the parameter is typed `FieldErrorCode`, so tsc closes it (callers pass `invalid_boolean`, `invalid_number`, `min_value`, `max_value`, `invalid_type`, `invalid_option`) | **Not hand-built writers** (listed so the census is complete): - Zod-derived rows, mapped by construction through `fieldsFromZodIssues` → `zodIssuesToFields` (ADR-0114 D3): `domains/analytics.ts:72`, `domains/automation.ts:1485`, `domains/data.ts:120`, `domains/notifications.ts:221`. - Empty-row refusals: `domains/data.ts:312` and `:396`, `validationFailure(versionRefusal, [])`. - Relays of a producer's own rows, not built here: `flowDefinitionRefusal`'s pass-through of a service-thrown validation failure (`automation.ts`, `validationFailureDetails(err)`), `domains/actions.ts:913` (an action body's thrown `err.fields`), and `sandbox/quickjs-runner.ts` (fields carried across the VM boundary). - **A reader, not a wire write:** `domains/packages.ts:888` compares a Zod issue's own code (`issue.code === 'unrecognized_keys'` on the install-request parse) to name misplaced install options in the message. It writes no `details.fields[]` row, and comparing a Zod issue against Zod's vocabulary is correct there. Result: 2 off-catalog rows, both fixed here; 0 listed as remaining. ## Pins and their ablation New file `packages/runtime/src/domains/field-error-code-catalog.test.ts` (41 tests): - **Part 1, per fixed site** (5 tests). Each drives the real refusal and asserts the envelope (`400` + `error.code` `VALIDATION_FAILED`), that every `details.fields[].code` parses under `FieldErrorCode`, the exact rows (`{ field, code }`), and that nothing reached the service (`cancelRun` / `restoreConsumedSuspension` / `saveMetaItem` not called). The run-lifecycle pin sends two unknown keys, `resaon` and `requestedBy`, on both doors. The clone pin covers `My Copy`, `9copy` and `_copy`. - **Part 2, the closed-set check** (36 tests). It drives every hand-built row under `src/domains/` in the table above through the real `HttpDispatcher` and parses each `details.fields[].code` under `FieldErrorCode`. Each case also names its expected `{ field, code }`, so a case that reached some other refusal fails instead of passing on a row it never meant to check, and a refusal with no rows fails instead of passing vacuously. No gate script or lint rule is added; the rows are enumerated by hand, and the file's header says a new hand-built row joins the table in the change that adds it. Messages are not pinned. **Ablation**, one leg per fixed site, run after the fix was committed (`5fd5f5e71`). The test imports `../http-dispatcher.js` relatively, so the subject resolves to `src/`, with no `dist/` in the path. The mutation went through `node scripts/ablation-replace.mjs` (anchor must hit, blob must change, restore proven against `HEAD`) inside a wrapper with an `EXIT INT TERM` trap that re-proves the restore by blob hash and `git diff HEAD`. | Leg | Mutation (restores the old code) | On disk | Result | Restore | |---|---|---|---|---| | automation | `code: 'unknown_field' as const,` → `code: 'unrecognized_keys' as const,` | anchor 1 → 0, replacement 0 → 1, blob `84d4f8b34e42` → `5ad01f696481` | **4 failed / 37 passed**: both Part 1 run-lifecycle pins and both Part 2 run-lifecycle unknown-key rows, each failing with `details.fields[].code "unrecognized_keys" … is not a FieldErrorCode member` | blob back to `84d4f8b34e42` == HEAD, `git diff HEAD` empty | | clone | `code: 'invalid_format'` → `code: 'invalid_string'` | anchor 1 → 0, replacement 0 → 1, blob `820646d40ba1` → `ffdb9582fd37` | **4 failed / 37 passed**: the three Part 1 clone pins and the Part 2 name-rule row, each failing with `details.fields[].code "invalid_string" … is not a FieldErrorCode member` | blob back to `820646d40ba1` == HEAD, `git diff HEAD` empty | Direction observed: red, as predicted. In every failing case the closed-set assertion (the catalog parse) is the one that fires, and no other test moved. Green on the restored tree: 41/41. ## Tests At ``5fd5f5e71``: - `pnpm --filter @objectstack/runtime test` (vitest project `local`): **426 files passed, 6712 tests passed, 19 skipped**, exit 0. - `pnpm --filter @objectstack/runtime test:repo` (vitest project `repo`): **3 files passed, 808 tests passed**, exit 0. - `pnpm --filter @objectstack/runtime typecheck` (`tsc --noEmit` and `check:test-typecheck` over `tsconfig.test.json`): exit 0. The new test file carries no ledger entry, so it compiles with zero errors. - Other packages' tests: no assertion on `unrecognized_keys` or `invalid_string` from these two doors. Every hit outside `packages/runtime` reads a Zod issue's own code on a local parse; in `packages/runtime` the hits are the package-install, flow-register and external-validation Zod paths. ## Gates All at `5fd5f5e71`, the branch's final commit. - **Derived set:** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` printed 67 commands (change set: 4 paths against merge base `bfc1539fe`). Each ran byte for byte, with its exit code captured before any pipe. - 66 exited 0 on the first pass. - `pnpm check:dual-build-cjs-loads` exited 3 (`PREREQUISITE NOT MET`: it reads built output, and 39 packages had no `dist/`). After a full `turbo run build --filter=!@objectstack/docs` (76 tasks, 75 cached) it exited 0: 111 `require` entry points across 69 packages load. - The four other dist-reading gates were re-run on the fully built tree, all exit 0: `check:dts-closure` (76 packages swept, against 32 on the first pass), `check:sourcemap-no-sources-content`, `check:lean-entry-closure` and `check:published-files`. - **Reconciliation:** `node scripts/pm/dispatch-gates.mjs --ran ran.list` printed "67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN", exit 0. - **Full union:** `pnpm lint` (`eslint . --no-inline-config`), exit 0, no findings. - **Owned by CI:** the families the derivation names as outside its runnable set (artifact rosters, wide-population families, path-scheduled CI jobs, type-check lanes). They are not re-run locally. - No exported symbol is renamed or removed, so `check-platform-checklist.mjs --changed-from` does not apply. - `origin/main` was not merged into this branch. ## Acceptance notes - **Name of the schema.** The card and claim say `FieldErrorCodeSchema`; the Zod enum the spec exports is `FieldErrorCode` (`packages/spec/src/api/errors.zod.ts`), and no `FieldErrorCodeSchema` export exists. The pins parse under `FieldErrorCode`. - **The class stays open one level up.** `validationFailure(message, fields)` (`packages/types/src/validation-failure.ts`) types `fields` as `unknown[]`, which is how both rows got past tsc. Typing its rows' `code` as `FieldErrorCode` would close the class at compile time for every caller in every package. It is a signature change to a published `@objectstack/types` export and is outside this claim's file surface, so it is not done here. Carrier: none. - **The closed-set check is enumerated, not discovered.** It holds every row that exists today; a new hand-built row is checked only once it joins the table. The dispatch ruled out a new gate script or lint rule. - **Wider read-only sweep.** The same AST walk over `packages/*/src` (tests excluded), for literal rows `{ field, code, message }` whose string-literal `code` is not a catalog member, finds three hits, all in `packages/metadata-protocol/src/protocol.ts` (`field_removed`, `field_type_change`, `field_required_no_default`). Those are `DESTRUCTIVE_CHANGE` findings delivered as `err.issues`, a different position with its own vocabulary, not `details.fields[]`. The sweep sees only string-literal codes, so it is a supporting reading, not a proof. - `packages/spec/**` is not touched: the catalog was read, never extended. --- _Generated by [Claude Code](https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5018fc3 commit 3b8e2b1

4 files changed

Lines changed: 363 additions & 2 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime): two hand-built 400 refusals now put an ADR-0114 `FieldErrorCode` catalog member on `details.fields[].code` instead of a Zod issue code (#24597)
6+
7+
Clause-②: no
8+
9+
`FieldErrorCode` is a closed catalog: a service may not invent a field code. Two refusals built their `details.fields[]` rows by hand and wrote Zod's issue code there instead of a catalog member:
10+
11+
- **The run-lifecycle body refusal** on `POST /api/v1/automation/:name/runs/:runId/cancel` and `POST /api/v1/automation/:name/runs/:runId/restore-suspension`. For an unknown body key, the row's `code` changes from `unrecognized_keys` to `unknown_field`. `unknown_field` is the code the `resume` door on the same run already answers for an unknown key.
12+
- **The permission-set clone door's name rule** on `POST /api/v1/security/permission-sets/:name/clone`. For a `name` that is not lowercase snake_case, the row's `code` changes from `invalid_string` to `invalid_format`.
13+
14+
What changes on the wire: a client that switched on `details.fields[].code` sees `unknown_field` where it saw `unrecognized_keys`, and `invalid_format` where it saw `invalid_string`. Both are members the published catalog already declares. Nothing else changes: each refusal keeps its status (400), its `error.code` (`VALIDATION_FAILED`), its message and its rows' `field` and `message`. No body is accepted or refused that was not before, and no catalog member is added.

‎packages/runtime/src/domains/automation.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -954,9 +954,14 @@ function refuseInvalidRunLifecycleBody(
954954
+ (unknownKeys.includes('requestedBy')
955955
? '; `requestedBy` is filled from the authenticated caller and is not settable from the wire'
956956
: ''),
957+
// `unknown_field` — the ADR-0114 D2 catalog member for "a
958+
// key the target does not declare", the code the resume
959+
// arm answers for the same condition. This row used to
960+
// hand-spell Zod's `unrecognized_keys`, a code outside the
961+
// closed catalog.
957962
unknownKeys.map((k) => ({
958963
field: k,
959-
code: 'unrecognized_keys' as const,
964+
code: 'unknown_field' as const,
960965
message: `not a ${door} body key — the ${door} body accepts \`reason\``,
961966
})),
962967
),

0 commit comments

Comments
 (0)