Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .changeset/unknown-key-strictness-step3.md
Original file line number Diff line number Diff line change
@@ -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`.
10 changes: 9 additions & 1 deletion content/docs/releases/v17.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 11 additions & 7 deletions docs/audits/2026-07-unknown-key-strictness-ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) |

Expand Down Expand Up @@ -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).
90 changes: 90 additions & 0 deletions packages/spec/src/automation/approval.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
ApprovalNodeApproverSchema,
ApprovalEscalationSchema,
ApprovalNodeConfigSchema,
DecisionOutputDefSchema,
getApprovalNodeConfigJsonSchema,
normalizeDecisionOutputs,
} from './approval.zod';
Expand Down Expand Up @@ -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<string, unknown> = {
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);
});
});
109 changes: 105 additions & 4 deletions packages/spec/src/automation/approval.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<typeof ApprovalNodeApproverSchema>;

/**
Expand All @@ -443,6 +477,15 @@ export type ApprovalNodeApprover = z.infer<typeof ApprovalNodeApproverSchema>;
* 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 `<nodeId>.<key>`. */
key: z.string().min(1).describe('Output key (the flow variable name under the node id)'),
Expand Down Expand Up @@ -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<typeof DecisionOutputDefSchema>;

/**
Expand Down Expand Up @@ -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'),
Expand All @@ -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<typeof ApprovalEscalationSchema>;

/**
Expand All @@ -544,6 +602,49 @@ export type ApprovalEscalation = z.infer<typeof ApprovalEscalationSchema>;
* 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'),
Expand Down Expand Up @@ -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<typeof ApprovalNodeConfigSchema>;

/**
Expand Down
Loading