Skip to content

Commit 821ac7a

Browse files
authored
feat(spec)!: reject unknown keys on the approval authoring schemas (#4001 step 3) (#4119)
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) carries additionalProperties:false into the Studio property form AND registerFlow's per-node config validation (#4027/#4040). Verified the #3746 hazard: z.toJSONSchema on the strict lazySchema does not throw. 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.
1 parent aaf746e commit 821ac7a

5 files changed

Lines changed: 246 additions & 12 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@objectstack/spec": major
3+
---
4+
5+
feat(spec)!: reject unknown keys on the approval authoring schemas (#4001 step 3)
6+
7+
Third click of the unknown-key strictness ratchet (flow + permission in
8+
#4071, RLS / sharing / position in #4099). Approval is a v17-new authoring
9+
surface — tightened while young, before stored volume exists:
10+
11+
- **`automation/approval.zod.ts`**`ApprovalNodeConfigSchema`,
12+
`ApprovalNodeApproverSchema`, `ApprovalEscalationSchema`, and
13+
`DecisionOutputDefSchema` are `.strict()` with fixable errors. An approval
14+
gate that quietly ignores half its config is the worst instance of the
15+
ADR-0078 trap — the request routes, but not the way the author declared.
16+
- The published JSON schema (`getApprovalNodeConfigJsonSchema`) now carries
17+
`additionalProperties: false` into the Studio property form AND
18+
`registerFlow()`'s per-node config validation (#4027/#4040), so an unknown
19+
key inside an approval node's `config` is rejected at registration too.
20+
21+
**Migration.** Any key now rejected was previously stripped and had no
22+
runtime effect — removing or renaming it never changes behavior. Mappings
23+
baked into the errors include the ADR-0019 re-home map for process-era
24+
concepts: `steps` → successive approval NODES on the canvas, `entryCriteria`
25+
→ the condition on the entering edge, `onApprove` / `onReject` → the nodes
26+
wired to the `approve` / `reject` out-edges, `rejectionBehavior` → a declared
27+
back-edge (ADR-0044) with `maxRevisions`. Plus spelling aliases:
28+
`mode` / `approvalMode``behavior`, `quorum``minApprovals`,
29+
`statusField``approvalStatusField`, `org``organization`,
30+
`expandAs``resolveAs`, `timeout` / `hours` / `sla``timeoutHours`,
31+
`to` / `target``escalateTo`, `name``key`, `widget``type`.

content/docs/releases/v17.mdx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ model), while `url` platform-wide means an HTTP endpoint to call (`http` node,
307307
webhooks). The singular `input` on `map` / `subflow` / `connector_action` is
308308
those nodes' own canonical key and is untouched.
309309

310-
### Flow, permission, RLS, sharing and position schemas reject unknown keys (#4001)
310+
### Flow, permission, RLS, sharing, position and approval schemas reject unknown keys (#4001)
311311

312312
Zod's default is `.strip`: a key a schema does not declare is silently
313313
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
341341
a deliberately flat position (ADR-0090 D3). Position also gains the
342342
`protection` block and ADR-0010 runtime envelope every sibling registered
343343
type already declared.
344+
- **Approval nodes** — all four authoring schemas (node config, approver,
345+
escalation, decision-output). Process-era keys carry the ADR-0019 re-home
346+
map (`steps` → successive approval nodes, `entryCriteria` → the entering
347+
edge's condition, `onApprove`/`onReject` → the `approve`/`reject`
348+
out-edges, `rejectionBehavior` → a declared back-edge). The published JSON
349+
schema carries `additionalProperties: false` into the Studio form and
350+
`registerFlow()` config validation, so a mis-keyed approval `config` is
351+
rejected at registration too.
344352

345353
Every rejection is written to be self-fixing: it names the offending key and,
346354
where recognisable, the canonical spelling (`steps``nodes`, edge

docs/audits/2026-07-unknown-key-strictness-ledger.md

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts).
166166
| `state-machine.zod.ts` | 7 | authorable (p) | |
167167
| `control-flow.zod.ts` | 6 | authorable (p) | validated structurally by `validateControlFlow` |
168168
| `bpmn-interop.zod.ts` | 5 | wire (p) | interop import shapes |
169-
| `approval.zod.ts` | 4 | authorable | **next candidate**v17 approval nodes are new authoring surface |
169+
| `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) |
170170
| `node-executor.zod.ts` | 4 | wire | executor contract |
171171
| `webhook.zod.ts` | 1 | authorable (p) | spec-only (#3461) |
172172

@@ -205,15 +205,19 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts).
205205

206206
1. `ui/app.zod.ts``AppSchema` + navigation union (highest-traffic remaining
207207
authorable type; needs union-error design so the strict error is readable).
208-
2. `automation/approval.zod.ts` — new v17 authoring surface, tighten while young.
209-
3. `data/hook.zod.ts`, `data/datasource.zod.ts``defineHook` / stack config.
210-
4. Promote this ledger to a machine-checked gate (pattern of
208+
2. `data/hook.zod.ts`, `data/datasource.zod.ts``defineHook` / stack config.
209+
3. Promote this ledger to a machine-checked gate (pattern of
211210
`packages/spec/liveness/` + `check:liveness`) once enough of the surface is
212211
classified that the table above is enforceable rather than descriptive.
213212

214-
Done in step 2 (this PR): `security/rls.zod.ts` + `security/sharing.zod.ts`
215-
strict; `PositionSchema` strict with the protection envelope declared (closing
216-
the known sibling gap below).
213+
Done in step 2: `security/rls.zod.ts` + `security/sharing.zod.ts` strict;
214+
`PositionSchema` strict with the protection envelope declared (closing the
215+
known sibling gap below).
216+
217+
Done in step 3: `automation/approval.zod.ts` — the four approval authoring
218+
schemas, with the ADR-0019 re-home map as wrong-layer guidance
219+
(`steps` / `entryCriteria` / `onApprove` / `onReject` / `rejectionBehavior`
220+
each point at where the concept lives on the flow graph now).
217221

218222
Long tail stays gated on a verification pass per shape — never a one-shot
219223
"make all ~453 sites strict" (ADR-0054 ratchet; #4001's own recommendation).

packages/spec/src/automation/approval.test.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
ApprovalNodeApproverSchema,
1414
ApprovalEscalationSchema,
1515
ApprovalNodeConfigSchema,
16+
DecisionOutputDefSchema,
1617
getApprovalNodeConfigJsonSchema,
1718
normalizeDecisionOutputs,
1819
} from './approval.zod';
@@ -317,3 +318,92 @@ describe('normalizeDecisionOutputs', () => {
317318
expect(normalizeDecisionOutputs('next_reviewers')).toEqual([]);
318319
});
319320
});
321+
322+
// #4001 step 3 — the four approval authoring schemas are `.strict()`: an
323+
// undeclared key used to be dropped by zod's default `.strip`, so an approval
324+
// gate shipped that quietly ignored part of its declared behavior. Approval is
325+
// a v17-new surface, tightened before stored volume exists. The published JSON
326+
// schema now carries additionalProperties:false into the Studio form AND
327+
// registerFlow()'s per-node config validation (#4027/#4040) — asserted below.
328+
describe('unknown keys are rejected, not stripped (#4001)', () => {
329+
const unknownKeyIssue = (schema: { safeParse: (v: unknown) => any }, value: unknown) => {
330+
const result = schema.safeParse(value);
331+
expect(result.success).toBe(false);
332+
return result.error!.issues.find((i: { code: string }) => i.code === 'unrecognized_keys');
333+
};
334+
const minimalConfig = { approvers: [{ type: 'manager' as const }] };
335+
336+
describe('ApprovalNodeConfigSchema', () => {
337+
it('rejects an undeclared key instead of silently dropping it', () => {
338+
expect(unknownKeyIssue(ApprovalNodeConfigSchema, { ...minimalConfig, notAKey: 1 })!.message)
339+
.toContain('`notAKey`');
340+
});
341+
342+
it('carries the ADR-0019 re-home guidance for process-era keys', () => {
343+
for (const key of ['steps', 'entryCriteria', 'onApprove', 'onReject', 'rejectionBehavior']) {
344+
const message = unknownKeyIssue(ApprovalNodeConfigSchema, { ...minimalConfig, [key]: [] })!.message;
345+
expect(message, `\`${key}\` should carry ADR-0019 guidance`).toContain('ADR-00');
346+
expect(message).toContain(`\`${key}\``);
347+
}
348+
});
349+
350+
it('points behavior/threshold synonyms at the canonical keys', () => {
351+
expect(unknownKeyIssue(ApprovalNodeConfigSchema, { ...minimalConfig, mode: 'unanimous' })!.message)
352+
.toContain('`mode` → `behavior`');
353+
expect(unknownKeyIssue(ApprovalNodeConfigSchema, { ...minimalConfig, quorum: 2 })!.message)
354+
.toContain('`quorum` → `minApprovals`');
355+
});
356+
357+
it('accepts every key the schema declares (guards APPROVAL_NODE_CONFIG_KEYS drift)', () => {
358+
const probes: Record<string, unknown> = {
359+
behavior: 'quorum', minApprovals: 2, lockRecord: false,
360+
approvalStatusField: 'approval_status', onEmptyApprovers: 'fail',
361+
decisionOutputs: ['next_approver', { key: 'picked', type: 'user' }],
362+
escalation: { enabled: true, timeoutHours: 4 }, maxRevisions: 1,
363+
};
364+
for (const [key, value] of Object.entries(probes)) {
365+
const result = ApprovalNodeConfigSchema.safeParse({ ...minimalConfig, [key]: value });
366+
const unknown = result.success
367+
? undefined
368+
: result.error.issues.find((i) => i.code === 'unrecognized_keys');
369+
expect(unknown, `\`${key}\` should be a declared config key`).toBeUndefined();
370+
}
371+
});
372+
});
373+
374+
describe('ApprovalNodeApproverSchema', () => {
375+
it('points org/resolveAs synonyms at the canonical keys', () => {
376+
expect(unknownKeyIssue(ApprovalNodeApproverSchema, { type: 'position', value: 'cfo', org: '$root' })!.message)
377+
.toContain('`org` → `organization`');
378+
expect(unknownKeyIssue(ApprovalNodeApproverSchema, { type: 'expression', value: 'vars.x', expandAs: 'user' })!.message)
379+
.toContain('`expandAs` → `resolveAs`');
380+
});
381+
});
382+
383+
describe('ApprovalEscalationSchema', () => {
384+
it('points timeout/target synonyms at the canonical keys', () => {
385+
expect(unknownKeyIssue(ApprovalEscalationSchema, { timeoutHours: 4, timeout: 4 })!.message)
386+
.toContain('`timeout` → `timeoutHours`');
387+
expect(unknownKeyIssue(ApprovalEscalationSchema, { timeoutHours: 4, target: 'ops' })!.message)
388+
.toContain('`target` → `escalateTo`');
389+
});
390+
});
391+
392+
describe('DecisionOutputDefSchema', () => {
393+
it('points name/widget synonyms at the canonical keys', () => {
394+
expect(unknownKeyIssue(DecisionOutputDefSchema, { key: 'k', name: 'k' })!.message)
395+
.toContain('`name` → `key`');
396+
expect(unknownKeyIssue(DecisionOutputDefSchema, { key: 'k', widget: 'user' })!.message)
397+
.toContain('`widget` → `type`');
398+
});
399+
});
400+
401+
it('publishes additionalProperties:false through the JSON schema (Studio + registerFlow)', () => {
402+
const js = getApprovalNodeConfigJsonSchema() as {
403+
additionalProperties?: boolean;
404+
properties?: { approvers?: { items?: { additionalProperties?: boolean } } };
405+
};
406+
expect(js.additionalProperties).toBe(false);
407+
expect(js.properties?.approvers?.items?.additionalProperties).toBe(false);
408+
});
409+
});

