From 5ea243a3181a817e538b19ee7f40d41e1f7551e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 16:19:54 +0000 Subject: [PATCH] =?UTF-8?q?feat(spec)!:=20reject=20unknown=20keys=20on=20t?= =?UTF-8?q?he=20flow-node=20config=20contracts=20(#4001=20=E6=89=B9=209)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourteen strip sites across the three automation node-config files close — the first `automation/` wave of the 2026-08-03 "necessary-and-complete" ruling. `automation/` remaining-strip: 67 → 53 (authorable 41 → 27). - io-node-config: NotifyConfig, HttpConfig - builtin-node-config: the CRUD quartet, Screen (+ field item + its options), Map - schemaless-node-config: Script, Subflow, Decision, DecisionCondition The deliberately-open `FlowNodeSchema.config` SLOT is untouched (ADR-0018 keeps `node.type` open for plugin executors); what closes is the per-node-type contract inside it. For `script` / `subflow` / `decision` this is the FIRST unknown-key gate, not a second one: `registerFlow()`'s #4277 rejection derives its declared set from a descriptor `configSchema`, and the schemaless class publishes none — so that walk skips them by construction. Curation is measured, not assumed. Every guidance/alias entry was checked against a repo-wide AST scan of 630 real flow-node payloads before it was written, and the campaign's finding 7 decided the one hard case: on a `decision`, `config.condition` is NOT renamed to `conditions` (one edit away, and taking that advice produces the double-declaration #4414 was filed for) — the mechanism is named instead. On a decision BRANCH the same word does rename, to `expression`. ADR-0087: no conversion needed. 160 flow nodes across the three shipped example apps were parsed against the new contracts — 52 carry one — with 0 unknown-key rejections, and all three `objectstack validate` runs pass. --- ...n-key-strictness-automation-node-config.md | 67 ++++++ .../automation/builtin-node-config.mdx | 40 +++- .../references/automation/io-node-config.mdx | 34 ++- .../automation/schemaless-node-config.mdx | 38 ++++ .../2026-07-unknown-key-strictness-ledger.md | 22 +- .../automation/builtin-node-config.test.ts | 200 ++++++++++++++++++ .../src/automation/builtin-node-config.zod.ts | 184 +++++++++++++++- .../src/automation/io-node-config.test.ts | 139 ++++++++++++ .../spec/src/automation/io-node-config.zod.ts | 87 +++++++- .../automation/schemaless-node-config.test.ts | 133 ++++++++++++ .../automation/schemaless-node-config.zod.ts | 135 +++++++++++- 11 files changed, 1047 insertions(+), 32 deletions(-) create mode 100644 .changeset/unknown-key-strictness-automation-node-config.md create mode 100644 packages/spec/src/automation/builtin-node-config.test.ts create mode 100644 packages/spec/src/automation/io-node-config.test.ts diff --git a/.changeset/unknown-key-strictness-automation-node-config.md b/.changeset/unknown-key-strictness-automation-node-config.md new file mode 100644 index 0000000000..612d2630ad --- /dev/null +++ b/.changeset/unknown-key-strictness-automation-node-config.md @@ -0,0 +1,67 @@ +--- +"@objectstack/spec": major +--- + +feat(spec)!: reject unknown keys on the flow-node config contracts (#4001 批 9) + +The first `automation/` wave of the 2026-08-03 "necessary-and-complete" +ruling. Fourteen strip sites across three files close, and `automation/`'s +remaining-strip count drops 67 → 53 (authorable 41 → 27). + +- **`automation/io-node-config.zod.ts`** — `NotifyConfigSchema`, + `HttpConfigSchema`. +- **`automation/builtin-node-config.zod.ts`** — the CRUD quartet + (`get_record` / `create_record` / `update_record` / `delete_record`), + `ScreenConfigSchema`, `ScreenFieldConfigSchema` and its `options` item, + `MapConfigSchema`. +- **`automation/schemaless-node-config.zod.ts`** — `ScriptConfigSchema`, + `SubflowConfigSchema`, `DecisionConfigSchema`, `DecisionConditionSchema`. + +The deliberately-open `FlowNodeSchema.config` SLOT is unchanged — ADR-0018 +keeps `node.type` open so plugins contribute their own executors, and closing +the slot would close that extension point. What is closed is the per-node-type +contract *inside* it. + +**Why the third file is different.** `registerFlow()` already hard-rejects +undeclared config keys against a node's descriptor `configSchema` (#4277), and +`script` / `subflow` / `decision` publish no descriptor `configSchema` — so +that walk skips them by construction. Until now those three had **no** +unknown-key enforcement at any layer. For them this is the first gate, not a +second one. + +**Migration.** Every key now rejected was previously stripped and had no +runtime effect, so removing or renaming one never changes behaviour. All three +shipped example apps were re-validated after the change and no stored shape +needed an ADR-0087 conversion (160 flow nodes walked, 52 carrying one of these +contracts, 0 rejections). The rejections carry their own prescriptions: + +- `notify`: `to` → `recipients`, `subject` → `title`, `body` → `message`, + `url` → `actionUrl`, `source: { object, id }` → `sourceObject` + `sourceId`. +- CRUD: `object` → `objectName`, `filters` → `filter`, + `fieldValues` → `fields`, `recordId` → a filter VALUE + (`filter: { id: '{record.id}' }` — no CRUD executor has ever read a + `recordId` key), and on `update_record` / `delete_record` `outputVariable` + is a documented absence, not a typo — read the row back with a following + `get_record`. +- `screen`: `object` → `objectName`, and on a field item + `visibleIf` → `visibleWhen`. +- `map` / `subflow`: `flow` → `flowName`. `subflow`'s `timeoutMs` belongs on + the NODE (`FlowNodeSchema.timeoutMs`), not in its config. +- `script`: `functionName` → `function`, `input` → `inputs` (the singular + stays canonical on `connector_action`'s `connectorConfig.input` — do not + "fix" that one). The five `actionType`-branch keys keep their existing + `retiredKey()` tombstones. +- `decision`: `config.condition` (singular) is **not** renamed to + `conditions`. Nothing reads it on a decision — it is the trigger gate on a + `start` node and inert everywhere else (#4414) — and declaring branches here + *and* on the out-edges is the double-declaration #4414 was filed for. + Branching lives on the out-edges. On a decision BRANCH the predicate slot is + `expression`, so `condition` → `expression` there. +- decision branch `target`: a VIRTUAL designer column projected from the + node's out-edges, never stored — route by matching the branch `label` to an + out-edge `label`. + +For a key rewritten at load by an ADR-0087 D2 conversion, reaching this +rejection means the config carries BOTH spellings: `renameConfigKey` leaves a +shadowed alias in place rather than clobbering the canonical winner, so the +retired twin is dead weight and should be deleted. diff --git a/content/docs/references/automation/builtin-node-config.mdx b/content/docs/references/automation/builtin-node-config.mdx index e84873b347..e9614d9bfa 100644 --- a/content/docs/references/automation/builtin-node-config.mdx +++ b/content/docs/references/automation/builtin-node-config.mdx @@ -55,9 +55,45 @@ parse the RAW stored config — their typed slots are strings (or `unknown` where values interpolate), so `\{token\}` templates pass and resolve at the -executor's existing interpolation points. Unknown keys are rejected earlier, +executor's existing interpolation points. -at `registerFlow()` (the tightened #4059 check); the parse here strips them. +## Unknown keys — closed here too, as of #4001 批 9 + +These contracts used to say "unknown keys are rejected earlier, at + +`registerFlow()` (the tightened #4059 check); the parse here strips them." + +The registration walk is still the first and more informative door — it + +descends NESTED config against the descriptor's JSON Schema, which is how it + +catches `fields[0].visibleIf` (#3528) and not just top-level typos — but + +"some other door is closed" is the exact reasoning #4001 exists to retire: + +the sibling of every guard in this campaign turned out to leave the other + +doors open, because its author was fixing one bug rather than auditing a + +surface. A config reaching `parse()` without passing registration (tooling + +that parses a contract directly, a host composing the engine itself) is no + +longer silently trimmed. + +The two doors are kept in agreement by `builtin-node-form-zod-ledger.test.ts`, + +which reconciles these key sets against the descriptors' in both directions. + +The per-key prescriptions below are the same curation the registration + +rejection carries in `FLOW_NODE_UNKNOWN_KEY_GUIDANCE` — the campaign's + +finding is that a bespoke guard's detection generalizes for free the moment + +a default flips, while its PROSE does not, so the prose is copied to the new + +door rather than left behind at the old one. Deliberately absent: diff --git a/content/docs/references/automation/io-node-config.mdx b/content/docs/references/automation/io-node-config.mdx index fe72aa5fd2..cfef1d6551 100644 --- a/content/docs/references/automation/io-node-config.mdx +++ b/content/docs/references/automation/io-node-config.mdx @@ -45,11 +45,39 @@ the INTERPOLATED config, because that is the shape its executor reads — a `\{token\}` in a typed slot (`timeoutMs`, `durable`) resolves to its real -type first. Unknown keys are the registration layer's job: `registerFlow()` +type first. -rejects keys the descriptor `configSchema` does not declare (the tightened +## Unknown keys — closed here too, as of #4001 批 9 -#4059 check), while the parse here strips them. +These contracts used to say "unknown keys are the registration layer's job": + +`registerFlow()` rejects keys the descriptor `configSchema` does not declare + +(the tightened #4059 check), and this parse merely stripped them. That is one + +door, and the #4001 campaign's second recurring finding is that a schema + +which strips by default leaves every OTHER door open — whoever writes the + +guard is fixing the bug in front of them, not auditing the surface. + +The registration check remains the first door a stored flow meets and the + +more informative one (it walks NESTED config against the descriptor's JSON + +Schema and prints the declared set per path, which a flat key list cannot). + +What changes is that a config reaching `parse()` by any OTHER route — a + +direct `NotifyConfigSchema.parse()` in tooling, a host that composes the + +engine without `registerFlow`, a future executor seam — no longer has its + +undeclared keys silently deleted. The two doors are kept in agreement by + +`io-node-form-zod-ledger.test.ts`, which reconciles this key set against the + +descriptor's in both directions. `connector_action` has no schema here on purpose: its config contract is diff --git a/content/docs/references/automation/schemaless-node-config.mdx b/content/docs/references/automation/schemaless-node-config.mdx index 6934c852f5..cd7b12846d 100644 --- a/content/docs/references/automation/schemaless-node-config.mdx +++ b/content/docs/references/automation/schemaless-node-config.mdx @@ -125,6 +125,44 @@ Undeclared aliases are NOT part of these contracts: `subflow`'s historical ever sees `flowName`. +## Unknown keys — closed as of #4001 批 9, and this class had NO other door + +The descriptor-schema'd builtins have a registration-time key gate: + +`registerFlow()` walks each node's `config` against the descriptor's + +`configSchema` and hard-rejects what it does not declare (#4277). **These + +three node types are exempt from that walk** — by construction, since it + +derives the declared set from a `configSchema` they publish none of + +(`validateNodeConfigKeys`' schemaless exemption). So until now the entire + +`script` / `subflow` / `decision` config surface had exactly zero unknown-key + +enforcement at any layer: the execute-time parse #4343 added checks types and + +requiredness, and Zod's default `.strip` deleted everything else in silence. + +That is the #4001 asymmetry in its purest form — a guard was written for the + +door in front of its author, and the class it structurally could not cover is + +precisely the class with no second door. Closing these shapes is therefore + +not a duplicate check for `script` and `subflow`; it is their first one. + +`decision` is still export-only, so its strictness binds at authoring + +(`tsc`), in the published JSON Schema, and in objectui's reconciliation — + +not at run time. It is closed anyway, because the campaign's whole finding + +is that a shape left open accretes a test, a form and a fixture that assert + +the openness, and then closing it is a migration instead of an edit. + **Source:** `packages/spec/src/automation/schemaless-node-config.zod.ts` diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 0151f81526..205fa7eb7b 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -535,9 +535,9 @@ not verdicts). | `bpmn-interop.zod.ts` | 5 | wire (p) | interop import shapes | | `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 | -| `io-node-config.zod.ts` | 2 | authorable | `NotifyConfigSchema` / `HttpConfigSchema` (#4045) — the sibling contracts that validate the **open** `config` slot on flow `notify` / `http` nodes. Authored per-node, so the open-slot exemption above does not extend to them; candidate once the executors' own drift is verified | -| `builtin-node-config.zod.ts` | 8 | authorable | Same family (#4045): the CRUD quartet, `screen`, `map`. Written from what the executors read rather than from the descriptors' `configSchema` literals, and reconciled bidirectionally by `builtin-node-form-zod-ledger.test.ts` — so unlike most rows here, this one already has a drift check of its own. Same candidacy note as `io-node-config` | -| `schemaless-node-config.zod.ts` | 4 | authorable | Same family, third panel (#4278): `script` / `subflow` / `decision` (+ the decision branch item) — the descriptor-schemaless nodes whose form lives in objectui's hand-written table. Written from the executors; the drift check is objectui's `flow-node-config.spec-reconciliation` test (cross-repo, via the published exports). Since #4343 `script` and `subflow` ARE parsed at execute time (`parse-config.ts`) — `script` once retiring its `actionType` branches left it flat — so strictness candidacy now follows `io-node-config` on the same terms rather than being moot; `decision` stays export-only | +| `io-node-config.zod.ts` | 2 | authorable | `NotifyConfigSchema` / `HttpConfigSchema` (#4045) — the sibling contracts that validate the **open** `config` slot on flow `notify` / `http` nodes. Authored per-node, so the open-slot exemption above does not extend to them. **Strict as of #4001 批 9**; the node `config` SLOT itself stays open (ADR-0018 keeps `node.type` open, so the slot cannot be closed without closing the plugin extension point). Five `guidance` entries carry the ADR-0087 notify aliases (`to`/`subject`/`body`/`url`/`source`) | +| `builtin-node-config.zod.ts` | 8 | authorable | Same family (#4045): the CRUD quartet, `screen`, `map`. Written from what the executors read rather than from the descriptors' `configSchema` literals, and reconciled bidirectionally by `builtin-node-form-zod-ledger.test.ts` — so unlike most rows here, this one already has a drift check of its own. **Strict as of #4001 批 9.** The curated tables are the `FLOW_NODE_UNKNOWN_KEY_GUIDANCE` prose from `service-automation`'s registration door, plus two entries that door never had: `recordId` (measured on CRUD nodes across the repo's own flow fixtures, read by no executor — on `delete_record` that is #3810 wearing a key that looks like a constraint) and `outputVariable` on `update_record` / `delete_record` (a documented ABSENCE, and the likeliest wrong key precisely because five sibling contracts declare it) | +| `schemaless-node-config.zod.ts` | 4 | authorable | Same family, third panel (#4278): `script` / `subflow` / `decision` (+ the decision branch item) — the descriptor-schemaless nodes whose form lives in objectui's hand-written table. Written from the executors; the drift check is objectui's `flow-node-config.spec-reconciliation` test (cross-repo, via the published exports — it compares `.shape` key sets, so strictness does not move it). Since #4343 `script` and `subflow` ARE parsed at execute time (`parse-config.ts`). **Strict as of #4001 批 9 — and this is the one row in the table where strictness is the FIRST unknown-key gate, not a second one**: `registerFlow()`'s #4277 rejection derives its declared set from a descriptor `configSchema`, so it structurally skips the schemaless class. `decision` stays export-only, closed anyway; its `condition` guidance suppresses a one-edit rename to `conditions` that #4414 proves is the worse outcome | | `webhook.zod.ts` | 1 | authorable (p) | spec-only (#3461) | | `time-relative-trigger.zod.ts` | 1 | authorable | **Undeclared until the #4001 re-measurement, and invisible for the worst possible reason**: `TimeRelativeTriggerSchema` is written `z\n .object({`, the old textual counter matched zero sites, and a zero-site file is SKIPPED by the coverage walk as "nothing to classify". So the gate whose whole promise is "no undeclared surface" reported green over an authorable schema — the same shape as `data/driver/`, one layer subtler, because this time the file was not hidden by the walk but by the counter feeding it. Classification is not a guess: the file's own `@example` blocks author it by hand into a flow start node (`config: { timeRelative: { object, dateField, offsetDays, filter } }`), which is the authoring door. A stripped key here means the sweep silently never matches — `offsetDay` for `offsetDays` returns a trigger that never fires, reported as configured | | `flow-function.zod.ts` | 1 | authorable | `FlowFunctionDeclarationSchema` (#4396) — the `{ handler, effect }` form of a `defineStack({ functions })` entry. Authored, but note what an undeclared key here would be: a sibling of a **live function**, not data. `defineStack`'s union already rejects a record whose `handler` is not callable, and the boot-path reader is the hand-written `normalizeFlowFunctionEntry` rather than a `.parse()` (re-validating a live handler every boot buys nothing), so strictness would bind at authoring only. Candidate on the same verify-first rule as its `*-node-config` neighbours | @@ -605,25 +605,31 @@ classes; where it does, the split is stated. **Only the authorable half is in th 2026-08-03 ruling's forced scope** — wire/open rows are listed so the arithmetic is complete and so nobody re-triages them from scratch next batch. -#### `automation/` — 67 strip of 75 +#### `automation/` — 53 strip of 75 | File | Strip | Sites | Class | Batch | |---|---|---|---|---| | `execution.zod.ts` | 13 | 13 | wire | **out of scope** — engine-emitted run state; the ledger row already says "never strict" | | `etl.zod.ts` | 10 | 10 | mixed | 7 authorable (`ETLSource` + `.incremental`, `ETLDestination`, `ETLTransformation`, `ETLPipeline` + `.retry` + `.notifications`), 3 wire (`ETLPipelineRun` + `.stats` + `.error` — run state) | -| `builtin-node-config.zod.ts` | 8 | 8 | authorable | CRUD quartet + `Screen` (+ `.options`) + `Map`; already has a bidirectional drift check (`builtin-node-form-zod-ledger.test.ts`) | | `flow.zod.ts` | 7 | 11 | mixed | 6 authorable (`FlowNode.connectorConfig` / `.position` / `.inputSchema` / `.waitEventConfig` / `.boundaryConfig`, `Flow.errorHandling`), 1 wire (`FlowVersionHistorySchema` — the ledger row already exempts it) | | `state-machine.zod.ts` | 6 | 6 | authorable (p) | `ActionRef` / `GuardRef` / `Transition` / `StateNode` + `.meta` / `StateMachine` | | `bpmn-interop.zod.ts` | 5 | 5 | wire (p) | **out of scope** — third-party BPMN import/export shapes; strictness turns an upstream addition into our parse crash | | `control-flow.zod.ts` | 5 | 5 | authorable (p) | `FlowRegion` / `Loop` / `ParallelBranch` / `Parallel` / `TryCatch` — validated structurally by `validateControlFlow` today, which is a sibling guard, not a key gate | | `node-executor.zod.ts` | 4 | 4 | wire | **out of scope** — executor registration contract, code-to-code | -| `schemaless-node-config.zod.ts` | 4 | 4 | authorable | `Script` / `Subflow` / `DecisionCondition` / `Decision`; `script` + `subflow` ARE parsed at execute time since #4343 | -| `io-node-config.zod.ts` | 2 | 2 | authorable | `NotifyConfig` / `HttpConfig` — the sibling contracts for the deliberately-open flow node `config` slot | | `flow-function.zod.ts` | 1 | 1 | authorable | `FlowFunctionDeclarationSchema`; binds at authoring only (the boot reader is `normalizeFlowFunctionEntry`, not a `.parse()`) | | `time-relative-trigger.zod.ts` | 1 | 1 | authorable | `TimeRelativeTriggerSchema` — **newly visible** (see its triage row); a stripped `offsetDay`/`withinDay` yields a trigger that never fires, reported as configured | | `webhook.zod.ts` | 1 | 1 | authorable (p) | `WebhookSchema`, spec-only (#3461) | -**Authorable strip in `automation/`: 41 of 67.** This is the ruling's "known main body". +Three rows left this table at **批 9** (#4001), the ruling's first `automation/` +wave — `builtin-node-config.zod.ts` (8), `schemaless-node-config.zod.ts` (4) and +`io-node-config.zod.ts` (2), all reaching zero strip. The reverse pin fired on +all three before the rows were removed, which is the only evidence that a +deletion here is bookkeeping rather than a guess. + +**Authorable strip in `automation/`: 27 of 53** (was 41 of 67). What remains of +the ruling's "known main body" is `etl` 7, `flow` 6, `state-machine` 6, +`control-flow` 5, and one each from `flow-function` / `time-relative-trigger` / +`webhook`. #### `ui/` — 124 strip of 198 diff --git a/packages/spec/src/automation/builtin-node-config.test.ts b/packages/spec/src/automation/builtin-node-config.test.ts new file mode 100644 index 0000000000..a07973b908 --- /dev/null +++ b/packages/spec/src/automation/builtin-node-config.test.ts @@ -0,0 +1,200 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * CRUD / `screen` / `map` config contracts — the #4001 批 9 closure (#4045). + * + * Live execute-time contracts (`parse-config.ts`), so these assertions are + * about behaviour, not documentation: an accepted shape runs and a rejected + * one refuses the node as a guard. + * + * Most of the file is about the PROSE. `service-automation`'s registration-time + * rejection already carried a curated table (`FLOW_NODE_UNKNOWN_KEY_GUIDANCE`), + * and this campaign's second finding is that a bespoke guard's DETECTION + * generalizes for free the moment a default flips while its prescriptions do + * not — so the prose was copied to this door and is pinned here. The two + * entries that door never had (`recordId`, and `outputVariable` on + * update/delete) were measured against every flow payload in the repo first. + */ + +import { describe, expect, it } from 'vitest'; + +import { + CreateRecordConfigSchema, + DeleteRecordConfigSchema, + GetRecordConfigSchema, + MapConfigSchema, + ScreenConfigSchema, + ScreenFieldConfigSchema, + UpdateRecordConfigSchema, +} from './builtin-node-config.zod.js'; + +interface Parseable { safeParse(v: unknown): { success: boolean; error?: { issues: ReadonlyArray<{ code: string; message: string }> } } } + +/** The unknown-key message, or `undefined` when the shape was accepted. */ +function unknownKeyMessage(schema: Parseable, value: unknown): string | undefined { + const result = schema.safeParse(value); + if (result.success) return undefined; + return result.error!.issues.find((i) => i.code === 'unrecognized_keys')?.message; +} + +describe('CRUD config contracts — strict as of #4001 批 9', () => { + it('accepts every declared key on each of the four', () => { + expect(GetRecordConfigSchema.parse({ + objectName: 'lead', filter: { status: 'new' }, fields: ['id'], limit: 5, outputVariable: 'leads', + })).toEqual({ objectName: 'lead', filter: { status: 'new' }, fields: ['id'], limit: 5, outputVariable: 'leads' }); + expect(CreateRecordConfigSchema.parse({ objectName: 'task', fields: { subject: 'hi' }, outputVariable: 'task' }).objectName).toBe('task'); + expect(UpdateRecordConfigSchema.parse({ objectName: 'lead', filter: { id: '1' }, fields: { status: 'won' } }).objectName).toBe('lead'); + expect(DeleteRecordConfigSchema.parse({ objectName: 'lead', filter: { status: 'stale' } }).objectName).toBe('lead'); + }); + + it.each([ + ['get_record', GetRecordConfigSchema, { objectName: 'lead' }], + ['create_record', CreateRecordConfigSchema, { objectName: 'task' }], + ['update_record', UpdateRecordConfigSchema, { objectName: 'lead' }], + ['delete_record', DeleteRecordConfigSchema, { objectName: 'lead' }], + ] as ReadonlyArray<[string, Parseable, Record]>)( + '%s: prescribes `objectName` for the retired `object` spelling — which edit distance cannot reach', + (nodeType, schema, base) => { + const message = unknownKeyMessage(schema, { ...base, object: 'lead' })!; + expect(message).toContain(`this ${nodeType} node config`); + expect(message).toContain('`objectName`'); + expect(message).toContain('flow-node-crud-object-alias'); + // The distance claim in the schema's comment, pinned: without the + // curated entry this key would get NO suggestion at all. + expect(message).not.toContain('`object` → '); + }, + ); + + it.each([ + ['get_record', GetRecordConfigSchema, { objectName: 'lead' }], + ['update_record', UpdateRecordConfigSchema, { objectName: 'lead' }], + ['delete_record', DeleteRecordConfigSchema, { objectName: 'lead' }], + ] as ReadonlyArray<[string, Parseable, Record]>)( + '%s: prescribes `filter` for the retired `filters` spelling, and names the #3810 hazard', + (_nodeType, schema, base) => { + const message = unknownKeyMessage(schema, { ...base, filters: { status: 'stale' } })!; + expect(message).toContain('flow-node-crud-filter-alias'); + expect(message).toContain('#3810'); + }, + ); + + it('create_record does NOT prescribe `filter` — it has no match map to point at', () => { + // Finding 12 generalized: never point an author at a key this shape does + // not declare. `create_record` is outside `flow-node-crud-filter-alias`'s + // node-type set precisely because it has no filter. + const message = unknownKeyMessage(CreateRecordConfigSchema, { objectName: 'task', filters: {} })!; + expect(message).not.toContain('flow-node-crud-filter-alias'); + expect(message).not.toContain('`filter` (singular)'); + }); + + it.each([ + ['create_record', CreateRecordConfigSchema, { objectName: 'task' }], + ['update_record', UpdateRecordConfigSchema, { objectName: 'lead' }], + ] as ReadonlyArray<[string, Parseable, Record]>)( + '%s: `fieldValues` gets the #2419 prescription, not a runtime alias', + (_nodeType, schema, base) => { + const message = unknownKeyMessage(schema, { ...base, fieldValues: { subject: 'hi' } })!; + expect(message).toContain('#2419'); + expect(message).toContain('`fields`'); + }, + ); + + it.each([ + ['get_record', GetRecordConfigSchema, { objectName: 'lead' }], + ['update_record', UpdateRecordConfigSchema, { objectName: 'lead' }], + ['delete_record', DeleteRecordConfigSchema, { objectName: 'lead' }], + ] as ReadonlyArray<[string, Parseable, Record]>)( + '%s: `recordId` — the shape the repo\'s own fixtures teach and no executor reads', + (_nodeType, schema, base) => { + const message = unknownKeyMessage(schema, { ...base, recordId: '{record.id}' })!; + expect(message).toContain('`filter`'); + expect(message).toContain('#3810'); + }, + ); + + it.each([ + ['update_record', UpdateRecordConfigSchema, { objectName: 'lead', filter: { id: '1' } }], + ['delete_record', DeleteRecordConfigSchema, { objectName: 'lead', filter: { id: '1' } }], + ] as ReadonlyArray<[string, Parseable, Record]>)( + '%s: `outputVariable` is a DOCUMENTED absence, so the rejection says so rather than staying silent', + (_nodeType, schema, base) => { + const message = unknownKeyMessage(schema, { ...base, outputVariable: 'updated' })!; + expect(message).toContain('#4045'); + expect(message).toContain('get_record'); + }, + ); + + it('the siblings that DO bind an output still accept it', () => { + expect(GetRecordConfigSchema.safeParse({ objectName: 'lead', outputVariable: 'lead' }).success).toBe(true); + expect(CreateRecordConfigSchema.safeParse({ objectName: 'task', outputVariable: 'task' }).success).toBe(true); + }); +}); + +describe('ScreenConfigSchema / ScreenFieldConfigSchema — strict as of #4001 批 9', () => { + it('accepts the flat and object-form shapes in full', () => { + expect(ScreenConfigSchema.safeParse({ + title: 'Details', description: 'Fill this in', waitForInput: true, + fields: [{ name: 'amount', label: 'Amount', type: 'number', required: true, defaultValue: 0, placeholder: '0.00', visibleWhen: "type == 'x'" }], + }).success).toBe(true); + expect(ScreenConfigSchema.safeParse({ + objectName: 'showcase_task', mode: 'edit', recordId: '{record.id}', idVariable: 'savedId', defaults: { status: 'open' }, + }).success).toBe(true); + }); + + it('rejects an undeclared key on the screen and on a field item', () => { + expect(unknownKeyMessage(ScreenConfigSchema, { title: 'x', heading: 'y' })) + .toContain('this screen node config'); + expect(unknownKeyMessage(ScreenFieldConfigSchema, { name: 'amount', hint: 'x' })) + .toContain('this screen field'); + }); + + it('`visibleIf` on a field carries the #3528 prescription — the typo the whole ladder descends from', () => { + const message = unknownKeyMessage(ScreenFieldConfigSchema, { name: 'amount', visibleIf: "type == 'x'" })!; + expect(message).toContain('`visibleWhen`'); + expect(message).toContain('#3528'); + }); + + it('`object` on a screen names `objectName`, which no other layer would tell the author', () => { + // `screen` is NOT in `flow-node-crud-object-alias`'s node-type set, so + // nothing rewrites it at load, and four edits is past the suggester's + // threshold. The alias entry is the only channel that carries it. + expect(unknownKeyMessage(ScreenConfigSchema, { object: 'showcase_task' })) + .toContain('`object` → `objectName`'); + }); + + it('closes the select-option item too — the eighth site', () => { + expect(ScreenFieldConfigSchema.safeParse({ + name: 'stage', options: [{ value: 'won', label: 'Won' }], + }).success).toBe(true); + const message = unknownKeyMessage(ScreenFieldConfigSchema, { + name: 'stage', options: [{ value: 'won', label: 'Won', disabled: true }], + }); + expect(message).toContain('this screen field option'); + expect(message).toContain('`disabled`'); + }); +}); + +describe('MapConfigSchema — strict as of #4001 批 9', () => { + it('accepts every declared key', () => { + expect(MapConfigSchema.parse({ + collection: '{tasks}', flowName: 'one_task_signoff', iteratorVariable: 'item', + indexVariable: 'i', itemObject: 'showcase_task', input: { id: '{item.id}' }, outputVariable: 'results', + }).flowName).toBe('one_task_signoff'); + }); + + it('prescribes `flowName` for the undeclared `flow` fallback', () => { + const message = unknownKeyMessage(MapConfigSchema, { collection: '{tasks}', flowName: 'per_row', flow: 'ignored' })!; + expect(message).toContain('this map node config'); + expect(message).toContain('flow-node-map-flow-alias'); + expect(message).toMatch(/delete/i); + }); + + it('rejects the SHADOWED alias the ADR-0087 conversion deliberately leaves behind', () => { + // `renameConfigKey` does nothing when the canonical key is already + // present, so `{ flowName, flow }` survives the load-path conversion + // intact — and under `.strip` the dead twin was then deleted in silence at + // this parse. That silence is the whole point of #4001: the author wrote + // two names for one thing and the platform picked one without saying so. + expect(MapConfigSchema.safeParse({ collection: '{r}', flowName: 'per_row', flow: 'ignored' }).success).toBe(false); + }); +}); diff --git a/packages/spec/src/automation/builtin-node-config.zod.ts b/packages/spec/src/automation/builtin-node-config.zod.ts index 90f69f32f3..645b5992c0 100644 --- a/packages/spec/src/automation/builtin-node-config.zod.ts +++ b/packages/spec/src/automation/builtin-node-config.zod.ts @@ -32,8 +32,29 @@ * type and `required` violations refuse the node as a guard. All of these * parse the RAW stored config — their typed slots are strings (or `unknown` * where values interpolate), so `{token}` templates pass and resolve at the - * executor's existing interpolation points. Unknown keys are rejected earlier, - * at `registerFlow()` (the tightened #4059 check); the parse here strips them. + * executor's existing interpolation points. + * + * ## Unknown keys — closed here too, as of #4001 批 9 + * + * These contracts used to say "unknown keys are rejected earlier, at + * `registerFlow()` (the tightened #4059 check); the parse here strips them." + * The registration walk is still the first and more informative door — it + * descends NESTED config against the descriptor's JSON Schema, which is how it + * catches `fields[0].visibleIf` (#3528) and not just top-level typos — but + * "some other door is closed" is the exact reasoning #4001 exists to retire: + * the sibling of every guard in this campaign turned out to leave the other + * doors open, because its author was fixing one bug rather than auditing a + * surface. A config reaching `parse()` without passing registration (tooling + * that parses a contract directly, a host composing the engine itself) is no + * longer silently trimmed. + * + * The two doors are kept in agreement by `builtin-node-form-zod-ledger.test.ts`, + * which reconciles these key sets against the descriptors' in both directions. + * The per-key prescriptions below are the same curation the registration + * rejection carries in `FLOW_NODE_UNKNOWN_KEY_GUIDANCE` — the campaign's + * finding is that a bespoke guard's detection generalizes for free the moment + * a default flips, while its PROSE does not, so the prose is copied to the new + * door rather than left behind at the old one. * * Deliberately absent: * - `assignment` — its config cannot be described by a fixed key set: with no @@ -52,6 +73,88 @@ import { z } from 'zod'; import { lazySchema } from '../shared/lazy-schema'; +import { strictObject } from '../shared/strict-object'; + +/** What a rejected key on these contracts silently did before #4001 批 9. */ +const BUILTIN_NODE_CONFIG_HISTORY = + 'Until #4001 an undeclared key here was dropped at the execute-time parse — the step still ran and the run ' + + 'still reported success, minus whatever the key was meant to configure.'; + +/** + * The two ADR-0087 D2 aliases every CRUD node shares. + * + * Both are retired SPELLINGS rather than typos, and both are rewritten at load + * (`flow-node-crud-object-alias`, `flow-node-crud-filter-alias`), so a config + * still carrying one at parse time carries the canonical key too — + * `renameConfigKey` leaves a shadowed alias in place instead of clobbering the + * winner. Hence each prescription answers both readings: the rename, and + * "delete the dead twin". + * + * `object` also earns its entry on distance alone: `object` → `objectName` is + * four edits against a threshold of two, so the suggester would say nothing at + * all for the single most common wrong spelling on this surface. + */ +const CRUD_ALIAS_GUIDANCE = { + object: + 'The object slot is `objectName`. `object` was the last tenant of the `readAliasedConfig` executor shim; it ' + + 'graduated into the ADR-0087 D2 conversion `flow-node-crud-object-alias` (#3796), which rewrites it at load — ' + + 'so a surviving `object` means `objectName` already won and this key is dead. Delete it.', + filters: + 'The match map is `filter` (singular). `filters` was a consumer-side executor fallback that graduated into the ' + + 'ADR-0087 D2 conversion `flow-node-crud-filter-alias`, which rewrites it at load; delete it once `filter` ' + + 'carries the pairs. Beware the half-migrated shape: an empty `filter` next to a populated `filters` is what ' + + 'made this alias dangerous enough to declare (#3810 — a match-everything write).', +} as const; + +/** + * `recordId` — the shape the repo's own flow fixtures teach and no CRUD + * executor has ever read. + * + * Measured, not guessed: the #4001 payload scan found it on `get_record`, + * `update_record` and `delete_record` nodes across `packages/spec`'s flow + * fixtures and the conversion registry's own illustrations, while every CRUD + * executor targets rows through `filter` only. Under `.strip` it parsed + * clean and then addressed nothing — on a `delete_record` that is precisely + * the #3810 hazard (a node that names no constraint) wearing a key that reads + * like one. + * + * Edit distance reaches none of the declared keys, so without this entry the + * rejection would name the key and offer nothing. + */ +const CRUD_RECORD_ID_GUIDANCE = + 'CRUD nodes address rows through `filter`, never through a `recordId` key — no executor has ever read one. ' + + "Write the id as a filter VALUE: `filter: { id: '{record.id}' }`, which is the shape the node's own descriptor " + + 'documents. This matters most on `delete_record`: a config whose only "constraint" is an unread key is a ' + + 'match-everything delete, the #3810 hazard.'; + +/** + * `fieldValues` — the AI-authoring dialect that never had a runtime reader. + * + * Kept verbatim in intent with `FLOW_NODE_UNKNOWN_KEY_GUIDANCE` in + * `service-automation`'s engine, which carries it at the registration door. + * Prime Directive #12's worked example is this exact key: framework#2419 + * proposed a `cfg.fields ?? cfg.fieldValues` runtime alias and it was rejected + * by design — the fix is the authoring source plus a loud rejection. + */ +const FIELD_VALUES_GUIDANCE = + 'The write map is `fields`. `fieldValues` was an AI-authoring dialect that never had a runtime reader, and a ' + + 'consumer-side `cfg.fields ?? cfg.fieldValues` alias was rejected by design (#2419) — the fix is the authoring ' + + 'source and this rejection, not a runtime fallback.'; + +/** + * `update_record` / `delete_record` bind no output — a documented absence, not + * an oversight (#4045 recorded it "so nobody re-chases it"). + * + * It is the single likeliest undeclared key on these two shapes, because the + * four sibling contracts (`get_record`, `create_record`, `map`, `script`, + * `subflow`) all declare it, and edit distance against the remaining keys + * reaches nothing. + */ +const NO_OUTPUT_VARIABLE_GUIDANCE = + 'This node binds no output — the executor reads no `outputVariable`, and #4045 recorded that absence ' + + 'deliberately after re-verifying the executor. Its siblings (`get_record`, `create_record`, `map`) do declare ' + + 'one, which is why the key looks universal and is not. To use what was written, follow this node with a ' + + '`get_record` that reads the row back.'; // ─── CRUD quartet ──────────────────────────────────────────────────── @@ -64,7 +167,11 @@ import { lazySchema } from '../shared/lazy-schema'; * widening the query (#3810). `limit > 1` selects `find` (a `records` list); * otherwise `findOne` (a single `record`). */ -export const GetRecordConfigSchema = lazySchema(() => z.object({ +export const GetRecordConfigSchema = lazySchema(() => strictObject({ + surface: 'this get_record node config', + history: BUILTIN_NODE_CONFIG_HISTORY, + guidance: { ...CRUD_ALIAS_GUIDANCE, recordId: CRUD_RECORD_ID_GUIDANCE }, +}, { /** Object to query (execute-time required). */ objectName: z.string().describe('Object to query'), /** Field/value pairs to match; operator objects and `{token}` templates are legal values. */ @@ -84,7 +191,14 @@ export type GetRecordConfig = z.input; export type GetRecordConfigParsed = z.infer; /** `create_record` node config — what the executor reads. `objectName` is execute-time required. */ -export const CreateRecordConfigSchema = lazySchema(() => z.object({ +export const CreateRecordConfigSchema = lazySchema(() => strictObject({ + surface: 'this create_record node config', + history: BUILTIN_NODE_CONFIG_HISTORY, + // `filters` is deliberately absent: `create_record` has no match map, so + // `flow-node-crud-filter-alias` never covered it and prescribing `filter` + // here would point an author at a key this shape does not declare. + guidance: { object: CRUD_ALIAS_GUIDANCE.object, fieldValues: FIELD_VALUES_GUIDANCE }, +}, { /** Object to insert into (execute-time required). */ objectName: z.string().describe('Object to insert into'), /** Field values to write on the new record; values interpolate `{token}` templates. */ @@ -102,7 +216,16 @@ export type CreateRecordConfigParsed = z.infer; * `update_record` node config — what the executor reads. No `outputVariable`: * the executor does not read one (recorded in #4045 so nobody re-chases it). */ -export const UpdateRecordConfigSchema = lazySchema(() => z.object({ +export const UpdateRecordConfigSchema = lazySchema(() => strictObject({ + surface: 'this update_record node config', + history: BUILTIN_NODE_CONFIG_HISTORY, + guidance: { + ...CRUD_ALIAS_GUIDANCE, + recordId: CRUD_RECORD_ID_GUIDANCE, + fieldValues: FIELD_VALUES_GUIDANCE, + outputVariable: NO_OUTPUT_VARIABLE_GUIDANCE, + }, +}, { /** Object to update (execute-time required). */ objectName: z.string().describe('Object to update'), /** Field/value pairs identifying the record(s) to update; an erased template condition refuses the node (#3810). */ @@ -116,7 +239,15 @@ export type UpdateRecordConfig = z.input; export type UpdateRecordConfigParsed = z.infer; /** `delete_record` node config — what the executor reads. The erased-condition guard matters most here (#3810). */ -export const DeleteRecordConfigSchema = lazySchema(() => z.object({ +export const DeleteRecordConfigSchema = lazySchema(() => strictObject({ + surface: 'this delete_record node config', + history: BUILTIN_NODE_CONFIG_HISTORY, + guidance: { + ...CRUD_ALIAS_GUIDANCE, + recordId: CRUD_RECORD_ID_GUIDANCE, + outputVariable: NO_OUTPUT_VARIABLE_GUIDANCE, + }, +}, { /** Object to delete from (execute-time required). */ objectName: z.string().describe('Object to delete from'), /** Field/value pairs identifying the record(s) to delete. */ @@ -134,7 +265,18 @@ export type DeleteRecordConfigParsed = z.infer; * `ScreenSpec` the client renders). `visibleWhen` is forwarded RAW — the client * re-evaluates it against the values collected so far (#3528). */ -export const ScreenFieldConfigSchema = lazySchema(() => z.object({ +export const ScreenFieldConfigSchema = lazySchema(() => strictObject({ + surface: 'this screen field', + history: BUILTIN_NODE_CONFIG_HISTORY, + guidance: { + visibleIf: + 'The visibility predicate is `visibleWhen` — bare CEL (ADR-0032), forwarded raw and re-evaluated ' + + 'client-side as the user types (#3528). `visibleIf` is four edits away from the right key, which is why ' + + 'the registration-time rejection prints the declared set rather than trusting a suggester; it is also the ' + + 'typo the whole undeclared-key ladder descends from — three diagnostic passes for a field that silently ' + + 'never hid.', + }, +}, { /** Field name — an item with an empty name is dropped. */ name: z.string().describe('Field name (the flow variable the value binds to)'), /** Display label. */ @@ -144,7 +286,10 @@ export const ScreenFieldConfigSchema = lazySchema(() => z.object({ /** Whether the runner requires a value before resume. */ required: z.boolean().optional().describe('Whether a value is required to submit'), /** Choices for a select-style field. */ - options: z.array(z.object({ + options: z.array(strictObject({ + surface: 'this screen field option', + history: BUILTIN_NODE_CONFIG_HISTORY, + }, { value: z.unknown().describe('Stored value'), label: z.string().describe('Display label'), })).optional().describe('Choices for a select-style field'), @@ -167,7 +312,17 @@ export type ScreenFieldConfig = z.input; * `recordId`, `defaults`, `idVariable` apply only there). `recordId` is what * makes `mode: 'edit'` usable — it names the record the form edits. */ -export const ScreenConfigSchema = lazySchema(() => z.object({ +export const ScreenConfigSchema = lazySchema(() => strictObject({ + surface: 'this screen node config', + history: BUILTIN_NODE_CONFIG_HISTORY, + // `object` → `objectName` is four edits against a threshold of two, so the + // suggester reaches it on no surface at all. It has a whole ADR-0087 + // conversion of its own on the CRUD nodes, which is exactly why an author + // arrives here already spelling it that way — but `screen` is NOT in that + // conversion's node-type set, so nothing rewrites it and the rename has to + // be said out loud. + aliases: { object: 'objectName' }, +}, { /** Heading (falls back to the node label). Interpolates `{token}`. */ title: z.string().optional().describe('Heading shown above the screen'), /** Body text. Interpolates `{token}`. */ @@ -208,7 +363,16 @@ export type ScreenConfigParsed = z.infer; * conversion `flow-node-map-flow-alias` rewrites it at load, so the executor * only ever sees `flowName` (#4045 — the `notify.source` graduation path). */ -export const MapConfigSchema = lazySchema(() => z.object({ +export const MapConfigSchema = lazySchema(() => strictObject({ + surface: 'this map node config', + history: BUILTIN_NODE_CONFIG_HISTORY, + guidance: { + flow: + 'The per-item subflow is named by `flowName`. `flow` was an undeclared executor fallback no schema or form ' + + 'described; it graduated into the ADR-0087 D2 conversion `flow-node-map-flow-alias` (#4045), which rewrites ' + + 'it at load — so a surviving `flow` means `flowName` already won and this key is dead. Delete it.', + }, +}, { /** The collection — a `{token}` template / bare variable name, or an inline array. */ collection: z.union([z.string(), z.array(z.unknown())]) .describe('Template/variable resolving to the array to process (an inline array is accepted)'), diff --git a/packages/spec/src/automation/io-node-config.test.ts b/packages/spec/src/automation/io-node-config.test.ts new file mode 100644 index 0000000000..0c3d81daf5 --- /dev/null +++ b/packages/spec/src/automation/io-node-config.test.ts @@ -0,0 +1,139 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `notify` / `http` config contracts — the #4001 批 9 closure (#4045, #4277). + * + * These are LIVE execute-time contracts (`parse-config.ts`), so what is pinned + * here is behaviour: a shape accepted runs, a shape rejected refuses the node + * as a guard. Before this batch an undeclared key was deleted in silence at + * this seam and the step reported success without it. + * + * The `guidance` assertions are the load-bearing half. This campaign's finding + * 7 is that a rejection's PROSE is behaviour — it tells the author what to do + * next, and a confidently wrong prescription is worse than none, because the + * author has no reason to doubt it. Every entry asserted below was measured + * against real payloads in the repo before it was written. + */ + +import { describe, expect, it } from 'vitest'; + +import { HttpConfigSchema, NotifyConfigSchema } from './io-node-config.zod.js'; + +/** The unknown-key message, or `undefined` when the shape was accepted. */ +function unknownKeyMessage(schema: { safeParse(v: unknown): { success: boolean; error?: { issues: ReadonlyArray<{ code: string; message: string }> } } }, value: unknown): string | undefined { + const result = schema.safeParse(value); + if (result.success) return undefined; + return result.error!.issues.find((i) => i.code === 'unrecognized_keys')?.message; +} + +describe('NotifyConfigSchema — strict as of #4001 批 9', () => { + it('accepts every declared key', () => { + const full = { + recipients: ['{record.assignee}'], + title: 'New task', + message: 'You have been assigned a task.', + channels: ['inbox'], + topic: 'notify', + severity: 'info', + sourceObject: 'showcase_task', + sourceId: '{record.id}', + actorId: '{trigger.userId}', + actionUrl: '/task/{record.id}', + payload: { taskName: '{record.name}' }, + }; + expect(NotifyConfigSchema.parse(full)).toEqual(full); + }); + + it('rejects an undeclared key instead of dropping it', () => { + // The pre-批-9 behaviour, stated as the thing that is no longer true: + // this parsed clean and the notification went out without a click target. + const message = unknownKeyMessage(NotifyConfigSchema, { + recipients: ['u1'], title: 'hi', sourceObjectt: 'showcase_task', + }); + expect(message).toContain('this notify node config'); + expect(message).toContain('`sourceObjectt`'); + // A one-character typo IS reachable by edit distance, so the suggestion + // must fire — this is the cheap half the curated table does not cover. + expect(message).toContain('`sourceObjectt` → `sourceObject`'); + }); + + it.each([ + ['to', ['u1'], '`recipients`'], + ['subject', 'New task', '`title`'], + ['body', 'Body text', '`message`'], + ['url', '/task/1', '`actionUrl`'], + ['source', { object: 'showcase_task', id: '1' }, '`sourceObject` + `sourceId`'], + ] as ReadonlyArray<[string, unknown, string]>)( + 'names the canonical key AND the dead-twin case for the retired `%s` alias', + (key, value, canonical) => { + const message = unknownKeyMessage(NotifyConfigSchema, { + recipients: ['u1'], title: 'hi', [key]: value, + }); + expect(message).toContain(canonical); + // Both readings must be served: the ADR-0087 conversion rewrites this + // key at load, so a config that still carries it at PARSE time also + // carries the canonical key — `renameConfigKey` leaves a shadowed alias + // in place rather than clobbering the winner. Without this half the + // prescription ("rename it") is wrong for the population that actually + // reaches this error. + expect(message).toContain('flow-node-notify-config-aliases'); + expect(message).toMatch(/delete/i); + }, + ); + + it('lists every violated key in one refusal', () => { + const message = unknownKeyMessage(NotifyConfigSchema, { + recipients: ['u1'], title: 'hi', to: ['u2'], subject: 'x', + }); + expect(message).toContain('`to`'); + expect(message).toContain('`subject`'); + }); + + it('never suggests a key the schema does not accept (finding 12)', () => { + const message = unknownKeyMessage(NotifyConfigSchema, { + recipients: ['u1'], title: 'hi', nonsense: 1, + })!; + const suggested = [...message.matchAll(/→ `([^`]+)`/g)].map((m) => m[1]!); + for (const key of suggested) { + expect(NotifyConfigSchema.safeParse({ recipients: ['u1'], title: 'hi', [key]: 'x' }) + .error?.issues.some((i) => i.code === 'unrecognized_keys')).not.toBe(true); + } + }); +}); + +describe('HttpConfigSchema — strict as of #4001 批 9', () => { + it('accepts every declared key', () => { + const full = { + url: 'https://example.test/hook', + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: { hello: 'world' }, + durable: true, + timeoutMs: 5000, + signingSecret: 'shh', + }; + expect(HttpConfigSchema.parse(full)).toEqual(full); + }); + + it('rejects an undeclared key and names the surface', () => { + const message = unknownKeyMessage(HttpConfigSchema, { url: 'https://x.test', retries: 3 }); + expect(message).toContain('this http node config'); + expect(message).toContain('`retries`'); + }); + + it('reaches the two plausible typos by edit distance, which is why it carries no curated table', () => { + // The claim in the schema's comment, pinned. If either of these stops + // being reachable, the comment is wrong and an entry is owed. + expect(unknownKeyMessage(HttpConfigSchema, { url: 'https://x.test', timeout: 5000 })) + .toContain('`timeout` → `timeoutMs`'); + expect(unknownKeyMessage(HttpConfigSchema, { url: 'https://x.test', header: {} })) + .toContain('`header` → `headers`'); + }); + + it('does not leak `notify`\'s vocabulary — `body` is canonical HERE', () => { + expect(HttpConfigSchema.safeParse({ url: 'https://x.test', body: { a: 1 } }).success).toBe(true); + // …and wrong on notify, where the guidance says so explicitly. + expect(unknownKeyMessage(NotifyConfigSchema, { recipients: ['u1'], title: 'hi', body: 'text' })) + .toContain('`body` IS canonical on an `http` node'); + }); +}); diff --git a/packages/spec/src/automation/io-node-config.zod.ts b/packages/spec/src/automation/io-node-config.zod.ts index 25465d5cd4..e68fe4ce7e 100644 --- a/packages/spec/src/automation/io-node-config.zod.ts +++ b/packages/spec/src/automation/io-node-config.zod.ts @@ -26,9 +26,26 @@ * post-interpolation guards still own "resolved to nothing". `http` parses * the INTERPOLATED config, because that is the shape its executor reads — * a `{token}` in a typed slot (`timeoutMs`, `durable`) resolves to its real - * type first. Unknown keys are the registration layer's job: `registerFlow()` - * rejects keys the descriptor `configSchema` does not declare (the tightened - * #4059 check), while the parse here strips them. + * type first. + * + * ## Unknown keys — closed here too, as of #4001 批 9 + * + * These contracts used to say "unknown keys are the registration layer's job": + * `registerFlow()` rejects keys the descriptor `configSchema` does not declare + * (the tightened #4059 check), and this parse merely stripped them. That is one + * door, and the #4001 campaign's second recurring finding is that a schema + * which strips by default leaves every OTHER door open — whoever writes the + * guard is fixing the bug in front of them, not auditing the surface. + * + * The registration check remains the first door a stored flow meets and the + * more informative one (it walks NESTED config against the descriptor's JSON + * Schema and prints the declared set per path, which a flat key list cannot). + * What changes is that a config reaching `parse()` by any OTHER route — a + * direct `NotifyConfigSchema.parse()` in tooling, a host that composes the + * engine without `registerFlow`, a future executor seam — no longer has its + * undeclared keys silently deleted. The two doors are kept in agreement by + * `io-node-form-zod-ledger.test.ts`, which reconciles this key set against the + * descriptor's in both directions. * * `connector_action` has no schema here on purpose: its config contract is * empty. The executor reads only the declared `FlowNodeSchema.connectorConfig` @@ -38,6 +55,54 @@ import { z } from 'zod'; import { lazySchema } from '../shared/lazy-schema'; +import { strictObject } from '../shared/strict-object'; + +/** + * What a rejected key on these contracts silently did before #4001 批 9. + * + * Shared by both schemas because the failure was identical: the step ran, the + * notification went out or the request was made, and the run reported success + * minus whatever the key was meant to configure. + */ +const IO_NODE_CONFIG_HISTORY = + 'Until #4001 an undeclared key here was dropped at the execute-time parse — the step still ran and ' + + 'the run still reported success, minus whatever the key was meant to configure.'; + +/** + * `notify` prescriptions for the four ADR-0087 D2 aliases and the nested + * `source` shape (#3796 / #4045). + * + * Each is a RETIRED SPELLING, not a typo, so a bare "did you mean" would + * under-serve it: `flow-node-notify-config-aliases` rewrites all five at load + * (including the `registerFlow` rehydration seam), which means a config that + * still carries one when it reaches this parse carries the canonical key too — + * `renameConfigKey` leaves a SHADOWED alias in place rather than clobbering the + * winner. So each prescription answers both readings: the rename, for whoever + * parses this contract directly, and "delete the dead twin", for whoever came + * through the load path. + */ +const NOTIFY_KEY_GUIDANCE: Readonly> = { + to: + 'The recipient slot is `recipients`. `to` is the pre-17 spelling, rewritten at load by the ADR-0087 D2 ' + + 'conversion `flow-node-notify-config-aliases` — so if `recipients` is already present, the conversion left ' + + '`to` behind as a dead twin (a shadowed alias is not clobbered) and it should be deleted.', + subject: + 'The heading slot is `title`. `subject` is the pre-17 spelling rewritten at load by ' + + '`flow-node-notify-config-aliases`; delete it once `title` carries the text.', + body: + 'The body slot is `message`. `body` is the pre-17 spelling rewritten at load by ' + + '`flow-node-notify-config-aliases`; delete it once `message` carries the text. (`body` IS canonical on an ' + + '`http` node — the key is wrong only here.)', + url: + 'The click-through slot is `actionUrl`. It was renamed at 17 because `url` elsewhere on the platform means ' + + '"HTTP endpoint to call" (`http` node, webhooks), a different concept from an in-app click target. ' + + '`flow-node-notify-config-aliases` rewrites it at load; delete it once `actionUrl` carries the link.', + source: + 'The click-through target is the flat PAIR `sourceObject` + `sourceId`, never a nested `source: { object, id }`. ' + + '`flow-node-notify-config-aliases` lifts the nested shape at load and drops it once every part is accounted ' + + 'for, so a surviving `source` means both flat keys were already set — delete it. Note the pair only takes ' + + 'effect together: a half-specified target is dropped so the inbox never renders a dead link.', +}; // ─── notify ────────────────────────────────────────────────────────── @@ -63,7 +128,11 @@ import { lazySchema } from '../shared/lazy-schema'; * conversion `flow-node-notify-config-aliases` rewrites them at load, so the * executor only ever sees the canonical keys below (#3796, #4045). */ -export const NotifyConfigSchema = lazySchema(() => z.object({ +export const NotifyConfigSchema = lazySchema(() => strictObject({ + surface: 'this notify node config', + history: IO_NODE_CONFIG_HISTORY, + guidance: NOTIFY_KEY_GUIDANCE, +}, { /** Who gets the notification — user id(s) / audience selector(s). */ recipients: z.union([z.string(), z.array(z.string())]) .describe('Recipient user id(s) / audience selector(s); `{token}` templates resolve per run'), @@ -114,7 +183,15 @@ export type NotifyConfigParsed = z.infer; * (retry / dead-letter) and returns `{ deliveryId }` instead of the * response; without an outbox it degrades to the inline call. */ -export const HttpConfigSchema = lazySchema(() => z.object({ +export const HttpConfigSchema = lazySchema(() => strictObject({ + surface: 'this http node config', + history: IO_NODE_CONFIG_HISTORY, + // No curated table: `http` has no retired spelling and no cross-surface + // near-miss this campaign's payload scan could attest. The two plausible + // typos are already reachable by edit distance (`timeout` → `timeoutMs`, + // `header` → `headers`), and inventing entries nothing refutes is how this + // campaign shipped four confidently-wrong prescriptions in one batch. +}, { /** Target URL (execute-time required). */ url: z.string().describe('Target URL'), /** HTTP method — default GET inline, POST when durable. */ diff --git a/packages/spec/src/automation/schemaless-node-config.test.ts b/packages/spec/src/automation/schemaless-node-config.test.ts index 84eec4198b..105306082c 100644 --- a/packages/spec/src/automation/schemaless-node-config.test.ts +++ b/packages/spec/src/automation/schemaless-node-config.test.ts @@ -20,11 +20,22 @@ import { describe, expect, it } from 'vitest'; import { z } from 'zod'; import { + DecisionConditionSchema, + DecisionConfigSchema, ScriptConfigSchema, SubflowConfigSchema, getSchemalessNodeConfigJsonSchemas, } from './schemaless-node-config.zod.js'; +interface Parseable { safeParse(v: unknown): { success: boolean; error?: { issues: ReadonlyArray<{ code: string; message: string }> } } } + +/** The unknown-key message, or `undefined` when the shape was accepted. */ +function unknownKeyMessage(schema: Parseable, value: unknown): string | undefined { + const result = schema.safeParse(value); + if (result.success) return undefined; + return result.error!.issues.find((i) => i.code === 'unrecognized_keys')?.message; +} + /** Every key the contract still declares, tombstones included. */ const SCRIPT_SHAPE_KEYS = [ 'actionType', 'function', 'inputs', 'outputVariable', @@ -119,6 +130,103 @@ describe('SubflowConfigSchema (#4343 — parsed at execute time)', () => { }); }); +describe('unknown keys — closed at #4001 批 9, and this class had no other gate', () => { + // The asymmetry worth stating once: `registerFlow()`'s #4277 undeclared-key + // rejection derives its declared set from a descriptor `configSchema`, and + // these three node types publish none — so the walk skips them BY + // CONSTRUCTION. Until this batch there was no layer at all at which a wrong + // key on a `script` / `subflow` / `decision` config was visible. + + it('script: rejects an undeclared key and names the surface', () => { + const message = unknownKeyMessage(ScriptConfigSchema, { function: 'score_lead', outputVariables: ['x'] })!; + expect(message).toContain('this script node config'); + // `outputVariables` is the exact key #4278 found objectui's form offering + // and no executor reading. One character from the real key, so the + // suggester earns its keep here. + expect(message).toContain('`outputVariables` → `outputVariable`'); + }); + + it.each([ + ['functionName', 'score_lead', '`function`'], + ['input', { leadId: '1' }, '`inputs`'], + ] as ReadonlyArray<[string, unknown, string]>)( + 'script: the retired `%s` alias gets its conversion named, not just a rename', + (key, value, canonical) => { + const message = unknownKeyMessage(ScriptConfigSchema, { function: 'f', [key]: value })!; + expect(message).toContain(canonical); + expect(message).toContain('flow-node-script-config-aliases'); + }, + ); + + it('script: `input`\'s prescription protects `connector_action`, where the singular IS canonical', () => { + // Without this the prescription reads as "the singular is always wrong", + // and an author obeying it globally breaks a working connector node. + expect(unknownKeyMessage(ScriptConfigSchema, { function: 'f', input: {} })) + .toContain('connectorConfig.input'); + }); + + it('script: the tombstoned keys are never offered as a suggestion (finding 12)', () => { + // `strictObject` filters unwritable keys out of the candidate list. Five + // `retiredKey()` tombstones sit in this shape, and `recipients` / + // `template` / `variables` / `script` are exactly the sort of near-miss a + // distance-based suggester reaches for. + for (const typo of ['recipient', 'templates', 'variable', 'scripts', 'actionTypes']) { + const message = unknownKeyMessage(ScriptConfigSchema, { function: 'f', [typo]: 'x' })!; + const suggested = [...message.matchAll(/→ `([^`]+)`/g)].map((m) => m[1]!); + for (const key of suggested) { + const issues = ScriptConfigSchema.safeParse({ function: 'f', [key]: 'x' }).error?.issues ?? []; + expect(issues.some((i) => i.message.startsWith('[REMOVED]') || /was removed in @objectstack\/spec/.test(i.message)), + `suggested \`${key}\` for \`${typo}\`, but that key is a tombstone`).toBe(false); + } + } + }); + + it('subflow: prescribes `flowName`, and points `timeoutMs` at the node it belongs on', () => { + expect(unknownKeyMessage(SubflowConfigSchema, { flowName: 'audit_flow', flow: 'ignored' })) + .toContain('flow-node-subflow-flow-alias'); + const timeout = unknownKeyMessage(SubflowConfigSchema, { flowName: 'audit_flow', timeoutMs: 30000 })!; + expect(timeout).toContain('FlowNodeSchema.timeoutMs'); + }); + + it('decision: `condition` gets the #4414 mechanism, NOT the one-edit rename to `conditions`', () => { + // The finding-7 case this batch had to get right. `condition` → + // `conditions` is one character, so a bare suggester proposes it with + // confidence — and taking that advice produces the double-declaration + // (branches here AND on the edges) that #4414 was filed for. Guidance + // suppresses the rename, so the assertion is as much about what is ABSENT. + const message = unknownKeyMessage(DecisionConfigSchema, { condition: "amount > 100000" })!; + expect(message).toContain('this decision node config'); + expect(message).toContain('#4414'); + expect(message).toContain('OUT-EDGES'); + expect(message).not.toContain('`condition` → `conditions`'); + }); + + it('decision branch: `condition` DOES rename here — the edge and the branch spell one intent two ways', () => { + // The mirror of the entry above, and deliberately the opposite verdict: + // on a branch item the predicate slot really is `expression`, and + // `FlowEdgeSchema` already aliases `expression` → `condition` going the + // other way. Same word, two surfaces, both directions declared. + expect(unknownKeyMessage(DecisionConditionSchema, { label: 'yes', condition: 'amount > 1' })) + .toContain('`condition` → `expression`'); + }); + + it('decision branch: `target` is named as VIRTUAL rather than renamed away', () => { + const message = unknownKeyMessage(DecisionConditionSchema, { label: 'yes', expression: 'a > 1', target: 'n3' })!; + expect(message).toContain('this decision branch'); + expect(message).toMatch(/virtual/i); + expect(message).toContain('flow-branch-label-unmatched'); + }); + + it('accepts every declared key on the decision pair', () => { + expect(DecisionConfigSchema.parse({ conditions: [{ label: 'big', expression: 'amount > 100000' }] })) + .toEqual({ conditions: [{ label: 'big', expression: 'amount > 100000' }] }); + // The no-conditions gateway shape stays legal — it is what every bundled + // example uses, and strictness must not turn "branch on the edges" into an + // error. + expect(DecisionConfigSchema.parse({})).toEqual({}); + }); +}); + describe('structural contract — what the downstream walkers require', () => { it('keeps the tombstoned keys IN the shape, so the ratchet can see them retired', () => { // A `retiredKey()` is still a property. Deleting it outright would read as @@ -144,4 +252,29 @@ describe('structural contract — what the downstream walkers require', () => { it('still converts without throwing, tombstones and all', () => { expect(() => z.toJSONSchema(SubflowConfigSchema, { unrepresentable: 'any' })).not.toThrow(); }); + + it('keeps the KEY SETS untouched — the property objectui reconciles across the repo seam', () => { + // #4001 批 9 closed these shapes without moving a single key, and that is + // the invariant the cross-repo check depends on: objectui's + // `flow-node-config.spec-reconciliation` test compares its hand-written + // `FLOW_NODE_CONFIG` table against `.shape` (not against a parse), so + // strictness is invisible to it — while an added or dropped key would + // break a repo we cannot fix from here. Pinned in THIS repo so the failure + // lands where the edit is made. + expect(Object.keys(SubflowConfigSchema.shape).sort()) + .toEqual(['flowName', 'input', 'outputVariable']); + expect(Object.keys(DecisionConfigSchema.shape).sort()).toEqual(['conditions']); + expect(Object.keys(DecisionConditionSchema.shape).sort()).toEqual(['expression', 'label']); + }); + + it('carries `additionalProperties: false` into the published JSON Schema without losing the expression markers', () => { + const json = getSchemalessNodeConfigJsonSchemas().decision as Record; + // #3746 hazard checked: `z.toJSONSchema` on a strict lazySchema does not throw… + expect(json.additionalProperties).toBe(false); + // …and the `.meta({ xExpression })` channel the expression ledger reads + // survives the conversion, one level down on the branch item. + const branch = ((json.properties as Record>>>) + .conditions.items.properties).expression; + expect(branch.xExpression).toBe('expression'); + }); }); diff --git a/packages/spec/src/automation/schemaless-node-config.zod.ts b/packages/spec/src/automation/schemaless-node-config.zod.ts index a91a2e69eb..62f0b966eb 100644 --- a/packages/spec/src/automation/schemaless-node-config.zod.ts +++ b/packages/spec/src/automation/schemaless-node-config.zod.ts @@ -74,11 +74,109 @@ * `flow` spelling graduated into the ADR-0087 D2 conversion * `flow-node-subflow-flow-alias` (the `map.flow` path), so the executor only * ever sees `flowName`. + * + * ## Unknown keys — closed as of #4001 批 9, and this class had NO other door + * + * The descriptor-schema'd builtins have a registration-time key gate: + * `registerFlow()` walks each node's `config` against the descriptor's + * `configSchema` and hard-rejects what it does not declare (#4277). **These + * three node types are exempt from that walk** — by construction, since it + * derives the declared set from a `configSchema` they publish none of + * (`validateNodeConfigKeys`' schemaless exemption). So until now the entire + * `script` / `subflow` / `decision` config surface had exactly zero unknown-key + * enforcement at any layer: the execute-time parse #4343 added checks types and + * requiredness, and Zod's default `.strip` deleted everything else in silence. + * + * That is the #4001 asymmetry in its purest form — a guard was written for the + * door in front of its author, and the class it structurally could not cover is + * precisely the class with no second door. Closing these shapes is therefore + * not a duplicate check for `script` and `subflow`; it is their first one. + * + * `decision` is still export-only, so its strictness binds at authoring + * (`tsc`), in the published JSON Schema, and in objectui's reconciliation — + * not at run time. It is closed anyway, because the campaign's whole finding + * is that a shape left open accretes a test, a form and a fixture that assert + * the openness, and then closing it is a migration instead of an edit. */ import { z } from 'zod'; import { lazySchema } from '../shared/lazy-schema'; import { retiredKey } from '../shared/retired-key'; +import { strictObject } from '../shared/strict-object'; + +/** + * What a rejected key on these contracts silently did before #4001 批 9 — and + * for `script` / `subflow` / `decision`, what NOTHING else was catching. + */ +const SCHEMALESS_NODE_CONFIG_HISTORY = + 'Until #4001 an undeclared key here was dropped in silence at every layer: these node types publish no ' + + "descriptor `configSchema`, so `registerFlow()`'s undeclared-key rejection (#4277) structurally skips them, " + + 'and the execute-time parse checked only types and requiredness.'; + +/** + * `script` prescriptions for the two ADR-0087 D2 aliases (#3796). + * + * `functionName` and `input` are retired SPELLINGS that + * `flow-node-script-config-aliases` rewrites at load, so — like the notify + * family — a config still carrying one at parse time carries the canonical key + * too (`renameConfigKey` leaves a shadowed alias alone). `input` earns its + * entry twice over: edit distance would suggest `inputs` without ever saying + * that `input` is *canonical* on `connector_action`'s `connectorConfig`, which + * is where the spelling leaked in from and where it must NOT be changed. + * + * The five `actionType`-branch keys need no entry here: `retiredKey()` puts the + * prescription in the shape itself, which is strictly stronger (it also types + * them `never`), and `strictObject` already keeps such keys out of the + * did-you-mean candidate list. + */ +const SCRIPT_KEY_GUIDANCE: Readonly> = { + functionName: + 'The callable reference is `function` (#1870). `functionName` was the AI/template-emitted alias, rewritten at ' + + 'load by the ADR-0087 D2 conversion `flow-node-script-config-aliases`; if `function` is already present the ' + + 'conversion left `functionName` behind as a dead twin — delete it.', + input: + 'The input map on a `script` node is `inputs` (plural). The singular `input` leaked in from ' + + "`connector_action`, where `connectorConfig.input` is a DIFFERENT and canonical surface — do not \"fix\" that " + + 'one. `flow-node-script-config-aliases` rewrites this key at load; delete it once `inputs` carries the values.', +}; + +/** `subflow` prescriptions — one retired spelling, one wrong layer. */ +const SUBFLOW_KEY_GUIDANCE: Readonly> = { + flow: + 'The invoked flow is named by `flowName`. `flow` was an undeclared executor fallback that no schema or form ' + + 'ever described; it graduated into the ADR-0087 D2 conversion `flow-node-subflow-flow-alias` (#4278), which ' + + 'rewrites it at load — so a surviving `flow` means `flowName` already won and this key is dead. Delete it.', + timeoutMs: + "A subflow step's timeout is the engine's per-node guard, so it belongs on the NODE, not in its config: " + + '`{ id, type: "subflow", timeoutMs: 30000, config: { … } }`. `FlowNodeSchema.timeoutMs` is the declared key.', +}; + +/** + * `decision` prescriptions for the legacy singular `config.condition` (#4414). + * + * This is the entry that could not be left to edit distance. `condition` → + * `conditions` is one character, so the suggester would confidently propose it + * — and taking that advice is the *worse* outcome: a decision that declares + * `conditions` here **and** carries per-edge `condition`s picks a branch and + * then lets that branch's edge re-decide, which is the double-declaration + * behind #4414 itself. Finding 7's shape ("this campaign's own helper + * signposting the way into the failure it exists to kill") applies exactly, so + * the rename is suppressed and the mechanism is named instead. + * + * The claim is measured, not assumed: `config.condition` is READ only on a + * `start` node (the trigger gate) and is inert on all nineteen other builtins — + * that is what `lint-flow-patterns`' `flow-inert-node-condition` advisory + * already says, and this table is where the same prose becomes a rejection. + */ +const DECISION_KEY_GUIDANCE: Readonly> = { + condition: + 'Nothing reads `config.condition` on a `decision`: the key is the trigger gate on a `start` node and is inert ' + + 'on every other node type (#4414), so a predicate written here never gates anything — it is still ' + + 'parse-validated at registration, which is why a malformed one is caught and an INERT one was not. Branching ' + + 'lives on the OUT-EDGES: give each branch its own `condition` and mark the fallback `isDefault: true`. Do not ' + + 'reach for the plural `conditions` here on the strength of the spelling — declaring branches here AND on the ' + + 'edges is the double-declaration #4414 was filed for. If the edges already carry the predicate, delete this key.', +}; // ─── script ────────────────────────────────────────────────────────── @@ -119,7 +217,11 @@ import { retiredKey } from '../shared/retired-key'; * `flow-node-script-branch-keys-removed` rewrites stored sources (moving a * shorthand `actionType` into `function`, where that is what it meant). */ -export const ScriptConfigSchema = lazySchema(() => z.object({ +export const ScriptConfigSchema = lazySchema(() => strictObject({ + surface: 'this script node config', + history: SCHEMALESS_NODE_CONFIG_HISTORY, + guidance: SCRIPT_KEY_GUIDANCE, +}, { /** * Registered function to call (`defineStack({ functions })`) — required: it * is the whole of what a `script` node does. @@ -198,7 +300,11 @@ export type ScriptConfigParsed = z.infer; * `flowName`. The node-level `timeoutMs` lives on {@link FlowNodeSchema}, not * here — a subflow step's timeout is the engine's per-node guard. */ -export const SubflowConfigSchema = lazySchema(() => z.object({ +export const SubflowConfigSchema = lazySchema(() => strictObject({ + surface: 'this subflow node config', + history: SCHEMALESS_NODE_CONFIG_HISTORY, + guidance: SUBFLOW_KEY_GUIDANCE, +}, { /** The flow to invoke (execute-time required). */ flowName: z.string().min(1).describe('Flow invoked as this step (it may pause — approval / screen / wait)'), /** Values passed to the child's input variables; `{token}` templates resolve against the parent's variables. */ @@ -231,7 +337,24 @@ export type SubflowConfigParsed = z.infer; * (objectui `flow-decision-edges`), never stored on the branch, so it is * deliberately absent here. */ -export const DecisionConditionSchema = lazySchema(() => z.object({ +export const DecisionConditionSchema = lazySchema(() => strictObject({ + surface: 'this decision branch', + history: SCHEMALESS_NODE_CONFIG_HISTORY, + // `condition` is the EDGE's spelling of the same intent one layer out + // (`FlowEdgeSchema` declares it, and already aliases `expression`/`when`/ + // `guard` TO it). The two surfaces spell one concept with two words, so the + // confusion is symmetric and the mirror alias belongs here — this is the + // `visibleWhen → visible` category, not a typo edit distance would reach. + aliases: { condition: 'expression' }, + guidance: { + target: + 'The designer\'s branch rows show a **Target** column, but it is VIRTUAL — objectui\'s `flow-decision-edges` ' + + "projects it from the node's out-edges and applies edits back to them; it is never stored on the branch. " + + "Route by making this branch's `label` match an out-edge's `label` exactly (a label nothing claims cannot " + + 'route: traversal falls back to considering every out-edge, and `os validate` reports it as ' + + '`flow-branch-label-unmatched`, #4414).', + }, +}, { /** Branch label — must match an out-edge's `label` to route anywhere. */ label: z.string().describe("Branch label; the winning branch resumes down the out-edge with this label (no match → the out-edge marked isDefault, or one labelled 'default')"), /** @@ -271,7 +394,11 @@ export type DecisionCondition = z.input; * parse-validates on every node at registration but the decision executor never * reads; branching predicates live in `conditions[]` or on the edges. */ -export const DecisionConfigSchema = lazySchema(() => z.object({ +export const DecisionConfigSchema = lazySchema(() => strictObject({ + surface: 'this decision node config', + history: SCHEMALESS_NODE_CONFIG_HISTORY, + guidance: DECISION_KEY_GUIDANCE, +}, { /** Ordered branches; first true expression wins, else the declared default edge. */ conditions: z.array(DecisionConditionSchema).optional() .describe('Ordered decision branches (first true expression wins; omit to branch purely on edge conditions)'),