From 6424803722cb33c6459efacc316210be573b371c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 10:24:02 +0000 Subject: [PATCH] feat(spec)!: reject unknown keys on the approval authoring schemas (#4001 step 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third click of the unknown-key strictness ratchet (flow + permission in #4071, RLS / sharing / position in #4099). Approval is a v17-new authoring surface — tightened while young, before stored volume exists: - ApprovalNodeConfigSchema, ApprovalNodeApproverSchema, ApprovalEscalationSchema and DecisionOutputDefSchema are .strict() with fixable errors via the shared strictUnknownKeyError factory. - ApprovalNodeConfig's guidance is the ADR-0019 re-home map: steps -> successive approval nodes on the canvas, entryCriteria -> the entering edge's condition, onApprove/onReject -> the approve/reject out-edges, rejectionBehavior -> a declared back-edge (ADR-0044) with maxRevisions. - The published JSON schema (getApprovalNodeConfigJsonSchema) now carries additionalProperties:false into the Studio property form AND registerFlow's per-node config validation (#4027/#4040) — deliberate: an approval gate that quietly ignores half its config is the worst ADR-0078 instance. Verified the #3746 hazard: z.toJSONSchema on the strict lazySchema does not throw and emits additionalProperties:false at both nesting levels (asserted in approval.test.ts). Verified: spec 6995 tests + tsc clean; all 12 check gates; plugin-approvals 326 / service-automation 457; dogfood 72 files / 418 tests; showcase / crm / todo validate clean. Refs #4001 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0147tNF4Snk7Ry1KGt4a5PY4 --- .changeset/unknown-key-strictness-step3.md | 31 +++++ content/docs/releases/v17.mdx | 10 +- .../2026-07-unknown-key-strictness-ledger.md | 18 +-- packages/spec/src/automation/approval.test.ts | 90 +++++++++++++++ packages/spec/src/automation/approval.zod.ts | 109 +++++++++++++++++- 5 files changed, 246 insertions(+), 12 deletions(-) create mode 100644 .changeset/unknown-key-strictness-step3.md diff --git a/.changeset/unknown-key-strictness-step3.md b/.changeset/unknown-key-strictness-step3.md new file mode 100644 index 0000000000..4f95aba829 --- /dev/null +++ b/.changeset/unknown-key-strictness-step3.md @@ -0,0 +1,31 @@ +--- +"@objectstack/spec": major +--- + +feat(spec)!: reject unknown keys on the approval authoring schemas (#4001 step 3) + +Third click of the unknown-key strictness ratchet (flow + permission in +#4071, RLS / sharing / position in #4099). Approval is a v17-new authoring +surface — tightened while young, before stored volume exists: + +- **`automation/approval.zod.ts`** — `ApprovalNodeConfigSchema`, + `ApprovalNodeApproverSchema`, `ApprovalEscalationSchema`, and + `DecisionOutputDefSchema` are `.strict()` with fixable errors. An approval + gate that quietly ignores half its config is the worst instance of the + ADR-0078 trap — the request routes, but not the way the author declared. +- The published JSON schema (`getApprovalNodeConfigJsonSchema`) now carries + `additionalProperties: false` into the Studio property form AND + `registerFlow()`'s per-node config validation (#4027/#4040), so an unknown + key inside an approval node's `config` is rejected at registration too. + +**Migration.** Any key now rejected was previously stripped and had no +runtime effect — removing or renaming it never changes behavior. Mappings +baked into the errors include the ADR-0019 re-home map for process-era +concepts: `steps` → successive approval NODES on the canvas, `entryCriteria` +→ the condition on the entering edge, `onApprove` / `onReject` → the nodes +wired to the `approve` / `reject` out-edges, `rejectionBehavior` → a declared +back-edge (ADR-0044) with `maxRevisions`. Plus spelling aliases: +`mode` / `approvalMode` → `behavior`, `quorum` → `minApprovals`, +`statusField` → `approvalStatusField`, `org` → `organization`, +`expandAs` → `resolveAs`, `timeout` / `hours` / `sla` → `timeoutHours`, +`to` / `target` → `escalateTo`, `name` → `key`, `widget` → `type`. diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index 46967ff265..0f0ce627e6 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -307,7 +307,7 @@ model), while `url` platform-wide means an HTTP endpoint to call (`http` node, webhooks). The singular `input` on `map` / `subflow` / `connector_action` is those nodes' own canonical key and is untouched. -### Flow, permission, RLS, sharing and position schemas reject unknown keys (#4001) +### Flow, permission, RLS, sharing, position and approval schemas reject unknown keys (#4001) Zod's default is `.strip`: a key a schema does not declare is silently discarded and the instance keeps parsing. On an authorable surface that is the @@ -341,6 +341,14 @@ schema to the two highest-risk authorable surfaces, per the triage in a deliberately flat position (ADR-0090 D3). Position also gains the `protection` block and ADR-0010 runtime envelope every sibling registered type already declared. +- **Approval nodes** — all four authoring schemas (node config, approver, + escalation, decision-output). Process-era keys carry the ADR-0019 re-home + map (`steps` → successive approval nodes, `entryCriteria` → the entering + edge's condition, `onApprove`/`onReject` → the `approve`/`reject` + out-edges, `rejectionBehavior` → a declared back-edge). The published JSON + schema carries `additionalProperties: false` into the Studio form and + `registerFlow()` config validation, so a mis-keyed approval `config` is + rejected at registration too. Every rejection is written to be self-fixing: it names the offending key and, where recognisable, the canonical spelling (`steps` → `nodes`, edge diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index e6e3143098..35bd074b61 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -166,7 +166,7 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `state-machine.zod.ts` | 7 | authorable (p) | | | `control-flow.zod.ts` | 6 | authorable (p) | validated structurally by `validateControlFlow` | | `bpmn-interop.zod.ts` | 5 | wire (p) | interop import shapes | -| `approval.zod.ts` | 4 | authorable | **next candidate** — v17 approval nodes are new authoring surface | +| `approval.zod.ts` | 4 | authorable | **strict as of #4001 step 3** — all four authoring schemas (node config / approver / escalation / decision-output). The published JSON schema carries `additionalProperties: false` into the Studio form AND `registerFlow()` config validation (#4027/#4040), so an unknown key in an approval node's `config` is rejected at registration too — verified: `z.toJSONSchema` on the strict lazySchema does not throw (#3746 hazard checked) | | `node-executor.zod.ts` | 4 | wire | executor contract | | `webhook.zod.ts` | 1 | authorable (p) | spec-only (#3461) | @@ -205,15 +205,19 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). 1. `ui/app.zod.ts` — `AppSchema` + navigation union (highest-traffic remaining authorable type; needs union-error design so the strict error is readable). -2. `automation/approval.zod.ts` — new v17 authoring surface, tighten while young. -3. `data/hook.zod.ts`, `data/datasource.zod.ts` — `defineHook` / stack config. -4. Promote this ledger to a machine-checked gate (pattern of +2. `data/hook.zod.ts`, `data/datasource.zod.ts` — `defineHook` / stack config. +3. Promote this ledger to a machine-checked gate (pattern of `packages/spec/liveness/` + `check:liveness`) once enough of the surface is classified that the table above is enforceable rather than descriptive. -Done in step 2 (this PR): `security/rls.zod.ts` + `security/sharing.zod.ts` -strict; `PositionSchema` strict with the protection envelope declared (closing -the known sibling gap below). +Done in step 2: `security/rls.zod.ts` + `security/sharing.zod.ts` strict; +`PositionSchema` strict with the protection envelope declared (closing the +known sibling gap below). + +Done in step 3: `automation/approval.zod.ts` — the four approval authoring +schemas, with the ADR-0019 re-home map as wrong-layer guidance +(`steps` / `entryCriteria` / `onApprove` / `onReject` / `rejectionBehavior` +each point at where the concept lives on the flow graph now). Long tail stays gated on a verification pass per shape — never a one-shot "make all ~453 sites strict" (ADR-0054 ratchet; #4001's own recommendation). diff --git a/packages/spec/src/automation/approval.test.ts b/packages/spec/src/automation/approval.test.ts index 3170ca532b..645bb4e89e 100644 --- a/packages/spec/src/automation/approval.test.ts +++ b/packages/spec/src/automation/approval.test.ts @@ -13,6 +13,7 @@ import { ApprovalNodeApproverSchema, ApprovalEscalationSchema, ApprovalNodeConfigSchema, + DecisionOutputDefSchema, getApprovalNodeConfigJsonSchema, normalizeDecisionOutputs, } from './approval.zod'; @@ -317,3 +318,92 @@ describe('normalizeDecisionOutputs', () => { expect(normalizeDecisionOutputs('next_reviewers')).toEqual([]); }); }); + +// #4001 step 3 — the four approval authoring schemas are `.strict()`: an +// undeclared key used to be dropped by zod's default `.strip`, so an approval +// gate shipped that quietly ignored part of its declared behavior. Approval is +// a v17-new surface, tightened before stored volume exists. The published JSON +// schema now carries additionalProperties:false into the Studio form AND +// registerFlow()'s per-node config validation (#4027/#4040) — asserted below. +describe('unknown keys are rejected, not stripped (#4001)', () => { + const unknownKeyIssue = (schema: { safeParse: (v: unknown) => any }, value: unknown) => { + const result = schema.safeParse(value); + expect(result.success).toBe(false); + return result.error!.issues.find((i: { code: string }) => i.code === 'unrecognized_keys'); + }; + const minimalConfig = { approvers: [{ type: 'manager' as const }] }; + + describe('ApprovalNodeConfigSchema', () => { + it('rejects an undeclared key instead of silently dropping it', () => { + expect(unknownKeyIssue(ApprovalNodeConfigSchema, { ...minimalConfig, notAKey: 1 })!.message) + .toContain('`notAKey`'); + }); + + it('carries the ADR-0019 re-home guidance for process-era keys', () => { + for (const key of ['steps', 'entryCriteria', 'onApprove', 'onReject', 'rejectionBehavior']) { + const message = unknownKeyIssue(ApprovalNodeConfigSchema, { ...minimalConfig, [key]: [] })!.message; + expect(message, `\`${key}\` should carry ADR-0019 guidance`).toContain('ADR-00'); + expect(message).toContain(`\`${key}\``); + } + }); + + it('points behavior/threshold synonyms at the canonical keys', () => { + expect(unknownKeyIssue(ApprovalNodeConfigSchema, { ...minimalConfig, mode: 'unanimous' })!.message) + .toContain('`mode` → `behavior`'); + expect(unknownKeyIssue(ApprovalNodeConfigSchema, { ...minimalConfig, quorum: 2 })!.message) + .toContain('`quorum` → `minApprovals`'); + }); + + it('accepts every key the schema declares (guards APPROVAL_NODE_CONFIG_KEYS drift)', () => { + const probes: Record = { + behavior: 'quorum', minApprovals: 2, lockRecord: false, + approvalStatusField: 'approval_status', onEmptyApprovers: 'fail', + decisionOutputs: ['next_approver', { key: 'picked', type: 'user' }], + escalation: { enabled: true, timeoutHours: 4 }, maxRevisions: 1, + }; + for (const [key, value] of Object.entries(probes)) { + const result = ApprovalNodeConfigSchema.safeParse({ ...minimalConfig, [key]: value }); + const unknown = result.success + ? undefined + : result.error.issues.find((i) => i.code === 'unrecognized_keys'); + expect(unknown, `\`${key}\` should be a declared config key`).toBeUndefined(); + } + }); + }); + + describe('ApprovalNodeApproverSchema', () => { + it('points org/resolveAs synonyms at the canonical keys', () => { + expect(unknownKeyIssue(ApprovalNodeApproverSchema, { type: 'position', value: 'cfo', org: '$root' })!.message) + .toContain('`org` → `organization`'); + expect(unknownKeyIssue(ApprovalNodeApproverSchema, { type: 'expression', value: 'vars.x', expandAs: 'user' })!.message) + .toContain('`expandAs` → `resolveAs`'); + }); + }); + + describe('ApprovalEscalationSchema', () => { + it('points timeout/target synonyms at the canonical keys', () => { + expect(unknownKeyIssue(ApprovalEscalationSchema, { timeoutHours: 4, timeout: 4 })!.message) + .toContain('`timeout` → `timeoutHours`'); + expect(unknownKeyIssue(ApprovalEscalationSchema, { timeoutHours: 4, target: 'ops' })!.message) + .toContain('`target` → `escalateTo`'); + }); + }); + + describe('DecisionOutputDefSchema', () => { + it('points name/widget synonyms at the canonical keys', () => { + expect(unknownKeyIssue(DecisionOutputDefSchema, { key: 'k', name: 'k' })!.message) + .toContain('`name` → `key`'); + expect(unknownKeyIssue(DecisionOutputDefSchema, { key: 'k', widget: 'user' })!.message) + .toContain('`widget` → `type`'); + }); + }); + + it('publishes additionalProperties:false through the JSON schema (Studio + registerFlow)', () => { + const js = getApprovalNodeConfigJsonSchema() as { + additionalProperties?: boolean; + properties?: { approvers?: { items?: { additionalProperties?: boolean } } }; + }; + expect(js.additionalProperties).toBe(false); + expect(js.properties?.approvers?.items?.additionalProperties).toBe(false); + }); +}); diff --git a/packages/spec/src/automation/approval.zod.ts b/packages/spec/src/automation/approval.zod.ts index f009320fc0..d2fbc34aa4 100644 --- a/packages/spec/src/automation/approval.zod.ts +++ b/packages/spec/src/automation/approval.zod.ts @@ -2,6 +2,7 @@ import { z } from 'zod'; import { lazySchema } from '../shared/lazy-schema'; +import { strictUnknownKeyError } from '../shared/suggestions.zod'; // Why the members sit in THIS order (the generated reference renders the JSDoc // below; this rationale stays in source): @@ -319,6 +320,39 @@ export const APPROVAL_BRANCH_LABELS = { resubmit: 'resubmit', } as const; +/* + * ── Unknown-key strictness (#4001 step 3) ─────────────────────────────────── + * + * The four AUTHORING schemas in this module are `.strict()`: a key they do not + * declare is a loud, fixable parse error, not a silent strip. Approval is a + * v17-new authoring surface — tightened while young, before any stored volume + * exists. Note the blast radius is wider than a spec parse: the published + * JSON schema (see {@link getApprovalNodeConfigJsonSchema}) carries + * `additionalProperties: false` into the Studio property form AND + * `registerFlow()`'s per-node config validation (#4027/#4040), so an unknown + * key inside an approval node's `config` is rejected at registration too — + * deliberately: an approval gate that quietly ignores half its config is the + * worst instance of the ADR-0078 trap. + */ + +/** Keys {@link ApprovalNodeApproverSchema} declares (drift-guarded by approval.test.ts). */ +const APPROVAL_APPROVER_KEYS = ['type', 'value', 'resolveAs', 'group', 'organization'] as const; + +const approvalApproverUnknownKeyError = strictUnknownKeyError({ + surface: 'this approval approver', + knownKeys: APPROVAL_APPROVER_KEYS, + aliases: { + approver: 'value', + userid: 'value', + org: 'organization', + grouplabel: 'group', + expandas: 'resolveAs', + }, + history: + 'Until #4001 these were dropped silently — the approver still parsed, so the ' + + 'request could route to the wrong slate without a diagnostic.', +}); + /** A single approver assignment on an Approval node. */ export const ApprovalNodeApproverSchema = lazySchema(() => z.object({ // `xEnumDeprecated` lists enum members that still PARSE but must not be @@ -433,7 +467,7 @@ export const ApprovalNodeApproverSchema = lazySchema(() => z.object({ + '`$parent` (one level up), or an organization slug. Omitted = the request\'s own organization.', xRef: { kind: 'organization', symbols: [...APPROVER_ORG_SYMBOLS] }, }), -})); +}, { error: approvalApproverUnknownKeyError }).strict()); export type ApprovalNodeApprover = z.infer; /** @@ -443,6 +477,15 @@ export type ApprovalNodeApprover = z.infer; * The runtime treats `key` as the whitelist entry either way — `type` and * `multiple` only shape the INPUT WIDGET, never the accepted value. */ +const decisionOutputUnknownKeyError = strictUnknownKeyError({ + surface: 'this decision-output declaration', + knownKeys: ['key', 'label', 'type', 'multiple', 'required'], + aliases: { name: 'key', widget: 'type', many: 'multiple' }, + history: + 'Until #4001 these were dropped silently — the declaration still parsed, so the ' + + 'decision dialog rendered a different input than the author specified.', +}); + export const DecisionOutputDefSchema = lazySchema(() => z.object({ /** The output key — what the flow receives as `.`. */ key: z.string().min(1).describe('Output key (the flow variable name under the node id)'), @@ -473,7 +516,7 @@ export const DecisionOutputDefSchema = lazySchema(() => z.object({ * filled them in. */ required: z.boolean().optional().describe('Approver must supply this output to approve'), -})); +}, { error: decisionOutputUnknownKeyError }).strict()); export type DecisionOutputDef = z.infer; /** @@ -509,6 +552,21 @@ export function normalizeDecisionOutputs( * Per-node SLA escalation — carried on the Approval node itself, so each * Approval step on the canvas defines its own SLA. */ +const approvalEscalationUnknownKeyError = strictUnknownKeyError({ + surface: 'this approval escalation', + knownKeys: ['enabled', 'timeoutHours', 'action', 'escalateTo', 'notifySubmitter'], + aliases: { + timeout: 'timeoutHours', + hours: 'timeoutHours', + sla: 'timeoutHours', + to: 'escalateTo', + target: 'escalateTo', + }, + history: + 'Until #4001 these were dropped silently — the escalation still parsed, so an SLA ' + + 'the author declared never fired the way they intended.', +}); + export const ApprovalEscalationSchema = lazySchema(() => z.object({ enabled: z.boolean().default(false).describe('Enable SLA-based escalation for this node'), timeoutHours: z.number().min(1).describe('Hours before escalation triggers'), @@ -525,7 +583,7 @@ export const ApprovalEscalationSchema = lazySchema(() => z.object({ xRef: { kind: 'position' }, }), notifySubmitter: z.boolean().default(true).describe('Notify the original submitter on escalation'), -})); +}, { error: approvalEscalationUnknownKeyError }).strict()); export type ApprovalEscalation = z.infer; /** @@ -544,6 +602,49 @@ export type ApprovalEscalation = z.infer; * first-class engine-adjacent state owned by `plugin-approvals`; this config * only describes how the node behaves. */ +/** Keys {@link ApprovalNodeConfigSchema} declares (drift-guarded by approval.test.ts). */ +const APPROVAL_NODE_CONFIG_KEYS = [ + 'approvers', 'behavior', 'minApprovals', 'lockRecord', 'approvalStatusField', + 'onEmptyApprovers', 'decisionOutputs', 'escalation', 'maxRevisions', +] as const; + +const approvalNodeConfigUnknownKeyError = strictUnknownKeyError({ + surface: "this approval node's config", + knownKeys: APPROVAL_NODE_CONFIG_KEYS, + aliases: { + approver: 'approvers', + approvalmode: 'behavior', + mode: 'behavior', + statusfield: 'approvalStatusField', + quorum: 'minApprovals', + }, + guidance: { + // The ADR-0019 re-home map: the process-level approval concepts an author + // (or AI) trained on Salesforce-style approval processes reaches for, each + // pointed at where the concept lives on the flow graph now. + steps: + '`steps` is not an approval-node config key — ADR-0019 collapsed the standalone ' + + 'approval process into Flow: successive approval STEPS are successive `approval` ' + + 'NODES on the canvas, each with its own config.', + entryCriteria: + '`entryCriteria` is not an approval-node config key — entry criteria are the ' + + '`condition` on the EDGE entering this node (ADR-0019).', + onApprove: + '`onApprove` is not an approval-node config key — on-approve actions are the ' + + "nodes wired to this node's `approve` out-edge (ADR-0019).", + onReject: + '`onReject` is not an approval-node config key — on-reject actions are the ' + + "nodes wired to this node's `reject` out-edge (ADR-0019).", + rejectionBehavior: + '`rejectionBehavior` is not an approval-node config key — back-to-previous is a ' + + 'declared BACK-EDGE to an earlier node (`type: \'back\'`, ADR-0044), and the ' + + 'revise loop is the `revise` out-edge with `maxRevisions` bounding it.', + }, + history: + 'Until #4001 these were dropped silently — the node still parsed, so an approval ' + + 'gate shipped that quietly ignored part of its declared behavior.', +}); + export const ApprovalNodeConfigSchema = lazySchema(() => z.object({ /** Who may act on this step. */ approvers: z.array(ApprovalNodeApproverSchema).min(1).describe('Allowed approvers for this node'), @@ -644,7 +745,7 @@ export const ApprovalNodeConfigSchema = lazySchema(() => z.object({ */ maxRevisions: z.number().int().min(0).default(3) .describe('Max send-backs for revision before auto-reject (0 = send-back disabled)'), -})); +}, { error: approvalNodeConfigUnknownKeyError }).strict()); export type ApprovalNodeConfig = z.infer; /**