packages/spec/src/automation/approval.zod.ts

Lines changed: 105 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { z } from 'zod';
44
import { lazySchema } from '../shared/lazy-schema';
5+
import { strictUnknownKeyError } from '../shared/suggestions.zod';
56

67
// Why the members sit in THIS order (the generated reference renders the JSDoc
78
// below; this rationale stays in source):
@@ -319,6 +320,39 @@ export const APPROVAL_BRANCH_LABELS = {
319320
resubmit: 'resubmit',
320321
} as const;
321322

323+
/*
324+
* ── Unknown-key strictness (#4001 step 3) ───────────────────────────────────
325+
*
326+
* The four AUTHORING schemas in this module are `.strict()`: a key they do not
327+
* declare is a loud, fixable parse error, not a silent strip. Approval is a
328+
* v17-new authoring surface — tightened while young, before any stored volume
329+
* exists. Note the blast radius is wider than a spec parse: the published
330+
* JSON schema (see {@link getApprovalNodeConfigJsonSchema}) carries
331+
* `additionalProperties: false` into the Studio property form AND
332+
* `registerFlow()`'s per-node config validation (#4027/#4040), so an unknown
333+
* key inside an approval node's `config` is rejected at registration too —
334+
* deliberately: an approval gate that quietly ignores half its config is the
335+
* worst instance of the ADR-0078 trap.
336+
*/
337+
338+
/** Keys {@link ApprovalNodeApproverSchema} declares (drift-guarded by approval.test.ts). */
339+
const APPROVAL_APPROVER_KEYS = ['type', 'value', 'resolveAs', 'group', 'organization'] as const;
340+
341+
const approvalApproverUnknownKeyError = strictUnknownKeyError({
342+
surface: 'this approval approver',
343+
knownKeys: APPROVAL_APPROVER_KEYS,
344+
aliases: {
345+
approver: 'value',
346+
userid: 'value',
347+
org: 'organization',
348+
grouplabel: 'group',
349+
expandas: 'resolveAs',
350+
},
351+
history:
352+
'Until #4001 these were dropped silently — the approver still parsed, so the ' +
353+
'request could route to the wrong slate without a diagnostic.',
354+
});
355+
322356
/** A single approver assignment on an Approval node. */
323357
export const ApprovalNodeApproverSchema = lazySchema(() => z.object({
324358
// `xEnumDeprecated` lists enum members that still PARSE but must not be
@@ -433,7 +467,7 @@ export const ApprovalNodeApproverSchema = lazySchema(() => z.object({
433467
+ '`$parent` (one level up), or an organization slug. Omitted = the request\'s own organization.',
434468
xRef: { kind: 'organization', symbols: [...APPROVER_ORG_SYMBOLS] },
435469
}),
436-
}));
470+
}, { error: approvalApproverUnknownKeyError }).strict());
437471
export type ApprovalNodeApprover = z.infer<typeof ApprovalNodeApproverSchema>;
438472

