diff --git a/.changeset/21898-flow-builtin-node-config-values-refused.md b/.changeset/21898-flow-builtin-node-config-values-refused.md new file mode 100644 index 00000000000..f5f8ab6f1cb --- /dev/null +++ b/.changeset/21898-flow-builtin-node-config-values-refused.md @@ -0,0 +1,43 @@ +--- +'@objectstack/spec': minor +--- + +A builtin flow node's `config` value that its executor contract refuses is refused at parse, with a location, in the contract's own words: `create_record` `outputVariable: 42`, a screen field `min: '1'`, a `get_record` `limit: '10'` and the like no longer pass the build doors and then fail every run. + +Clause-②: yes (narrowing) + + + +**BREAKING**: an accept-set narrowing on a published authoring surface, shipped as `minor` under the launch-window convention for accept-set narrowings. + +**Why.** Every builtin executor parses its node's `config` against the contract `getBuiltinNodeConfigContracts()` names before it acts, and refuses the node on any finding. The build doors judged only the keys that contract requires, left out, so a present value it refuses passed `FlowSchema.parse`, `objectstack validate` and `objectstack compile` (compile copied it into `dist/objectstack.json`), registered, and failed every run that reached the node: `create_record 'mk': config does not satisfy the create_record contract — config.outputVariable: Invalid input: expected string, received number`. + +**What is refused.** A node of any builtin type (`get_record`, `create_record`, `update_record`, `delete_record`, `notify`, `http`, `screen`, `script`, `subflow`, `map`, `loop`, `parallel`, `try_catch`), at any depth, whose present config value its executor contract refuses — a wrong type, a value outside the declared set or range, an empty `function` / `flowName`, or a rule finding on present keys (a `notify` `template` beside an inline `title`). The refusal is the existing closed-set code `node-config-refused-by-contract`, `params: { nodeType, key }`, anchored at the key (`nodes.N.config.outputVariable`, `nodes.N.config.fields.0.min`), from the one judge `flowNodeConfigRefusals` that `FlowSchema.parse`, `AutomationEngine.registerFlow` (which parses first) and `objectstack validate` share. The issue's `code` is `custom`. That covers `FlowSchema`, `defineFlow()`, `defineStack` (`STACK_SCHEMA_INVALID`, 422, at `flows.N.nodes.M.config.`), `os validate`, `os compile`, an artifact's parse, `registerFlow` and the metadata save door (`422 INVALID_METADATA`). + +**What the build doors still accept, byte for byte.** Every value its contract accepts, and the values this arm holds back: + +- a value carrying a `{token}` (also spelled with double braces or a leading `$`) — never refused at the build doors for its pre-interpolation type. That is not a promise it runs: only `http` interpolates its config before it parses, so only an `http` slot sees the token's resolved value. Every other builtin parses its config as authored, so a token in one of its number or boolean slots (`limit: '{n}'`, `maxIterations: '{cap}'`, a screen field `min: '{m}'`, `multi: '{bulk}'`) still fails at its first run, exactly as before — write a literal there; +- on `http`, any value with a token inside it, and `signingSecret` (the credential channel may supply it); +- a `loop` with no `body` (its executor does not parse it), and the region slots of `loop`, `parallel` and `try_catch`; +- an undeclared or retired key, a screen field's `visibleWhen` and a CRUD `fields` value — each keeps the judge it had. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| `outputVariable: 42` | `outputVariable: 'taskId'` — the variable's name | +| a screen field `min: '1'`, `max: '10'` | `min: 1`, `max: 10` | +| `limit: '10'`, `maxIterations: '5'` (any number slot outside `http`) | `limit: 10`, `maxIterations: 5` — a literal number only: these executors parse the config as authored, so a `{token}` here passes the build and fails every run | +| `multi: 'true'`, a screen field `required: 'yes'` (any boolean slot outside `http`) | `multi: true`, `required: true` — a literal boolean only, for the same reason | +| `http` `timeoutMs: '5000'`, `durable: 'yes'` | `timeoutMs: 5000`, `durable: true` — or, on `http` alone, a sole-token template such as `timeoutMs: '{timeout}'`: `http` interpolates before it parses, so the token resolves to its value's type first | +| `severity: 'loud'`, `mode: 'view'` | one of the declared values (`'info'` / `'warning'` / `'critical'`; `'create'` / `'edit'`) | +| a `notify` with both `template` and `title` | one content path, as the refusal's sentence says | + +**The one-line fix: write the value the contract declares at the key the refusal names.** The runtime never ran such a node, so the fix changes nothing a working flow does. + +**Who is affected, measured.** At `833d57c9cf`, every builtin node `config` authored in this repository's examples, docs, skills and `packages/qa` fixtures (96 nodes), and every one in hotcrm at `4054ec2680` (138 nodes), parses under this arm. A second census at `d1c7d8d392` that also reads helper calls, same-file constants and assignments into a node config (1065 configs in this repository, 138 in hotcrm) found no other real writer; 64 configs here take a value from an import, a call or a spread that no static reading evaluates, and are not counted either way. The one real writer found to store a refused value is the Studio flow designer, which saved a screen field's Min / Max as strings until objectui `5ba255538a`. Deployed metadata, and other repositories, were not measured. Where such a node already sits in a stored flow, the whole flow is refused at registration: at boot it is skipped with a warn naming it, its trigger not armed, while the flows beside it register. + +### The kit + +- **The refusal.** The value half of the executor-contract arm of `flowNodeConfigRefusals` in `automation/flow-node-config-refusals.ts`; no new code joins `FLOW_SLOT_REFUSAL_CODES`, and `getBuiltinNodeConfigContracts()` keeps its 13 entries. +- **The ledger.** The D3 semantic entry `flow-builtin-node-config-values-refused` (protocol 18). No key is removed, so there is no tombstone, and there is no D2 conversion: the platform cannot know the value the author meant. diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 9ccdb3ca3fd..e3bedb044b0 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -2353,6 +2353,9 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { it('tolerates a screen with no fields, a non-array fields, and no config', () => { expect(validateStackExpressions(screenFlow([]))).toHaveLength(0); + // The expression walk tolerates the non-array `fields`; the one finding is + // the screen contract's own refusal of that value, from the flow's one + // config judge. expect(validateStackExpressions({ objects, flows: [{ @@ -2360,7 +2363,7 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { nodes: [{ id: 's', type: 'screen', config: { fields: 'nope' } }, { id: 's2', type: 'screen' }], edges: [], }], - })).toHaveLength(0); + }).map((issue) => issue.where)).toEqual(["flow 'f' · node 's' (screen) config.fields"]); }); }); }); diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-advisories.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-advisories.test.ts index b39601a751b..64bdc37b62f 100644 --- a/packages/metadata-protocol/src/protocol-publish-drafts-advisories.test.ts +++ b/packages/metadata-protocol/src/protocol-publish-drafts-advisories.test.ts @@ -202,7 +202,7 @@ const advisoryFlow = (name: string) => ({ /** The same flow with the bulk write bounded — no finding of any severity. */ const cleanFlow = (name: string) => { const flow = advisoryFlow(name); - (flow.nodes[1] as any).config.filter = [{ field: 'created_at', operator: 'lt', value: '2020-01-01' }]; + (flow.nodes[1] as any).config.filter = { created_at: { $lt: '2020-01-01' } }; return flow; }; diff --git a/packages/objectql/src/publish-meta-response-conformance.test.ts b/packages/objectql/src/publish-meta-response-conformance.test.ts index cdf2ba40787..370b0a16ff6 100644 --- a/packages/objectql/src/publish-meta-response-conformance.test.ts +++ b/packages/objectql/src/publish-meta-response-conformance.test.ts @@ -410,7 +410,7 @@ describe('publishMetaItem carries the runtime authoring gate\'s advisories (#917 /** The same flow with the bulk write bounded — no finding of any severity. */ const cleanFlow = () => { const flow = advisoryFlow(); - (flow.nodes[1] as any).config.filter = [{ field: 'created_at', operator: 'lt', value: '2020-01-01' }]; + (flow.nodes[1] as any).config.filter = { created_at: { $lt: '2020-01-01' } }; // [#21470] Named for the row it is saved under (`bounded_purge`): a // body `name` that is not its row's is refused by the write doors. flow.name = 'bounded_purge'; diff --git a/packages/objectql/src/save-meta-response-conformance.test.ts b/packages/objectql/src/save-meta-response-conformance.test.ts index 0c2ebd9a66e..5a5e587a538 100644 --- a/packages/objectql/src/save-meta-response-conformance.test.ts +++ b/packages/objectql/src/save-meta-response-conformance.test.ts @@ -295,7 +295,7 @@ describe('saveMetaItem carries the runtime authoring gate\'s advisories (#4717 /** The same flow with the bulk write bounded — no finding of any severity. */ const cleanFlow = () => { const flow = advisoryFlow(); - (flow.nodes[1] as any).config.filter = [{ field: 'created_at', operator: 'lt', value: '2020-01-01' }]; + (flow.nodes[1] as any).config.filter = { created_at: { $lt: '2020-01-01' } }; // [#21470] Named for the row it is saved under (`bounded_purge`): a // body `name` that is not its row's is refused by the write doors. flow.name = 'bounded_purge'; diff --git a/packages/services/service-automation/src/builtin/config-parse.test.ts b/packages/services/service-automation/src/builtin/config-parse.test.ts index 59e4d80d379..5d1543d4b85 100644 --- a/packages/services/service-automation/src/builtin/config-parse.test.ts +++ b/packages/services/service-automation/src/builtin/config-parse.test.ts @@ -109,15 +109,33 @@ async function runStripped( return engine.execute('f'); } +/** + * #21898 — a VALUE the executor contract refuses is refused at the build doors + * too now ({@link doorRefusal} pins that half, at the key). The execute-time + * parse is met the same way as above: {@link runPatched} registers the node + * with a value the contract accepts and writes the refused one into the stored + * flow before the run. + */ +async function runPatched( + engine: AutomationEngine, + type: string, + whole: Record, + patch: Record, + extra?: { nodes?: any[]; edges?: any[]; variables?: any[] }, +) { + const stored = engine.registerFlow('f', flowWith(type, whole, extra)); + Object.assign(stored.nodes.find((n) => n.id === 'n1')!.config as Record, patch); + return engine.execute('f'); +} + describe('execute-time config parse (#4277)', () => { it('refuses a wrong-typed declared key, naming the exact path', async () => { + // `limit` must be a number — the executor never honored a string here, so + // this was dead config that now fails loudly instead of silently doing + // nothing: at registration, at the key, and at the run. + expect(doorRefusal('get_record', { objectName: 'crm_lead', limit: 'ten' })).toContain('refused at `limit`'); const engine = engineWith(); - // `limit` is declared (so registration accepts it) but must be a number — - // the executor never honored a string here, so this was dead config that - // now fails loudly instead of silently doing nothing. - engine.registerFlow('f', flowWith('get_record', { objectName: 'crm_lead', limit: 'ten' })); - - const result = await engine.execute('f'); + const result = await runPatched(engine, 'get_record', { objectName: 'crm_lead', limit: 10 }, { limit: 'ten' }); expect(result.success).toBe(false); expect(result.error).toContain('get_record'); expect(result.error).toContain('config.limit'); @@ -126,16 +144,16 @@ describe('execute-time config parse (#4277)', () => { it('a parse refusal is a guard — a fault edge does NOT route it', async () => { const engine = engineWith(); - engine.registerFlow('f', flowWith( + const result = await runPatched( + engine, 'get_record', - { objectName: 'crm_lead', limit: 'ten' }, + { objectName: 'crm_lead', limit: 10 }, + { limit: 'ten' }, { nodes: [{ id: 'recover', type: 'assignment', label: 'R', config: { recovered: true } }], edges: [{ id: 'e3', source: 'n1', target: 'recover', type: 'fault' }], }, - )); - - const result = await engine.execute('f'); + ); // Routable would mean success-via-recovery; a guard stays fatal (#3863). expect(result.success).toBe(false); expect(result.error).toContain('does not satisfy the get_record contract'); @@ -180,19 +198,17 @@ describe('execute-time config parse (#4277)', () => { }); it('http still refuses a statically wrong-typed slot', async () => { + expect(doorRefusal('http', { url: 'https://example.test', timeoutMs: 'soon' })).toContain('refused at `timeoutMs`'); const engine = engineWith(); - engine.registerFlow('f', flowWith('http', { url: 'https://example.test', timeoutMs: 'soon' })); - - const result = await engine.execute('f'); + const result = await runPatched(engine, 'http', { url: 'https://example.test', timeoutMs: 1000 }, { timeoutMs: 'soon' }); expect(result.success).toBe(false); expect(result.error).toContain('config.timeoutMs'); }); it('screen refuses an out-of-enum mode instead of silently treating it as create', async () => { + expect(doorRefusal('screen', { objectName: 'crm_lead', mode: 'view' })).toContain('refused at `mode`'); const engine = engineWith(); - engine.registerFlow('f', flowWith('screen', { objectName: 'crm_lead', mode: 'view' })); - - const result = await engine.execute('f'); + const result = await runPatched(engine, 'screen', { objectName: 'crm_lead', mode: 'edit' }, { mode: 'view' }); expect(result.success).toBe(false); expect(result.error).toContain('config.mode'); }); @@ -313,10 +329,9 @@ describe('execute-time config parse (#4277)', () => { }); it('subflow refuses an empty flowName — declared is not the same as named', async () => { + expect(doorRefusal('subflow', { flowName: '' })).toContain('refused at `flowName`'); const engine = engineWith(); - engine.registerFlow('f', flowWith('subflow', { flowName: '' })); - - const result = await engine.execute('f'); + const result = await runPatched(engine, 'subflow', { flowName: 'child' }, { flowName: '' }); expect(result.success).toBe(false); expect(result.error).toContain('config.flowName'); }); diff --git a/packages/services/service-automation/src/builtin/notify-node.test.ts b/packages/services/service-automation/src/builtin/notify-node.test.ts index 7f1b32d32e7..f115c567a22 100644 --- a/packages/services/service-automation/src/builtin/notify-node.test.ts +++ b/packages/services/service-automation/src/builtin/notify-node.test.ts @@ -271,11 +271,15 @@ describe('notify (baseline node)', () => { }); it('refuses a node carrying BOTH template and inline title (the contract superRefine, at the parse seam)', async () => { - engine.registerFlow('notify_flow', notifyFlow({ + // #21898 — refused at registration now, at the key the rule names… + expect(() => engine.registerFlow('notify_flow', notifyFlow({ recipients: ['user_1'], title: 'Deal won', template: 'crm.large_deal_won', - })); + }))).toThrow(/refused at `template`/); + // …and the executor still refuses one that reaches it past the doors. + const stored = engine.registerFlow('notify_flow', notifyFlow({ recipients: ['user_1'], title: 'Deal won' })); + (stored.nodes.find((n) => n.id === 'notify')!.config as Record).template = 'crm.large_deal_won'; const result = await engine.execute('notify_flow'); expect(result.success).toBe(false); expect(result.error).toContain('`template`'); diff --git a/packages/services/service-automation/src/builtin/notify-template-slots.test.ts b/packages/services/service-automation/src/builtin/notify-template-slots.test.ts index da7492f80f8..06ced6ffa1c 100644 --- a/packages/services/service-automation/src/builtin/notify-template-slots.test.ts +++ b/packages/services/service-automation/src/builtin/notify-template-slots.test.ts @@ -114,8 +114,16 @@ describe('notify — title / message are template slots', () => { expect(payload).toMatchObject(RENDERED); }); - it('refuses a value that is neither a string nor a template envelope at the contract parse, before anything is sent', async () => { - const { result } = await deliveredFor({ title: 42 }); + it('refuses a value that is neither a string nor a template envelope — at registration, and at the contract parse past the doors, before anything is sent', async () => { + // The flow parse judges a builtin node's present config value + // against its executor contract, so registration refuses it at the key… + expect(() => engine.registerFlow('notify_template_flow', notifyFlow({ recipients: ['user_1'], title: 42 }))) + .toThrow(/refused at `title`/); + // …and the executor still refuses one that reaches it past the doors. + const stored = engine.registerFlow('notify_template_flow', notifyFlow({ recipients: ['user_1'], title: TITLE })); + const node = stored.nodes.find((n) => n.id === 'notify')!; + node.config = { ...node.config, title: 42 }; + const result = await engine.execute('notify_template_flow', { params: PARAMS } as any); expect(result.success).toBe(false); expect(String(result.error)).toContain('does not satisfy the notify contract'); expect(String(result.error)).toContain('config.title'); diff --git a/packages/spec/src/automation/flow-approval-node-config-contract.test.ts b/packages/spec/src/automation/flow-approval-node-config-contract.test.ts index 2fe35620cc8..7f18e75d0c3 100644 --- a/packages/spec/src/automation/flow-approval-node-config-contract.test.ts +++ b/packages/spec/src/automation/flow-approval-node-config-contract.test.ts @@ -21,7 +21,8 @@ * `validateStackExpressions` calls the same judge. * * No plugin is loaded for any of it: the contract is the spec's own. The - * builtin arm is unchanged, presence-only — a control below holds it there. + * builtin arm judges no key membership — a control below holds it there; its + * value half has its own pins (`flow-builtin-node-config-values.test.ts`). */ import { describe, expect, it } from 'vitest'; @@ -174,7 +175,7 @@ describe('what stays accepted (lit controls)', () => { expect(issuesOf(flowWith({ approvers: APPROVERS }))).toEqual([]); }); - it('CONTROL: the builtin arm stays presence-only — an undeclared key on a builtin node still parses', () => { + it('CONTROL: the builtin arm judges no key membership — an undeclared key on a builtin node still parses', () => { const flow = { ...flowWith(VALID), nodes: [ diff --git a/packages/spec/src/automation/flow-builtin-node-config-values.test.ts b/packages/spec/src/automation/flow-builtin-node-config-values.test.ts new file mode 100644 index 00000000000..9cedf79087d --- /dev/null +++ b/packages/spec/src/automation/flow-builtin-node-config-values.test.ts @@ -0,0 +1,332 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21898] The build doors refuse a VALUE a builtin node's executor contract + * refuses — the value half of the executor-contract arm of + * `flowNodeConfigRefusals`, beside the presence half (#20316) and the declared + * approval contract judged whole (#21850). + * + * Every builtin executor parses its config against the contract in + * `getBuiltinNodeConfigContracts()` before it acts, and refuses the node on any + * finding. A present value the contract refuses — `create_record`'s + * `outputVariable: 42`, a screen field's `min: '1'` — used to pass + * `FlowSchema`, `objectstack validate` and `objectstack compile`, register, and + * then fail every run that reached the node. The pins below hold the refusal at + * every door that parses a flow, and the controls hold what stays accepted: + * + * - a valid node of every builtin type; + * - a `{token}` value in a typed slot, in each spelling an author writes — + * never refused for its pre-interpolation type; + * - the places the build cannot know what the run parses, or another judge + * owns the finding (key membership, a region slot, a `predicate` / `value` + * ledger slot, `http`'s run-resolved `signingSecret`, a legacy `loop` with + * no `body`). + */ + +import { describe, expect, it } from 'vitest'; + +import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas'; +import { MIGRATIONS_BY_MAJOR } from '../migrations/registry'; +import { ArtifactStagePackageBodySchema, ObjectStackDefinitionSchema, defineStack } from '../stack.zod'; +import { flowNodeConfigRefusals, getBuiltinNodeConfigContracts } from './flow-node-config-refusals'; +import { FlowSchema } from './flow.zod'; + +const ENTRY_ID = 'flow-builtin-node-config-values-refused'; + +type Config = Record; + +/** A one-node region body a container probe can hold — node ids are one space across the flow. */ +const region = (id: string) => ({ nodes: [{ id, type: 'assignment', label: 'Inner', config: { assignments: { x: 1 } } }], edges: [] }); +const REGION = region('inner'); + +/** A config each builtin's contract accepts — the accept controls, and the base every probe edits. */ +const VALID: Readonly> = { + get_record: { objectName: 'account', filter: { status: 'open' }, fields: ['name'], limit: 10, outputVariable: 'rows' }, + create_record: { objectName: 'task', fields: { title: 'X' }, outputVariable: 'taskId' }, + update_record: { objectName: 'task', filter: { id: '{record.id}' }, fields: { status: 'done' }, multi: false }, + delete_record: { objectName: 'task', filter: { id: '{record.id}' }, multi: false }, + notify: { recipients: ['u1'], title: 'Hello', severity: 'info', channels: ['inbox'] }, + http: { url: 'https://example.test/hook', method: 'POST', headers: { 'X-Kind': 'probe' }, durable: false, timeoutMs: 5000 }, + screen: { + title: 'How many?', + fields: [{ name: 'qty', label: 'Qty', type: 'number', required: true, min: 1, max: 10 }], + waitForInput: true, + }, + script: { function: 'score_lead', inputs: { leadId: '{record.id}' }, outputVariable: 'score' }, + subflow: { flowName: 'child_flow', input: { id: '{record.id}' }, outputVariable: 'out' }, + map: { collection: '{rows}', flowName: 'per_item', iteratorVariable: 'item' }, + loop: { collection: '{rows}', iteratorVariable: 'row', maxIterations: 50, body: REGION }, + parallel: { branches: [{ name: 'a', ...region('inner_a') }, { name: 'b', ...region('inner_b') }] }, + try_catch: { try: REGION, errorVariable: 'err', retry: { maxRetries: 2, backoffMs: 100 } }, +}; + +/** start → one node of `type` → end. */ +function flowWith(type: string, config: unknown, name = 'value_probe') { + return { + name, + label: 'Value probe', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'n', type, label: 'N', ...(config === undefined ? {} : { config }) }, + { id: 'done', type: 'end', label: 'Done' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'n' }, + { id: 'e2', source: 'n', target: 'done' }, + ], + }; +} + +interface IssueSig { code: string; path: string; message: string } + +function issuesOf(flow: unknown): IssueSig[] { + const r = FlowSchema.safeParse(flow); + return r.success ? [] : r.error.issues.map((i) => ({ code: i.code, path: i.path.join('.'), message: i.message })); +} + +/** The builtin contract's own sentence at one issue path — read, never re-spelled. */ +function contractSentence(type: string, config: unknown, path: string): string { + const own = getBuiltinNodeConfigContracts().get(type)!.schema.safeParse(config); + return own.success ? '' : own.error?.issues.find((i) => i.path.join('.') === path)?.message ?? ''; +} + +describe('the measured pair: refused at save, with its location', () => { + it('create_record with outputVariable 42 is refused at nodes.1.config.outputVariable', () => { + const config = { ...VALID.create_record, outputVariable: 42 }; + const issues = issuesOf(flowWith('create_record', config)); + expect(issues.map(({ code, path }) => ({ code, path }))).toEqual([ + { code: 'custom', path: 'nodes.1.config.outputVariable' }, + ]); + // The judge's own words, carrying the contract's own sentence. + expect(issues[0]!.message).toBe(flowNodeConfigRefusals('create_record', config)[0]!.message); + expect(issues[0]!.message).toContain(contractSentence('create_record', config, 'outputVariable')); + expect(contractSentence('create_record', config, 'outputVariable')).toMatch(/expected string, received number/); + }); + + it('a screen field with a string min is refused at nodes.1.config.fields.0.min', () => { + const config = { ...VALID.screen, fields: [{ name: 'qty', type: 'number', min: '1' }] }; + const issues = issuesOf(flowWith('screen', config)); + expect(issues.map(({ code, path }) => ({ code, path }))).toEqual([ + { code: 'custom', path: 'nodes.1.config.fields.0.min' }, + ]); + expect(issues[0]!.message).toContain(contractSentence('screen', config, 'fields.0.min')); + }); + + it('the judge answers both with the closed-set code, its params and the path', () => { + const refusals = [ + ...flowNodeConfigRefusals('create_record', { ...VALID.create_record, outputVariable: 42 }), + ...flowNodeConfigRefusals('screen', { fields: [{ name: 'qty', min: '1' }] }), + ]; + expect(refusals.map(({ code, params, path, source }) => ({ code, params, path, source }))).toEqual([ + { code: 'node-config-refused-by-contract', params: { nodeType: 'create_record', key: 'outputVariable' }, path: 'outputVariable', source: '' }, + { code: 'node-config-refused-by-contract', params: { nodeType: 'screen', key: 'fields[0].min' }, path: 'fields[0].min', source: '' }, + ]); + }); + + it('a node inside an ADR-0031 region body is refused at the path the author wrote', () => { + const body = { nodes: [{ id: 'mk', type: 'create_record', label: 'Mk', config: { objectName: 'task', outputVariable: 42 } }], edges: [] }; + const issues = issuesOf(flowWith('loop', { collection: '{rows}', body })); + expect(issues.map(({ path }) => path)).toEqual(['nodes.1.config.body.nodes.0.config.outputVariable']); + }); +}); + +describe('one refusal per judged builtin, at each typed key', () => { + // [type, the edit onto its VALID config, the key the refusal names] + const PROBES: ReadonlyArray = [ + ['get_record', { limit: '10' }, 'limit'], + ['get_record', { fields: 'name' }, 'fields'], + ['create_record', { objectName: 5 }, 'objectName'], + ['update_record', { multi: 'true' }, 'multi'], + ['delete_record', { multi: 1 }, 'multi'], + ['notify', { severity: 'loud' }, 'severity'], + ['notify', { recipients: [5] }, 'recipients'], + ['notify', { template: 'crm.deal_won' }, 'template'], + ['http', { timeoutMs: '5000' }, 'timeoutMs'], + ['http', { durable: 'yes' }, 'durable'], + ['http', { headers: { 'X-Kind': 5 } }, 'headers.X-Kind'], + ['screen', { waitForInput: 'yes' }, 'waitForInput'], + ['screen', { mode: 'view' }, 'mode'], + ['screen', { fields: [{ name: 'qty', required: 'yes' }] }, 'fields[0].required'], + ['script', { function: '' }, 'function'], + ['script', { inputs: 'leadId' }, 'inputs'], + ['subflow', { flowName: '' }, 'flowName'], + ['subflow', { input: ['x'] }, 'input'], + ['map', { flowName: 7 }, 'flowName'], + ['map', { collection: 5 }, 'collection'], + ['loop', { collection: 5 }, 'collection'], + ['loop', { maxIterations: 0 }, 'maxIterations'], + ['try_catch', { errorVariable: 5 }, 'errorVariable'], + ['try_catch', { retry: { maxRetries: 99 } }, 'retry.maxRetries'], + ]; + + for (const [type, edit, key] of PROBES) { + it(`${type} ${JSON.stringify(edit)} is refused at ${key}`, () => { + const config = { ...VALID[type], ...edit }; + const refusals = flowNodeConfigRefusals(type, config); + expect(refusals.map(({ code, path }) => ({ code, path }))).toEqual([{ code: 'node-config-refused-by-contract', path: key }]); + expect(refusals[0]!.params).toEqual({ nodeType: type, key }); + expect(issuesOf(flowWith(type, config)).map(({ path }) => path)).toHaveLength(1); + }); + } + + it('a rule finding carries the rule\'s own words and no closing instruction', () => { + const config = { ...VALID.notify, template: 'crm.deal_won' }; + const [refusal] = flowNodeConfigRefusals('notify', config); + expect(refusal!.message).toContain(contractSentence('notify', config, 'template')); + expect(refusal!.message).not.toMatch(/Write a value the notify contract accepts/); + }); + + it('a type finding closes on what to write at the key', () => { + const [refusal] = flowNodeConfigRefusals('create_record', { ...VALID.create_record, outputVariable: 42 }); + expect(refusal!.message).toMatch(/Write a value the create_record contract accepts at `outputVariable`\.$/); + }); +}); + +describe('what stays accepted (lit controls)', () => { + it('CONTROL: a valid node of every builtin type parses, and the judge says nothing', () => { + expect(Object.keys(VALID).sort()).toEqual([...getBuiltinNodeConfigContracts().keys()].sort()); + for (const [type, config] of Object.entries(VALID)) { + expect(getBuiltinNodeConfigContracts().get(type)!.schema.safeParse(config).success, type).toBe(true); + expect(flowNodeConfigRefusals(type, config), type).toEqual([]); + expect(issuesOf(flowWith(type, config)), type).toEqual([]); + } + }); + + it('CONTROL: a token value in a typed slot is never refused for its pre-interpolation type', () => { + const TOKENS: ReadonlyArray = [ + ['get_record', { limit: '{page.size}' }], + ['http', { timeoutMs: '{timeout}' }], + ['http', { durable: '{{durable_flag}}' }], + ['http', { headers: '{headers}' }], + ['screen', { fields: [{ name: 'qty', min: '${minimum}', max: '{maximum}' }] }], + ['update_record', { multi: '{bulk}' }], + ['loop', { maxIterations: '{cap}' }], + ['try_catch', { retry: { maxRetries: '{tries}' } }], + ]; + for (const [type, edit] of TOKENS) { + const config = { ...VALID[type], ...edit }; + // The contract itself refuses the string — only the judge holds back. + expect(getBuiltinNodeConfigContracts().get(type)!.schema.safeParse(config).success, JSON.stringify(edit)).toBe(false); + expect(flowNodeConfigRefusals(type, config), JSON.stringify(edit)).toEqual([]); + expect(issuesOf(flowWith(type, config)), JSON.stringify(edit)).toEqual([]); + } + }); + + it('CONTROL: a token-free sibling does not shield a refused value (http judges each slot on its own)', () => { + const config = { ...VALID.http, url: 'https://example.test/{path}', timeoutMs: '5000' }; + expect(flowNodeConfigRefusals('http', config).map(({ path }) => path)).toEqual(['timeoutMs']); + }); + + it('CONTROL: key membership is not this arm\'s — an undeclared key and a tombstoned one', () => { + expect(flowNodeConfigRefusals('http', { ...VALID.http, bogusKey: 1 })).toEqual([]); + expect(flowNodeConfigRefusals('screen', { fields: [{ name: 'qty', visible: 'x' }] })).toEqual([]); + expect(flowNodeConfigRefusals('script', { ...VALID.script, actionType: 'email' })).toEqual([]); + expect(flowNodeConfigRefusals('try_catch', { ...VALID.try_catch, retry: { retryDelayMs: 500 } })).toEqual([]); + // CONTROL: the contracts refuse all four — the arm holds back, the contract does not. + for (const [type, config] of [ + ['http', { ...VALID.http, bogusKey: 1 }], + ['screen', { fields: [{ name: 'qty', visible: 'x' }] }], + ['script', { ...VALID.script, actionType: 'email' }], + ['try_catch', { ...VALID.try_catch, retry: { retryDelayMs: 500 } }], + ] as const) { + expect(getBuiltinNodeConfigContracts().get(type)!.schema.safeParse(config).success, type).toBe(false); + } + }); + + it('CONTROL: a slot another judge owns is not re-judged here', () => { + // A `predicate` ledger slot: `predicateSlotRefusal` judges it at the other two doors. + expect(flowNodeConfigRefusals('screen', { fields: [{ name: 'qty', visibleWhen: { dialect: 'cel', source: 'x' } }] })).toEqual([]); + // A `value` ledger slot: the value-envelope pass judges it. + expect(flowNodeConfigRefusals('create_record', { objectName: 'task', fields: { title: { dialect: 'cel', source: 1 } } })).toEqual([]); + // A region slot: `validateControlFlow` judges a region's shape at registration. + expect(flowNodeConfigRefusals('try_catch', { try: 5 })).toEqual([]); + expect(flowNodeConfigRefusals('parallel', { branches: [{ name: 'a', ...REGION }] })).toEqual([]); + // http's signing secret: the credential channel may hold the value the run parses. + expect(flowNodeConfigRefusals('http', { ...VALID.http, signingSecret: 7 })).toEqual([]); + }); + + it('CONTROL: a legacy loop with no body is not parsed by its executor, so nothing is judged', () => { + expect(getBuiltinNodeConfigContracts().get('loop')!.parsedWhen!({ collection: 5 })).toBe(false); + expect(flowNodeConfigRefusals('loop', { collection: 5, maxIterations: 0 })).toEqual([]); + }); + + it('CONTROL: a key left out keeps its presence code beside a refused value', () => { + const refusals = flowNodeConfigRefusals('create_record', { outputVariable: 42 }); + expect(refusals.map(({ code, path }) => ({ code, path }))).toEqual([ + { code: 'node-config-key-missing', path: 'objectName' }, + { code: 'node-config-refused-by-contract', path: 'outputVariable' }, + ]); + }); +}); + +describe('every door that parses a flow refuses it', () => { + const stackWith = (flows: unknown[]) => ({ + manifest: { id: 'com.example.values', name: 'values', version: '1.0.0', type: 'app', namespace: 'val' }, + objects: [{ name: 'val_task', label: 'Task', fields: { title: { type: 'text', label: 'Title' } } }], + flows, + }); + const refused = flowWith('create_record', { objectName: 'val_task', outputVariable: 42 }, 'val_refused'); + const screenRefused = flowWith('screen', { fields: [{ name: 'qty', type: 'number', min: '1' }] }, 'val_screen'); + const accepted = flowWith('create_record', { objectName: 'val_task', outputVariable: 'taskId' }, 'val_ok'); + + it('defineStack wraps the refusal in its ADR-0112 envelope, at flows.1.nodes.1.config.outputVariable', () => { + let refusal: { code?: unknown; status?: unknown; issues?: Array<{ path: unknown[]; code: string }> } | undefined; + try { + defineStack(stackWith([accepted, refused]) as never); + } catch (e) { + refusal = e as typeof refusal; + } + expect(refusal, 'defineStack must refuse the flow').toBeDefined(); + expect({ code: refusal!.code, status: refusal!.status }).toEqual({ code: 'STACK_SCHEMA_INVALID', status: 422 }); + expect(refusal!.issues!.map((i) => ({ path: i.path.join('.'), code: i.code }))).toEqual([ + { path: 'flows.1.nodes.1.config.outputVariable', code: 'custom' }, + ]); + }); + + it('CONTROL: defineStack accepts the valid flow alone', () => { + expect(() => defineStack(stackWith([accepted]) as never)).not.toThrow(); + }); + + it('ObjectStackDefinitionSchema — the stack parse validate and compile run — refuses both pins', () => { + const r = ObjectStackDefinitionSchema.safeParse(stackWith([refused, screenRefused])); + expect(r.success).toBe(false); + expect(r.success ? [] : r.error.issues.map((i) => i.path.join('.'))).toEqual([ + 'flows.0.nodes.1.config.outputVariable', + 'flows.1.nodes.1.config.fields.0.min', + ]); + expect(ObjectStackDefinitionSchema.safeParse(stackWith([accepted])).success).toBe(true); + }); + + it('the registered `flow` type schema — what the metadata save door validates against — refuses it too', () => { + const schema = getMetadataTypeSchema('flow') as unknown as typeof FlowSchema; + expect(schema).toBeDefined(); + const r = schema.safeParse(screenRefused); + expect(r.success).toBe(false); + expect(r.success ? [] : r.error.issues.map((i) => i.path.join('.'))).toEqual(['nodes.1.config.fields.0.min']); + expect(schema.safeParse(accepted).success).toBe(true); + }); + + it('an artifact\'s parse refuses it', () => { + const body = { id: 'com.example.values', name: 'values', version: '1.0.0', type: 'app' }; + const r = ArtifactStagePackageBodySchema.safeParse({ ...body, flows: [refused] }); + expect(r.success).toBe(false); + expect(r.success ? [] : r.error.issues.map((i) => i.path.join('.'))).toEqual(['flows.0.nodes.1.config.outputVariable']); + const ok = ArtifactStagePackageBodySchema.safeParse({ ...body, flows: [accepted] }); + expect(ok.success, JSON.stringify(ok.error?.issues ?? [])).toBe(true); + }); +}); + +describe('the ADR-0087 ledger', () => { + it('registers one D3 entry at protocol 18, with no D2 conversion', () => { + const entries = MIGRATIONS_BY_MAJOR[18]!.semantic.filter((e) => e.id === ENTRY_ID); + expect(entries, 'the narrowing needs its own D3 entry').toHaveLength(1); + const [entry] = entries; + expect(entry!.conversionIds ?? []).toEqual([]); + expect(entry!.acceptanceCriteria.length).toBeGreaterThan(0); + }); + + it('names the step-18 rationale fragment for it', () => { + expect(MIGRATIONS_BY_MAJOR[18]!.rationale).toContain(ENTRY_ID); + }); +}); diff --git a/packages/spec/src/automation/flow-node-config-refusals.ts b/packages/spec/src/automation/flow-node-config-refusals.ts index f915eb58bcd..01e47e2d7e3 100644 --- a/packages/spec/src/automation/flow-node-config-refusals.ts +++ b/packages/spec/src/automation/flow-node-config-refusals.ts @@ -11,7 +11,10 @@ * open `z.record`, so what its executor requires, or refuses, was checked by * nobody until the run. And (#21850) **the whole config of a plugin node type * whose contract the spec itself declares** — `approval`, judged against - * `ApprovalNodeConfigSchema` with no plugin loaded. + * `ApprovalNodeConfigSchema` with no plugin loaded. And (#21898) **a value a + * builtin node's executor contract refuses**, where the build can know what + * the run will parse — see {@link flowNodeConfigRefusals}' first arm for what + * that excludes, and why. * * Its refusal codes join the closed flow slot table * (`FLOW_SLOT_REFUSAL_CODES`, `flow-node-expression-paths.ts`); the @@ -132,11 +135,12 @@ let cachedDeclaredPluginNodeConfigContracts: ReadonlyMap): string { /** * Is the key an issue names ABSENT from the authored config — its parent - * reached, and the key itself not there (or `undefined`)? The one question - * the contract arm asks: a present value of the wrong type is a different - * finding, and not this judge's. + * reached, and the key itself not there (or `undefined`)? The contract arm's + * first question: a key left out is `node-config-key-missing` (or the rule's + * own code), and a present value is a different finding — refused, for a + * builtin, only where {@link builtinValueJudged} holds (#21898). */ function absentAt(config: Readonly>, path: ReadonlyArray): boolean { let parent: unknown = config; @@ -228,6 +233,135 @@ function insideValueSlot(nodeType: string, path: ReadonlyArray): bo }); } +// ─── The builtin VALUE arm (#21898) ─────────────────────────────────── + +/** + * [#21898] A `{token}` the run interpolates — the interpolator's own token + * shape (`service-automation` `builtin/template.ts`, `interpolateString`, and + * the lint's `TEMPLATE_TOKEN_RE`), which also matches the double-brace and + * dollar-brace spellings an author may write. A string carrying one means + * something only after interpolation, so the build never judges its type. + */ +const INTERPOLATION_TOKEN = /\{[^{}]+\}/; + +/** + * [#21898] Does `value`, or anything inside it, carry a {@link + * INTERPOLATION_TOKEN}? Cycle-safe: a flow arriving as hand-built objects + * rather than parsed JSON may hold a self-reference. + */ +function carriesInterpolationToken(value: unknown, seen: Set = new Set()): boolean { + if (typeof value === 'string') return INTERPOLATION_TOKEN.test(value); + if (value === null || typeof value !== 'object' || seen.has(value)) return false; + seen.add(value); + return (Array.isArray(value) ? value : Object.values(value)).some((v) => carriesInterpolationToken(v, seen)); +} + +/** The authored value an issue path names, or `undefined` when the walk leaves the config. */ +function authoredAt(config: Readonly>, path: ReadonlyArray): unknown { + let at: unknown = config; + for (const segment of path) { + if (at === null || typeof at !== 'object') return undefined; + at = (at as Record)[segment as PropertyKey]; + } + return at; +} + +/** + * [#21898] The builtin node types whose executor parses its contract AFTER + * interpolating the whole config — today `http` alone (`builtin/http-nodes.ts`: + * `parseNodeConfig(…, interpolate(raw, …))`). Its contract describes the + * INTERPOLATED shape, so a `{token}` anywhere may change what the contract + * sees — a sole token resolves to its value's real type — and a rule, which may + * read keys other than the one its issue names, is judged only on a config that + * carries no token at all. Every other builtin parses its config as authored. + */ +const PARSED_AFTER_INTERPOLATION: ReadonlySet = new Set(['http']); + +/** + * [#21898] Builtin config keys whose run-time value may not be the authored + * one: `http.signingSecret` — a secret the write-only flow credential channel + * holds takes the literal's place before the parse (`builtin/http-nodes.ts`), + * so the build cannot know what the contract will see there. + */ +const RUN_RESOLVED_KEYS: Readonly> = { http: ['signingSecret'] }; + +/** The ledger roles whose slots another judge owns — never re-judged by the value arm. */ +const LEDGER_JUDGED_ROLES: ReadonlySet = new Set(['predicate', 'value']); + +/** A ledger path (`fields[].visibleWhen`, `fields.*`) → match segments: `[]` any index, `*` any key. */ +function ledgerSlotSegments(path: string): string[] { + const segments: string[] = []; + for (const part of path.split('.')) { + if (part.endsWith('[]')) segments.push(part.slice(0, -2), '[]'); + else segments.push(part); + } + return segments; +} + +/** + * [#21898] Does an issue path sit AT or INSIDE a `predicate` or `value` slot the + * expression ledger declares for this node type? Those slots have judges of + * their own: `predicateSlotRefusal` (whose non-strings the flow parse leaves to + * `registerFlow` and `objectstack validate` by ruling — `flow.zod.ts`), and the + * value-envelope pass. + */ +function atOrInsideJudgedLedgerSlot(nodeType: string, path: ReadonlyArray): boolean { + return FLOW_NODE_EXPRESSION_PATHS.some((entry) => { + if (entry.nodeType !== nodeType || !LEDGER_JUDGED_ROLES.has(entry.role)) return false; + const segments = ledgerSlotSegments(entry.path); + if (path.length < segments.length) return false; + return segments.every((segment, i) => + segment === '*' ? typeof path[i] === 'string' + : segment === '[]' ? typeof path[i] === 'number' + : segment === path[i]); + }); +} + +/** One contract issue, as much of it as the value arm reads. */ +interface ContractIssue { + readonly code: string; + readonly path: ReadonlyArray; + readonly message: string; +} + +/** + * [#21898] Is a builtin contract's issue at a PRESENT key a value the build can + * refuse — one the run would refuse for the same reason? Every carve-out is a + * place where the build does not know what the run will parse, or where + * another judge owns the finding: + * + * - key membership — an undeclared key (`unrecognized_keys`) and a tombstoned + * one (a `retiredKey()`, an `invalid_type` expecting `never`): registration + * refuses an undeclared key against the descriptor with its own + * prescriptions, the lint names the retired script keys, and the conversion + * layer rewrites a retired spelling before the two doors that convert first; + * - an ADR-0031 region slot, the slot itself included (`try: 5`): a region's + * shape is `validateControlFlow`'s, and its nodes are the region walk's; + * - a `predicate` or `value` ledger slot, at or inside it + * ({@link atOrInsideJudgedLedgerSlot}); + * - a run-resolved key ({@link RUN_RESOLVED_KEYS}); + * - a value carrying a `{token}` anywhere inside it: it is never refused for + * its pre-interpolation type, even where the executor parses the authored + * config and so refuses it at the run; + * - for a contract parsed after interpolation ({@link + * PARSED_AFTER_INTERPOLATION}), a rule (`custom`) when the config carries a + * token anywhere. + */ +function builtinValueJudged( + nodeType: string, + authored: Readonly>, + issue: ContractIssue, +): boolean { + if (issue.code === 'unrecognized_keys') return false; + if (issue.code === 'invalid_type' && (issue as { readonly expected?: unknown }).expected === 'never') return false; + if ((FLOW_REGION_SLOTS_BY_TYPE.get(nodeType) ?? []).some((slot) => slot.key === issue.path[0])) return false; + if (atOrInsideJudgedLedgerSlot(nodeType, issue.path)) return false; + if ((RUN_RESOLVED_KEYS[nodeType] ?? []).includes(issue.path[0] as string)) return false; + if (carriesInterpolationToken(authoredAt(authored, issue.path))) return false; + if (PARSED_AFTER_INTERPOLATION.has(nodeType) && issue.code === 'custom' && carriesInterpolationToken(authored)) return false; + return true; +} + /** * [#21654] The write nodes, each with the verb its refusal names — the same * three, in the same words, as the run-time refusal in `service-automation` @@ -316,26 +450,24 @@ function unrecognizedKeysOf(issue: { readonly code: string }): readonly string[] } /** - * Every reason a node's `config` is refused on SHAPE or PRESENCE, and (#21654) - * a write node's static TARGET in the stored-metadata family — the ONE judge - * `FlowSchema.parse`, `AutomationEngine.registerFlow` (which parses first) and - * `objectstack validate` share (#20316). + * Every reason a node's `config` is refused on SHAPE, PRESENCE or (#21898) + * VALUE, and (#21654) a write node's static TARGET in the stored-metadata + * family — the ONE judge `FlowSchema.parse`, `AutomationEngine.registerFlow` + * (which parses first) and `objectstack validate` share (#20316). * * Three arms. * - * ## The executor contract — a key it requires, absent + * ## The executor contract — a key it requires absent, or a value it refuses * * For a type in {@link getBuiltinNodeConfigContracts}, the config is parsed * against the executor's own contract, on the executor's own condition * (`config ?? {}`, as `parseNodeConfig` reads it; `loop` only with a `body`), - * and a failure is kept ONLY where the key it names is absent from what was - * authored. That keeps the judge to one question — "would the run refuse this - * node for a key it leaves out?" — and leaves every other contract finding - * (a present value of the wrong type, an undeclared key) where it lives - * today. Issues inside an ADR-0031 region are the region's own and skipped, - * and so are issues inside a ledger `value` slot (`fields.*`, - * `assignments.*`): a key missing inside an authored value is a malformed - * value, refused by the value-envelope pass, not a config key left out. + * and a failure is kept where it answers one question: "would the run refuse + * this node, for this reason, whatever it is handed?" Issues inside an + * ADR-0031 region are the region's own and skipped, and so are issues inside + * a ledger `value` slot (`fields.*`, `assignments.*`): a key missing inside an + * authored value is a malformed value, refused by the value-envelope pass, not + * a config key left out. * * - A key the contract simply requires → `node-config-key-missing`, whose * message names the key and the node type. @@ -343,6 +475,31 @@ function unrecognizedKeysOf(issue: { readonly code: string }): readonly string[] * with no `template` needs `title`; a `lookup` screen field needs its * `reference`) → `node-config-key-required-by-rule`, whose message is the * contract's own. + * - (#21898) A value the contract refuses at a key the author wrote → + * `node-config-refused-by-contract`, anchored at the key + * (`outputVariable` for a `create_record` `42`; `fields[0].min` for a + * screen field's `'1'`), the same code and message the declared plugin + * contract below uses. Kept only where {@link builtinValueJudged} holds — + * key MEMBERSHIP is not judged here (an undeclared key, a tombstoned one), + * nor a region slot, a `predicate` / `value` ledger slot, a run-resolved + * key, or any value carrying a `{token}`: never refused for its + * pre-interpolation type. + * + * Where the build cannot read the config whole, it reads only what is sound, + * and each such type is named here, not skipped in silence: + * + * - `http` parses AFTER interpolating its whole config, so a value is judged + * only when nothing inside it carries a `{token}` (its interpolation is then + * the identity), a rule only when the whole config carries none, and + * `signingSecret` never (the credential channel may supply it); + * - `loop` parses only with a `body` (`parsedWhen`), so a legacy flat-graph + * loop is judged for nothing, and a loop with a body on its own keys + * (`collection`, `iteratorVariable`, `indexVariable`, `maxIterations`); + * - the region containers — `loop` (`body`), `parallel` (`branches`) and + * `try_catch` (`try`, `catch`) — hold regions, judged as graphs of their + * own; a container is judged on the keys beside them (`try_catch`'s + * `errorVariable` and `retry`), so `parallel`, whose one key is its region + * slot, is judged for presence alone. * * A key only the conversion layer spells canonically (`object` → * `objectName`, `flow` → `flowName`, …) is judged AFTER the conversion at @@ -363,7 +520,9 @@ function unrecognizedKeysOf(issue: { readonly code: string }): readonly string[] * type and the key around the contract's own sentence — for an unknown key, * its did-you-mean (`timeout` → `timeoutHours`). * - * The builtin arm stays presence-only: nothing here widens what it judges. + * Whole, where the builtin arm is not: no carve-out above applies to it — + * the approval executor parses its authored config, with no interpolation and + * no region, before it does anything else. * * ## The decision branch shape * @@ -446,7 +605,9 @@ export function flowNodeConfigRefusals(nodeType: string, config: unknown): FlowN if (insideRegion(nodeType, issue.path)) continue; if (insideValueSlot(nodeType, issue.path)) continue; const absent = absentAt(authored, issue.path); - if (!absent && !whole) continue; + // [#21898] A present builtin value: refused where the build knows what the + // run will parse — see `builtinValueJudged`. + if (!absent && !whole && !builtinValueJudged(nodeType, authored, issue)) continue; const key = ledgerPathOf(issue.path); if (seen.has(key)) continue; seen.add(key); diff --git a/packages/spec/src/automation/flow-node-config-required.test.ts b/packages/spec/src/automation/flow-node-config-required.test.ts index 6013b465850..9322ab3e43b 100644 --- a/packages/spec/src/automation/flow-node-config-required.test.ts +++ b/packages/spec/src/automation/flow-node-config-required.test.ts @@ -160,9 +160,15 @@ describe('FlowSchema.parse refuses a key the node\'s executor contract requires, expect(issuesOf(flowWith(node('loop')))).toEqual([]); }); - it('a PRESENT value of the wrong type is not this rule\'s finding — absence only', () => { - expect(issuesOf(flowWith(node('get_record', { objectName: 42 })))).toEqual([]); - expect(issuesOf(flowWith(node('script', { function: '' })))).toEqual([]); + it('a PRESENT value of the wrong type is not this rule\'s finding — the value arm refuses it under its own code', () => { + // #21898: present values are judged since, by `node-config-refused-by-contract` + // (`flow-builtin-node-config-values.test.ts`); this rule still reports absence only. + expect(flowNodeConfigRefusals('get_record', { objectName: 42 }).map(({ code, path }) => ({ code, path }))).toEqual([ + { code: 'node-config-refused-by-contract', path: 'objectName' }, + ]); + expect(flowNodeConfigRefusals('script', { function: '' }).map(({ code, path }) => ({ code, path }))).toEqual([ + { code: 'node-config-refused-by-contract', path: 'function' }, + ]); }); it('a key missing INSIDE an authored value is not a config key left out — the value-envelope pass owns it', () => { diff --git a/packages/spec/src/automation/flow-write-node-stored-metadata-target.test.ts b/packages/spec/src/automation/flow-write-node-stored-metadata-target.test.ts index ba1e7ea5c03..491d67bbef4 100644 --- a/packages/spec/src/automation/flow-write-node-stored-metadata-target.test.ts +++ b/packages/spec/src/automation/flow-write-node-stored-metadata-target.test.ts @@ -125,10 +125,17 @@ describe('what stays accepted at save (lit controls)', () => { }); it.each(WRITE_NODES)('CONTROL: %s with a dynamic objectName — a {token} template or an expression envelope — is not judged at save', (nodeType) => { - for (const objectName of ['{record.target}', '{target}', { dialect: 'cel', source: "'sys_metadata'" }]) { + for (const objectName of ['{record.target}', '{target}']) { expect(issuesOf(flowWith({ type: nodeType, config: configFor(nodeType, objectName) })), JSON.stringify(objectName)).toEqual([]); expect(flowNodeConfigRefusals(nodeType, configFor(nodeType, objectName))).toEqual([]); } + // #21898: an envelope is no name this arm can read, and no value the node's + // contract accepts (`objectName` is a string): the value arm refuses it, + // and this arm still says nothing. + const envelope = { dialect: 'cel', source: "'sys_metadata'" }; + expect(flowNodeConfigRefusals(nodeType, configFor(nodeType, envelope)).map(({ code, path }) => ({ code, path }))).toEqual([ + { code: 'node-config-refused-by-contract', path: 'objectName' }, + ]); }); it.each(FAMILY)('CONTROL: a get_record node on \'%s\' is not judged by this arm — a read is not a write', (table) => { diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 17df6773920..e070c2ec6c0 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -167,7 +167,9 @@ export const FLOW_PAUSE_CAPABLE_NODE_TYPES: readonly string[] = [ * rule reaches in beside it, still without closing the key set (#20316): a * key the node's executor contract requires, left out, and a `decision` * branch list the executor cannot read — `flowNodeConfigRefusals`, in the - * same superRefine. + * same superRefine — and (#21898) a VALUE rule with it, again without closing + * the key set: a value a builtin node's executor contract refuses, where the + * build can know what the run will parse. */ /** @@ -1474,19 +1476,23 @@ export const FlowSchema = lazySchema(() => strictObject( // node against at run time, so a flow carrying one used to register and // then fail every run that reached the node (`loop` with a `body` and no // `collection`, `map` with no `collection`, a CRUD node with no - // `objectName`, …). For a builtin only ABSENCE is judged: a present value - // of the wrong type, or an undeclared key, stays where it is judged today. - // The one plugin node contract the spec declares, `approval` (#21850), is - // judged WHOLE — its executor refuses the node on any contract finding — - // so its undeclared keys and refused values are refused here too; + // `objectName`, …). For a builtin (#21898) a present VALUE its contract + // refuses is refused too (`create_record` `outputVariable: 42`, a screen + // field `min: '1'`), where the build can know what the run parses — never + // a value carrying a `{token}`, a region slot, a ledger predicate or + // value slot, or `http`'s run-resolved `signingSecret`; an undeclared key + // stays where it is judged today. The one plugin node contract the spec + // declares, `approval` (#21850), is judged WHOLE — its executor refuses + // the node on any contract finding — so its undeclared keys are refused + // here too; // - a `decision` branch list the executor cannot read — `conditions` not an // array, a branch that is not an object, and a branch whose `label` is // absent, blank or not text. The last one never failed a run at all: the // matched branch reported no label, and traversal took EVERY out-edge. // - // A PRESENCE rule for a builtin, never a key-set closure: the node `config` - // stays the open record the header of this module describes, and only the - // approval node's declared contract closes its key set. Walked with + // A PRESENCE and VALUE rule for a builtin, never a key-set closure: the node + // `config` stays the open record the header of this module describes, and + // only the approval node's declared contract closes its key set. Walked with // `collectFlowGraphs`, so a node inside an ADR-0031 region body is judged at // the path the author wrote; a container's own judgement skips its regions' // insides, which this same walk reaches as graphs of their own. diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-values-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-values-refused.ts new file mode 100644 index 00000000000..c20240aed57 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-values-refused.ts @@ -0,0 +1,81 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// #21898 — the D3 entry for the build doors refusing a VALUE a builtin flow +// node's executor contract refuses: the value half of the executor-contract arm +// of `flowNodeConfigRefusals` (`flow-node-config-refusals.ts`), beside the +// presence half (`flow-node-config-required-keys-refused`) and the approval +// contract judged whole (`flow-approval-node-config-contract-refused`). It +// narrows a flow's accept set; no key is removed, so there is no tombstone and +// no RETIRED_KEYS_BY_MAJOR row. There is no D2 conversion either: the platform +// cannot know the value the author meant, and the runtime never ran such a node. +// +// No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it +// inside a code span and a table cell. +export const entry: SemanticMigration = { + id: 'flow-builtin-node-config-values-refused', + surface: + 'a builtin flow node (get_record, create_record, update_record, delete_record, notify, http, screen, ' + + 'script, subflow, map, loop, parallel, try_catch) whose config carries a value its executor ' + + 'contract refuses — a value of the wrong type (create_record outputVariable 42, a screen field min ' + + 'written as the string 1, get_record limit as a string, update_record multi as a string), a value ' + + 'outside the declared set or range (notify severity loud, screen mode view, loop maxIterations 0, ' + + 'try_catch retry.maxRetries above 10), an empty script function or subflow flowName, or a rule ' + + 'finding on present keys (a notify template beside an inline title). Never a value carrying a ' + + 'token in braces, an undeclared or retired key, a region slot, or an http signingSecret. ' + + 'Reachable wherever a flow is authored or stored: defineStack({ flows }) sources, defineFlow(), an ' + + 'exported stack passed to objectstack validate or objectstack compile, a flow saved from the Studio ' + + 'flow designer, and a flow row already sitting in sys_metadata', + replacement: + 'the value the contract declares, written at the key the refusal names: a string where it wants a ' + + 'string (`outputVariable: \'taskId\'`), a number where it wants a number (`min: 1`, `limit: 10`, ' + + '`maxIterations: 5`, `timeoutMs: 5000`), a boolean where it wants a boolean (`multi: true`, ' + + '`durable: true`), one of the declared values (`severity: \'warning\'`, `mode: \'edit\'`), or a value ' + + 'inside the declared range. Outside `http`, a number or boolean slot takes a LITERAL only: those ' + + 'executors parse the config as authored, so a `{token}` template there (`limit: \'{page.size}\'`, ' + + '`maxIterations: \'{cap}\'`) passes the build doors and still fails every run. Only `http` ' + + 'interpolates its config before it parses, so only an `http` slot may also take a sole-token template ' + + 'that resolves to the declared type (`timeoutMs: \'{timeout}\'`, `durable: \'{durable}\'`). For a rule ' + + 'finding, follow the rule\'s own sentence (keep `template` or the inline `title` / `message`, not both)', + reason: + 'Every builtin executor (`service-automation` `builtin/`) parses its node\'s `config` against the ' + + 'contract `getBuiltinNodeConfigContracts()` names before it acts, and refuses the node on any ' + + 'finding. The build doors judged only the keys that contract requires, left out, so a present value ' + + 'it refuses — `create_record` `outputVariable: 42`, a screen field `min: \'1\'` (the shape the Studio ' + + 'designer used to store for a field\'s Min / Max) — passed `FlowSchema.parse`, `objectstack ' + + 'validate` and `objectstack compile`, registered, and then failed every run that reached the node: ' + + 'the config is metadata, and no rerun could succeed. The one judge `FlowSchema.parse`, ' + + '`AutomationEngine.registerFlow` (which parses first) and `objectstack validate` share ' + + '(`flowNodeConfigRefusals`) now refuses such a value as `node-config-refused-by-contract`, anchored ' + + 'at the key, in the contract\'s own words — the code the approval contract already uses. It judges ' + + 'only what the build can know the run will parse, and holds one more class back by ruling: a value ' + + 'carrying a `{token}` is never refused at the build doors for its pre-interpolation type — which is ' + + 'no promise it runs, since every builtin but `http` parses its config as authored and so still ' + + 'refuses a token in a number or boolean slot at its first run; `http` parses after interpolating its ' + + 'whole config, so only token-free ' + + 'values are judged there and never `signingSecret`, which the credential channel may supply; a ' + + '`loop` with no `body` is not parsed by its executor and is judged for nothing; the region slots of ' + + '`loop`, `parallel` and `try_catch` are judged as graphs of their own and by `validateControlFlow`. ' + + 'An undeclared or retired key, a `predicate` ledger slot (a screen field `visibleWhen`) and a `value` ' + + 'ledger slot (a CRUD `fields` value) keep the judges they had. ⚠️ No D2 conversion: the platform ' + + 'cannot know the value the author meant. ⚠️ Where such a node already sits the whole flow is ' + + 'refused: registered from the metadata registry or `sys_metadata` at boot it is skipped with a ' + + '`warn` naming it, its trigger not armed, while the flows beside it register; a ' + + '`defineStack({ flows })` source throws `StackSchemaInvalidError` for the whole stack; an artifact ' + + 'file is refused whole at load. ADR-0087, ADR-0031.', + acceptanceCriteria: + 'Run `objectstack validate` over every stack authored in config files, and boot every deployed ' + + 'stack. Each refusal names the node and the key: `FlowSchema.parse` anchors a `custom` issue at ' + + '`nodes.N.config.` (`nodes.N.config.outputVariable`, `nodes.N.config.fields.0.min`, or the ' + + 'region path `nodes.N.config.body.nodes.M.config…`), `objectstack validate` prints the same path, ' + + 'and `validateStackExpressions` phrases it as `node \'mk\' (create_record) config.outputVariable`. ' + + 'For each hit write the value the contract declares, per the replacement. Two proofs. (1) For a ' + + 'stack authored in config files, `objectstack validate` is clean. (2) Boot the stack and confirm ' + + 'each flow REGISTERS: no `failed to register flow` warn for it — that warn line is the locator for ' + + 'a row that exists only in `sys_metadata`. A node whose values its contract accepts parses and ' + + 'registers byte-identically to before. A `{token}` template parses and registers as before too, and ' + + 'runs only where the run parses it after interpolation (`http`) or where the slot takes a string; in a ' + + 'number or boolean slot of any other builtin it fails at its first run exactly as it did, so write a ' + + 'literal there.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 7fbbfba7c48..50ef39e36ac 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5595,6 +5595,25 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ + 'the platform cannot know what the author meant. Its D3 record is the semantic entry ' + '`flow-approval-node-config-contract-refused`.', }, + { + id: 'flow-builtin-node-config-values-refused', + order: 85, + text: + 'Then the builtin arm stops being presence-only: a present value a builtin node\'s executor ' + + 'contract refuses is refused at parse, at `nodes.N.config.`, with the same ' + + '`node-config-refused-by-contract` code. Every builtin executor parses its config against that ' + + 'contract before it acts, so a `create_record` `outputVariable: 42` or a screen field `min: \'1\'` ' + + 'used to pass `objectstack validate` and `objectstack compile`, register, and fail every run that ' + + 'reached the node. The arm judges only what the build can know the run will parse: never a value ' + + 'carrying a `{token}`, whatever its slot\'s type (held back by ruling, not admitted: outside `http` ' + + 'such a token in a number or boolean slot still fails at its first run, so those slots take a ' + + 'literal); on `http`, which parses after interpolating, only ' + + 'token-free values and never the credential-held `signingSecret`; on a `loop`, only one with a ' + + '`body`; on the region containers, never the region slots. Key membership is untouched. No key ' + + 'is removed, so there is no tombstone, and no D2 conversion exists: the platform cannot know the ' + + 'value the author meant. Its D3 record is the semantic entry ' + + '`flow-builtin-node-config-values-refused`.', + }, { id: 'flow-decision-edge-branching-first-match', order: 45, @@ -13139,6 +13158,83 @@ const step18: MigrationStep = { + 'row that exists only in `sys_metadata`. An approval node the contract accepts parses and ' + 'registers byte-identically to before.', }, + // #21898 — the D3 entry for the build doors refusing a VALUE a builtin flow + // node's executor contract refuses: the value half of the executor-contract arm + // of `flowNodeConfigRefusals` (`flow-node-config-refusals.ts`), beside the + // presence half (`flow-node-config-required-keys-refused`) and the approval + // contract judged whole (`flow-approval-node-config-contract-refused`). It + // narrows a flow's accept set; no key is removed, so there is no tombstone and + // no RETIRED_KEYS_BY_MAJOR row. There is no D2 conversion either: the platform + // cannot know the value the author meant, and the runtime never ran such a node. + // + // No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it + // inside a code span and a table cell. + { + id: 'flow-builtin-node-config-values-refused', + surface: + 'a builtin flow node (get_record, create_record, update_record, delete_record, notify, http, screen, ' + + 'script, subflow, map, loop, parallel, try_catch) whose config carries a value its executor ' + + 'contract refuses — a value of the wrong type (create_record outputVariable 42, a screen field min ' + + 'written as the string 1, get_record limit as a string, update_record multi as a string), a value ' + + 'outside the declared set or range (notify severity loud, screen mode view, loop maxIterations 0, ' + + 'try_catch retry.maxRetries above 10), an empty script function or subflow flowName, or a rule ' + + 'finding on present keys (a notify template beside an inline title). Never a value carrying a ' + + 'token in braces, an undeclared or retired key, a region slot, or an http signingSecret. ' + + 'Reachable wherever a flow is authored or stored: defineStack({ flows }) sources, defineFlow(), an ' + + 'exported stack passed to objectstack validate or objectstack compile, a flow saved from the Studio ' + + 'flow designer, and a flow row already sitting in sys_metadata', + replacement: + 'the value the contract declares, written at the key the refusal names: a string where it wants a ' + + 'string (`outputVariable: \'taskId\'`), a number where it wants a number (`min: 1`, `limit: 10`, ' + + '`maxIterations: 5`, `timeoutMs: 5000`), a boolean where it wants a boolean (`multi: true`, ' + + '`durable: true`), one of the declared values (`severity: \'warning\'`, `mode: \'edit\'`), or a value ' + + 'inside the declared range. Outside `http`, a number or boolean slot takes a LITERAL only: those ' + + 'executors parse the config as authored, so a `{token}` template there (`limit: \'{page.size}\'`, ' + + '`maxIterations: \'{cap}\'`) passes the build doors and still fails every run. Only `http` ' + + 'interpolates its config before it parses, so only an `http` slot may also take a sole-token template ' + + 'that resolves to the declared type (`timeoutMs: \'{timeout}\'`, `durable: \'{durable}\'`). For a rule ' + + 'finding, follow the rule\'s own sentence (keep `template` or the inline `title` / `message`, not both)', + reason: + 'Every builtin executor (`service-automation` `builtin/`) parses its node\'s `config` against the ' + + 'contract `getBuiltinNodeConfigContracts()` names before it acts, and refuses the node on any ' + + 'finding. The build doors judged only the keys that contract requires, left out, so a present value ' + + 'it refuses — `create_record` `outputVariable: 42`, a screen field `min: \'1\'` (the shape the Studio ' + + 'designer used to store for a field\'s Min / Max) — passed `FlowSchema.parse`, `objectstack ' + + 'validate` and `objectstack compile`, registered, and then failed every run that reached the node: ' + + 'the config is metadata, and no rerun could succeed. The one judge `FlowSchema.parse`, ' + + '`AutomationEngine.registerFlow` (which parses first) and `objectstack validate` share ' + + '(`flowNodeConfigRefusals`) now refuses such a value as `node-config-refused-by-contract`, anchored ' + + 'at the key, in the contract\'s own words — the code the approval contract already uses. It judges ' + + 'only what the build can know the run will parse, and holds one more class back by ruling: a value ' + + 'carrying a `{token}` is never refused at the build doors for its pre-interpolation type — which is ' + + 'no promise it runs, since every builtin but `http` parses its config as authored and so still ' + + 'refuses a token in a number or boolean slot at its first run; `http` parses after interpolating its ' + + 'whole config, so only token-free ' + + 'values are judged there and never `signingSecret`, which the credential channel may supply; a ' + + '`loop` with no `body` is not parsed by its executor and is judged for nothing; the region slots of ' + + '`loop`, `parallel` and `try_catch` are judged as graphs of their own and by `validateControlFlow`. ' + + 'An undeclared or retired key, a `predicate` ledger slot (a screen field `visibleWhen`) and a `value` ' + + 'ledger slot (a CRUD `fields` value) keep the judges they had. ⚠️ No D2 conversion: the platform ' + + 'cannot know the value the author meant. ⚠️ Where such a node already sits the whole flow is ' + + 'refused: registered from the metadata registry or `sys_metadata` at boot it is skipped with a ' + + '`warn` naming it, its trigger not armed, while the flows beside it register; a ' + + '`defineStack({ flows })` source throws `StackSchemaInvalidError` for the whole stack; an artifact ' + + 'file is refused whole at load. ADR-0087, ADR-0031.', + acceptanceCriteria: + 'Run `objectstack validate` over every stack authored in config files, and boot every deployed ' + + 'stack. Each refusal names the node and the key: `FlowSchema.parse` anchors a `custom` issue at ' + + '`nodes.N.config.` (`nodes.N.config.outputVariable`, `nodes.N.config.fields.0.min`, or the ' + + 'region path `nodes.N.config.body.nodes.M.config…`), `objectstack validate` prints the same path, ' + + 'and `validateStackExpressions` phrases it as `node \'mk\' (create_record) config.outputVariable`. ' + + 'For each hit write the value the contract declares, per the replacement. Two proofs. (1) For a ' + + 'stack authored in config files, `objectstack validate` is clean. (2) Boot the stack and confirm ' + + 'each flow REGISTERS: no `failed to register flow` warn for it — that warn line is the locator for ' + + 'a row that exists only in `sys_metadata`. A node whose values its contract accepts parses and ' + + 'registers byte-identically to before. A `{token}` template parses and registers as before too, and ' + + 'runs only where the run parses it after interpolation (`http`) or where the slot takes a string; in a ' + + 'number or boolean slot of any other builtin it fails at its first run exactly as it did, so write a ' + + 'literal there.', + }, // The absent half of the decision-branch predicate rule. A SEPARATE entry from // `flow-predicate-slot-blank-string-refused` on purpose: that one keeps the // run a blank predicate made (it evaluated `false`, so `'false'` runs the same