439473
/**
@@ -443,6 +477,15 @@ export type ApprovalNodeApprover = z.infer<typeof ApprovalNodeApproverSchema>;
443477
* The runtime treats `key` as the whitelist entry either way — `type` and
444478
* `multiple` only shape the INPUT WIDGET, never the accepted value.
445479
*/
480+
const decisionOutputUnknownKeyError = strictUnknownKeyError({
481+
surface: 'this decision-output declaration',
482+
knownKeys: ['key', 'label', 'type', 'multiple', 'required'],
483+
aliases: { name: 'key', widget: 'type', many: 'multiple' },
484+
history:
485+
'Until #4001 these were dropped silently — the declaration still parsed, so the ' +
486+
'decision dialog rendered a different input than the author specified.',
487+
});
488+
446489
export const DecisionOutputDefSchema = lazySchema(() => z.object({
447490
/** The output key — what the flow receives as `<nodeId>.<key>`. */
448491
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({
473516
* filled them in.
474517
*/
475518
required: z.boolean().optional().describe('Approver must supply this output to approve'),
476-
}));
519+
}, { error: decisionOutputUnknownKeyError }).strict());
477520
export type DecisionOutputDef = z.infer<typeof DecisionOutputDefSchema>;
478521

479522
/**
@@ -509,6 +552,21 @@ export function normalizeDecisionOutputs(
509552
* Per-node SLA escalation — carried on the Approval node itself, so each
510553
* Approval step on the canvas defines its own SLA.
511554
*/
555+
const approvalEscalationUnknownKeyError = strictUnknownKeyError({
556+
surface: 'this approval escalation',
557+
knownKeys: ['enabled', 'timeoutHours', 'action', 'escalateTo', 'notifySubmitter'],
558+
aliases: {
559+
timeout: 'timeoutHours',
560+
hours: 'timeoutHours',
561+
sla: 'timeoutHours',
562+
to: 'escalateTo',
563+
target: 'escalateTo',
564+
},
565+
history:
566+
'Until #4001 these were dropped silently — the escalation still parsed, so an SLA ' +
567+
'the author declared never fired the way they intended.',
568+
});
569+
512570
export const ApprovalEscalationSchema = lazySchema(() => z.object({
513571
enabled: z.boolean().default(false).describe('Enable SLA-based escalation for this node'),
514572
timeoutHours: z.number().min(1).describe('Hours before escalation triggers'),
@@ -525,7 +583,7 @@ export const ApprovalEscalationSchema = lazySchema(() => z.object({
525583
xRef: { kind: 'position' },
526584
}),
527585
notifySubmitter: z.boolean().default(true).describe('Notify the original submitter on escalation'),
528-
}));
586+
}, { error: approvalEscalationUnknownKeyError }).strict());
529587
export type ApprovalEscalation = z.infer<typeof ApprovalEscalationSchema>;
530588

531589
/**
@@ -544,6 +602,49 @@ export type ApprovalEscalation = z.infer<typeof ApprovalEscalationSchema>;
544602
* first-class engine-adjacent state owned by `plugin-approvals`; this config
545603
* only describes how the node behaves.
546604
*/
605+
/** Keys {@link ApprovalNodeConfigSchema} declares (drift-guarded by approval.test.ts). */
606+
const APPROVAL_NODE_CONFIG_KEYS = [
607+
'approvers', 'behavior', 'minApprovals', 'lockRecord', 'approvalStatusField',
608+
'onEmptyApprovers', 'decisionOutputs', 'escalation', 'maxRevisions',
609+
] as const;
610+
611+
const approvalNodeConfigUnknownKeyError = strictUnknownKeyError({
612+
surface: "this approval node's config",
613+
knownKeys: APPROVAL_NODE_CONFIG_KEYS,
614+
aliases: {
615+
approver: 'approvers',
616+
approvalmode: 'behavior',
617+
mode: 'behavior',
618+
statusfield: 'approvalStatusField',
619+
quorum: 'minApprovals',
620+
},
621+
guidance: {
622+
// The ADR-0019 re-home map: the process-level approval concepts an author
623+
// (or AI) trained on Salesforce-style approval processes reaches for, each
624+
// pointed at where the concept lives on the flow graph now.
625+
steps:
626+
'`steps` is not an approval-node config key — ADR-0019 collapsed the standalone ' +
627+
'approval process into Flow: successive approval STEPS are successive `approval` ' +
628+
'NODES on the canvas, each with its own config.',
629+
entryCriteria:
630+
'`entryCriteria` is not an approval-node config key — entry criteria are the ' +
631+
'`condition` on the EDGE entering this node (ADR-0019).',
632+
onApprove:
633+
'`onApprove` is not an approval-node config key — on-approve actions are the ' +
634+
"nodes wired to this node's `approve` out-edge (ADR-0019).",
635+
onReject:
636+
'`onReject` is not an approval-node config key — on-reject actions are the ' +
637+
"nodes wired to this node's `reject` out-edge (ADR-0019).",
638+
rejectionBehavior:
639+
'`rejectionBehavior` is not an approval-node config key — back-to-previous is a ' +
640+
'declared BACK-EDGE to an earlier node (`type: \'back\'`, ADR-0044), and the ' +
641+
'revise loop is the `revise` out-edge with `maxRevisions` bounding it.',
642+
},
643+
history:
644+
'Until #4001 these were dropped silently — the node still parsed, so an approval ' +
645+
'gate shipped that quietly ignored part of its declared behavior.',
646+
});
647+
547648
export const ApprovalNodeConfigSchema = lazySchema(() => z.object({
548649
/** Who may act on this step. */
549650
approvers: z.array(ApprovalNodeApproverSchema).min(1).describe('Allowed approvers for this node'),
@@ -644,7 +745,7 @@ export const ApprovalNodeConfigSchema = lazySchema(() => z.object({
644745
*/
645746
maxRevisions: z.number().int().min(0).default(3)
646747
.describe('Max send-backs for revision before auto-reject (0 = send-back disabled)'),
647-
}));
748+
}, { error: approvalNodeConfigUnknownKeyError }).strict());
648749
export type ApprovalNodeConfig = z.infer<typeof ApprovalNodeConfigSchema>;
649750

650751
/**

0 commit comments

Comments
 (0)