diff --git a/.changeset/20418-connector-action-config-required.md b/.changeset/20418-connector-action-config-required.md new file mode 100644 index 00000000000..b3dc187c34d --- /dev/null +++ b/.changeset/20418-connector-action-config-required.md @@ -0,0 +1,63 @@ +--- +'@objectstack/spec': minor +--- + +fix(spec)!: a `connector_action` flow node its executor cannot dispatch — no `connectorConfig` block, or an empty `connectorId` / `actionId` — is refused at authoring (#20418) + +Clause-②: no (narrowing) + + + +**BREAKING** — an accept-set narrowing on authored `connector_action` flow nodes, shipped +as `minor` under the launch-window convention (`check-changeset-no-major` refuses `major` +until GA; breaking-ness is carried by this banner and the ADR-0087 disposition above, not by +the level). + +**What changed.** A `connector_action` node's contract is its sibling `connectorConfig` +block — the executor reads nothing else, and refuses the node when `connectorId` or +`actionId` is empty. The block was optional on the node and both ids were any string inside +it, so `FlowSchema.parse`, `AutomationEngine.registerFlow` and `objectstack validate` all +admitted a node with no block, or with an empty id, and every run that reached the node then +failed at the executor's guard. The flow parse now refuses what that read refuses, at any +depth including an ADR-0031 region body, and `registerFlow` and `objectstack validate` meet +the refusal through that parse: + +- **No `connectorConfig` block** — a `custom` issue at `nodes.N.connectorConfig`, whose + message prescribes the block and says that keys left under `config` are not read. +- **`connectorId` or `actionId` empty, or only whitespace** — a `custom` issue at + `nodes.N.connectorConfig.connectorId` / `.actionId`. Whitespace is refused with the empty + string (the spec's one notion of blank): a connector `name` is a snake_case identifier, so + it names nothing a dispatch can reach. + +The rule is judged in the flow walk, not by `FlowNodeSchema` alone, so a node nested in a +`loop` / `parallel` / `try_catch` body is refused at the path the author wrote +(`nodes.N.config.body.nodes.M.connectorConfig`). `FlowNodeSchema.parse` of a lone node is +unchanged. + +The Studio flow designer seeds a new connector node with `connectorId: ''` and +`actionId: ''`, so a connector node added and saved before it is configured is now refused +at save. 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 `failed to register flow` +warn naming it while the flows beside it register; a `defineStack({ flows })` source throws +`StackSchemaInvalidError` for the whole stack; an artifact file is refused whole at load. + +The `node-config-key-missing` refusal (`FLOW_SLOT_REFUSAL_CODES`) now describes the old +behaviour in the past tense — "the flow used to register, and then every run that reached +this node failed there" — because the doors that message is shown at refuse the flow. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| `{ type: 'connector_action', label: 'Post' }` | the connector and the action it dispatches — `connectorConfig: { connectorId: 'slack', actionId: 'chat.postMessage', input: { channel: 'C0WINS000', text: 'Done' } }` | +| `connectorConfig: { connectorId: '', actionId: '' }` | the registered connector's `name` and one of its action keys — `{ connectorId: 'rest', actionId: 'request' }` | +| `config: { connectorId: 'slack' }` (no `actionId`, no block) | the complete pair in the block — `connectorConfig: { connectorId: 'slack', actionId: 'chat.postMessage' }` | + +**One-line fix:** write the `connectorConfig` block the node dispatches by, or delete a +connector node you cannot configure yet — there is no placeholder connector. + +**Unchanged.** A connector node carrying a complete block parses, registers and dispatches +as before, and `input` stays optional. A complete `connectorId` / `actionId` / `input` trio +written under `config` is still lifted into the block before `registerFlow` and +`objectstack validate` judge it (the `flow-node-connector-config-lift` conversion). Other +node types are not asked for a `connectorConfig`. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 5fa10b1e134..e0dd6ccfb80 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -139,12 +139,12 @@ Each node performs a specific action in the flow. | `type` | `string` | ✅ | Node type — a built-in id from the table above **or** a plugin-registered one. Per ADR-0018 the spec does not gate this with a closed enum; it is checked against the live action registry once that registry is complete — plugins contribute node types while they start, so flows registered during boot are checked in one pass when the vocabulary closes (all plugins started), and anything registered after that (Studio publish, dev reload) is checked immediately. Unknown types warn, never reject; executing one fails with `NO_EXECUTOR` | | `label` | `string` | ✅ | Display label | | `config` | `object` | optional | Type-specific configuration — the registered executor's `configSchema` owns its shape. Keys that schema does not declare are rejected at `registerFlow()`, and the built-in executors `parse()` the value against their Zod contract before running (#4277) | -| `connectorConfig` | `object` | optional | `{ connectorId, actionId, input }` for a `connector_action` node | +| `connectorConfig` | `object` | ✅ on `connector_action` | `{ connectorId, actionId, input }` — the only input a `connector_action` node's executor reads. `FlowSchema` refuses a `connector_action` node without it, and one whose `connectorId` or `actionId` is blank (empty or whitespace only), at any depth including a region body. `connectorId` is the registered connector's `name`, `actionId` one of the action keys it declares; `input` is optional | | `position` | `{ x, y }` | optional | Visual position on canvas | | `timeoutMs` | `number` | optional | Per-node execution timeout | | `inputSchema` | `object` | optional | Declared input parameter types, for Studio form generation and runtime validation | -| `waitEventConfig` | `object` | optional | `wait`-node event descriptor (`eventType`, `timerDuration`, `signalName`). `timeoutMs` / `onTimeout` were removed in 17 (#4158) — `wait` has no timeout; `timerDuration` accepts a bare number as milliseconds | -| `boundaryConfig` | `object` | optional | BPMN boundary-event descriptor (interop) | +| `waitEventConfig` | `object` | ✅ on `wait` | `wait`-node event descriptor (`eventType`, `timerDuration`, `signalName`). `FlowSchema` refuses a `wait` node without it; `eventType` has no default, and `eventType: 'timer'` requires a non-blank `timerDuration`. `timeoutMs` / `onTimeout` were removed in 17 (#4158) — `wait` has no timeout; `timerDuration` is a string, and a quoted bare number (`'60000'`) is read as milliseconds | +| `boundaryConfig` | `object` | ✅ on `boundary_event` | BPMN boundary-event descriptor (interop). `FlowSchema` refuses a `boundary_event` node without it | The flow, node, edge, and variable **shells are `.strict()`** — a key they do not diff --git a/packages/services/service-automation/src/builtin/connector-nodes.test.ts b/packages/services/service-automation/src/builtin/connector-nodes.test.ts index b71b5462eee..7a6e6f4f9cf 100644 --- a/packages/services/service-automation/src/builtin/connector-nodes.test.ts +++ b/packages/services/service-automation/src/builtin/connector-nodes.test.ts @@ -201,25 +201,72 @@ describe('connector_action (baseline node)', () => { expect(received).toEqual({}); }); - it('fails the step when connectorConfig is missing required fields', async () => { - engine.registerFlow('bad_config', { - name: 'bad_config', - label: 'Bad Config', - type: 'autolaunched', + /** + * #20418 — `registerFlow`, the second of the three doors, refuses a node + * this executor cannot dispatch: it parses first (`FlowSchema`), and the + * flow parse judges the `connectorConfig` block the way this executor + * reads it. Before, each of these shapes REGISTERED and then failed every + * run at the guard below — the last test in this block is that ground. + */ + function unconfiguredFlow(name: string, call: Record) { + return { + name, + label: name, + type: 'autolaunched' as const, nodes: [ { id: 'start', type: 'start', label: 'Start' }, - { id: 'call', type: 'connector_action', label: 'No Config' }, + { id: 'call', type: 'connector_action', label: 'Call', ...call }, { id: 'end', type: 'end', label: 'End' }, ], edges: [ { id: 'e1', source: 'start', target: 'call' }, { id: 'e2', source: 'call', target: 'end' }, ], - }); + }; + } + + /** The issues `registerFlow` threw, as `[code, path]`, or `undefined` when it registered. */ + function refusalOf(flow: { name: string }): Array<[string, unknown[]]> | undefined { + try { + engine.registerFlow(flow.name, flow as never); + return undefined; + } catch (e) { + return ((e as { issues?: Array<{ code: string; path: unknown[] }> }).issues ?? []) + .map((i) => [i.code, i.path] as [string, unknown[]]); + } + } + + it('registerFlow refuses a node with no connectorConfig block, naming the block', async () => { + expect(refusalOf(unconfiguredFlow('no_block', {}))).toEqual([['custom', ['nodes', 1, 'connectorConfig']]]); + expect(await engine.listFlows()).not.toContain('no_block'); + }); + + it('registerFlow refuses the designer seed — both ids blank — naming each key', () => { + expect(refusalOf(unconfiguredFlow('blank_ids', { connectorConfig: { connectorId: '', actionId: '', input: {} } }))) + .toEqual([ + ['custom', ['nodes', 1, 'connectorConfig', 'connectorId']], + ['custom', ['nodes', 1, 'connectorConfig', 'actionId']], + ]); + }); + + it('CONTROL — the same node with its block registers', () => { + expect(refusalOf(unconfiguredFlow('configured', { connectorConfig: { connectorId: 'fake', actionId: 'echo' } }))) + .toBeUndefined(); + }); + + it('what the refused shape did at run time: the step failed at the guard, every run', async () => { + // It can no longer register, so the run registers the block whole and + // deletes it from the stored node — the shape this executor meets when + // a node reaches it past the doors. + const stored = engine.registerFlow( + 'stripped', + unconfiguredFlow('stripped', { connectorConfig: { connectorId: 'fake', actionId: 'echo' } }) as never, + ); + delete (stored.nodes[1] as { connectorConfig?: unknown }).connectorConfig; - const result = await engine.execute('bad_config'); + const result = await engine.execute('stripped'); expect(result.success).toBe(false); - expect(result.error).toContain('connectorId'); + expect(result.error).toContain("connector_action 'call': connectorConfig.connectorId and .actionId are required"); }); }); diff --git a/packages/services/service-automation/src/guard-refusal-inventory.test.ts b/packages/services/service-automation/src/guard-refusal-inventory.test.ts index 6368f76e450..70a1421ab85 100644 --- a/packages/services/service-automation/src/guard-refusal-inventory.test.ts +++ b/packages/services/service-automation/src/guard-refusal-inventory.test.ts @@ -85,8 +85,13 @@ function flowWithHandler(name: string, node: Record) { * inventory classifies, so those rows register the node WHOLE and remove the * key from the stored flow before the run: the shape an executor meets when a * config reaches it past the doors. + * + * `stripBlock` (#20418): the same move for a node whose contract is a SIBLING + * block rather than `config` — a `connector_action` with no `connectorConfig` + * is refused at the build doors too, so its row registers the block whole and + * removes it from the stored node before the run. */ -const GUARDS: Array<{ name: string; why: string; node: Record; expect: string; strip?: string }> = [ +const GUARDS: Array<{ name: string; why: string; node: Record; expect: string; strip?: string; stripBlock?: string }> = [ // Since #4277 a missing REQUIRED key is refused by the executor's contract // parse (parse-config.ts) before the hand-written guard runs, so those // entries pin the parse refusal's fragment. The classification is the @@ -168,7 +173,8 @@ const GUARDS: Array<{ name: string; why: string; node: Record; { name: 'connector_action without connectorId/actionId', why: 'required config keys', - node: { type: 'connector_action', config: {} }, + node: { type: 'connector_action', connectorConfig: { connectorId: 'crm', actionId: 'push' } }, + stripBlock: 'connectorConfig', expect: 'are required', }, { @@ -208,7 +214,7 @@ describe('#3863 — the guard inventory stays un-routable', () => { it.each(GUARDS.map((g, i) => ({ ...g, i })))( '$name stays fatal with a fault edge ($why)', - async ({ node, expect: fragment, i, strip }) => { + async ({ node, expect: fragment, i, strip, stripBlock }) => { let handlerRan = false; engine.registerNodeExecutor({ type: 'script', @@ -220,6 +226,7 @@ describe('#3863 — the guard inventory stays un-routable', () => { const flowName = `guard_case_${i}`; const stored = engine.registerFlow(flowName, flowWithHandler(flowName, node) as any); if (strip) delete (stored.nodes.find((n) => n.id === 'op')!.config as Record)[strip]; + if (stripBlock) delete (stored.nodes.find((n) => n.id === 'op') as unknown as Record)[stripBlock]; const result = await engine.execute(flowName, { record: { id: 'r1', owner: 'usr_7' } } as any); diff --git a/packages/spec/src/automation/connector-action-config-required.test.ts b/packages/spec/src/automation/connector-action-config-required.test.ts new file mode 100644 index 00000000000..2196c444169 --- /dev/null +++ b/packages/spec/src/automation/connector-action-config-required.test.ts @@ -0,0 +1,126 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #20418 — a `connector_action` node its executor cannot dispatch is refused at + * `FlowSchema.parse`, the first of the three doors. + * + * The node's contract is its SIBLING block `connectorConfig`, and the executor + * reads nothing else: `if (!cfg?.connectorId || !cfg?.actionId)` refuses the + * node. Measured before the change, all three build doors admitted what that + * read refuses — the block absent, or an id present and blank (the Studio + * designer's seed for a new node) — at the top level and inside a region body, + * and every run then failed at the node. + * + * Refused rows assert the issue `code` and the exact `path`, and that the + * message names the block or the key it refuses — never the prose around it. + * `registerFlow` meets the same refusal through this parse + * (`service-automation`'s `connector-nodes.test.ts`). + */ + +import { describe, expect, it } from 'vitest'; + +import { FlowNodeSchema, FlowSchema } from './flow.zod'; + +type Node = Record; + +/** start → → end, chained by unconditional edges. */ +function flowWith(...middle: Node[]) { + const nodes: Node[] = [{ id: 'start', type: 'start', label: 'Start' }, ...middle, { id: 'end', type: 'end', label: 'End' }]; + const edges = nodes.slice(1).map((n, i) => ({ id: `e${i}`, source: String(nodes[i].id), target: String(n.id) })); + return { name: 'connector_probe', label: 'Connector probe', type: 'autolaunched', nodes, edges }; +} + +const connector = (extra: Node = {}): Node => ({ id: 'call', type: 'connector_action', label: 'Call', ...extra }); +const COMPLETE = { connectorId: 'slack', actionId: 'chat.postMessage', input: { channel: 'C1', text: 'Hi' } }; + +/** A `loop` whose body holds `inner` — the ADR-0031 region the walk must reach. */ +const loopAround = (inner: Node): Node => ({ + id: 'each', type: 'loop', label: 'Each', + config: { collection: [1], iteratorVariable: 'item', body: { nodes: [inner], edges: [] } }, +}); + +function issuesOf(flow: unknown) { + const result = FlowSchema.safeParse(flow); + return result.success ? [] : result.error.issues.map((i) => ({ code: i.code, path: i.path, message: i.message })); +} + +describe('FlowSchema refuses a connector_action node with no connectorConfig block', () => { + it('top level: one `custom` issue at the block, prescribing it', () => { + const issues = issuesOf(flowWith(connector())); + expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 1, 'connectorConfig']]]); + expect(issues[0].message).toContain('requires a `connectorConfig` block'); + }); + + it('inside a region body: refused at the path the author wrote', () => { + const issues = issuesOf(flowWith(loopAround(connector()))); + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'connectorConfig']], + ]); + }); + + it('the keys written under `config` instead: refused at the block, and told to move them', () => { + // A direct parse meets the pre-conversion spelling, like every other + // tombstone; `registerFlow` and `objectstack validate` convert a complete + // pair into the block first (the `flow-node-connector-config-lift` D2 entry). + const issues = issuesOf(flowWith(connector({ config: { connectorId: 'slack', actionId: 'chat.postMessage' } }))); + expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 1, 'connectorConfig']]]); + expect(issues[0].message).toContain('from `config` into the block'); + }); +}); + +describe('FlowSchema refuses a blank connectorId / actionId', () => { + it('the designer seed (both ids empty): one issue per key', () => { + const issues = issuesOf(flowWith(connector({ connectorConfig: { connectorId: '', actionId: '', input: {} } }))); + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'connectorConfig', 'connectorId']], + ['custom', ['nodes', 1, 'connectorConfig', 'actionId']], + ]); + expect(issues[0].message).toContain('`connectorConfig.connectorId` holds a string that is blank'); + expect(issues[1].message).toContain('`connectorConfig.actionId` holds a string that is blank'); + }); + + it.each([ + ['a whitespace-only connectorId', { connectorId: ' \t', actionId: 'chat.postMessage' }, 'connectorId'], + ['an empty actionId', { connectorId: 'slack', actionId: '' }, 'actionId'], + ])('%s: refused at that key only', (_name, block, key) => { + const issues = issuesOf(flowWith(connector({ connectorConfig: block }))); + expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 1, 'connectorConfig', key]]]); + }); + + it('inside a region body: refused at the path the author wrote', () => { + const issues = issuesOf(flowWith(loopAround(connector({ connectorConfig: { connectorId: 'slack', actionId: '' } })))); + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'connectorConfig', 'actionId']], + ]); + }); +}); + +describe('what is NOT refused by this rule', () => { + it('CONTROL — a complete block parses, at the top level and in a region body, with and without `input`', () => { + expect(issuesOf(flowWith(connector({ connectorConfig: COMPLETE })))).toEqual([]); + expect(issuesOf(flowWith(connector({ connectorConfig: { connectorId: 'rest', actionId: 'request' } })))).toEqual([]); + expect(issuesOf(flowWith(loopAround(connector({ connectorConfig: COMPLETE }))))).toEqual([]); + }); + + it('CONTROL — another node type carries no connectorConfig and is not asked for one', () => { + expect(issuesOf(flowWith({ id: 'n', type: 'assignment', label: 'N' }))).toEqual([]); + }); + + it('a block the node shape already refuses is not reported a second time', () => { + // `{}` and a non-string id fail the block's own shape; this rule judges + // strings only, so the author sees one issue per key, not two. + expect(issuesOf(flowWith(connector({ connectorConfig: {} }))).map((i) => [i.code, i.path])).toEqual([ + ['invalid_type', ['nodes', 1, 'connectorConfig', 'connectorId']], + ['invalid_type', ['nodes', 1, 'connectorConfig', 'actionId']], + ]); + expect(issuesOf(flowWith(connector({ connectorConfig: { connectorId: 5, actionId: 'a' } }))).map((i) => i.code)) + .toEqual(['invalid_type']); + }); + + it('FlowNodeSchema alone still parses the designer seed — the refusal is the FLOW\'s', () => { + // The node contract a designer seed is held to on its own stays structural; + // the flow it is saved into is what gets refused (see the second describe). + const seed = connector({ connectorConfig: { connectorId: '', actionId: '', input: {} } }); + expect(FlowNodeSchema.safeParse(seed).success).toBe(true); + }); +}); diff --git a/packages/spec/src/automation/flow-node-config-refusals.ts b/packages/spec/src/automation/flow-node-config-refusals.ts index bd1d620a25b..d82bd029f7d 100644 --- a/packages/spec/src/automation/flow-node-config-refusals.ts +++ b/packages/spec/src/automation/flow-node-config-refusals.ts @@ -82,7 +82,9 @@ let cachedBuiltinNodeConfigContracts: ReadonlyMap { if (cachedBuiltinNodeConfigContracts === undefined) { @@ -182,8 +184,8 @@ function nodeConfigKeyMissingMessage(nodeType: string, key: string): string { return ( `This \`${nodeType}\` node's config leaves out \`${key}\`, which the ${nodeType} contract requires. Its executor ` + 'parses the config against that contract before it does anything else and refuses the node without it — so ' - + 'the flow registers, and then every run that reaches this node fails there; the config is metadata, and ' - + `re-running changes nothing. Write \`${key}\` on the node's \`config\`.` + + 'the flow used to register, and then every run that reached this node failed there; the config is metadata, ' + + `and re-running changes nothing. Write \`${key}\` on the node's \`config\`.` ); } diff --git a/packages/spec/src/automation/flow-slot-refusal-codes.test.ts b/packages/spec/src/automation/flow-slot-refusal-codes.test.ts index 3f1629a7291..c74494b6df4 100644 --- a/packages/spec/src/automation/flow-slot-refusal-codes.test.ts +++ b/packages/spec/src/automation/flow-slot-refusal-codes.test.ts @@ -88,8 +88,8 @@ const LABEL_MISSING = (path: string, found: string): string => const KEY_MISSING = (nodeType: string, key: string): string => `This \`${nodeType}\` node's config leaves out \`${key}\`, which the ${nodeType} contract requires. Its executor ` + 'parses the config against that contract before it does anything else and refuses the node without it — so the ' - + 'flow registers, and then every run that reaches this node fails there; the config is metadata, and re-running ' - + `changes nothing. Write \`${key}\` on the node's \`config\`.`; + + 'flow used to register, and then every run that reached this node failed there; the config is metadata, and ' + + `re-running changes nothing. Write \`${key}\` on the node's \`config\`.`; /** The notify contract's own words for a node with no content source — read, never re-spelled. */ const NOTIFY_TITLE_RULE = (() => { diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index e53fc907aad..f04a84a1e46 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -1106,8 +1106,8 @@ describe('BPMN — Parallel Gateway & Join Gateway', () => { nodes: [ { id: 'start', type: 'start', label: 'Start' }, { id: 'fork', type: 'parallel_gateway', label: 'Fork — Parallel Approval' }, - { id: 'finance_review', type: 'connector_action', label: 'Finance Review' }, - { id: 'legal_review', type: 'connector_action', label: 'Legal Review' }, + { id: 'finance_review', type: 'connector_action', label: 'Finance Review', connectorConfig: { connectorId: 'finance_desk', actionId: 'request_review' } }, + { id: 'legal_review', type: 'connector_action', label: 'Legal Review', connectorConfig: { connectorId: 'legal_desk', actionId: 'request_review' } }, { id: 'join', type: 'join_gateway', label: 'Join — All Approved' }, { id: 'final_approve', type: 'update_record', label: 'Final Approve', config: { objectName: 'contract', filter: { id: '{record.id}' }, fields: { status: 'approved' } } }, { id: 'end', type: 'end', label: 'End' }, diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index d2b34efb0ec..bae1a5ccd35 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -19,6 +19,7 @@ import { EvaluatedExpressionInputSchema } from '../shared/expression.zod'; * no longer constrains authored flows — plugins extend the vocabulary. */ import { lazySchema } from '../shared/lazy-schema'; +import { NON_BLANK_STRING } from '../shared/refinement-projection'; import { retiredKey } from '../shared/retired-key'; import { retryPolicyShape } from '../shared/retry-policy.zod'; import { strictObject } from '../shared/strict-object'; @@ -367,6 +368,12 @@ export const FlowNodeSchema = lazySchema(() => flowNodeObject().transform( * executor for that type at all (`NO_EXECUTOR` plus a startup `warn`, measured * in the same characterization run), so unlike `wait` there is no silent * executor branch to retire behind this. + * + * ⚠️ The third sibling block, `connector_action`'s `connectorConfig`, is + * required at the FLOW level instead — {@link connectorActionConfigRefusals}, + * walked by the `FlowSchema` superRefine — precisely because of the region + * caveat above: a refusal here leaves a region-nested node admitted by the + * flow parse. Read the reason there. */ function requireTypeScopedConfig { + if (node === null || typeof node !== 'object') return []; + const { type, connectorConfig } = node as { type?: unknown; connectorConfig?: unknown }; + if (type !== 'connector_action') return []; + if (connectorConfig === undefined) { + return [{ + path: ['connectorConfig'], + message: + 'a `connector_action` node requires a `connectorConfig` block naming the connector and the action it ' + + 'dispatches — the block is the only thing its executor reads, and a node without one used to register ' + + 'and then fail every run that reached it. Declare it, e.g. `connectorConfig: { connectorId: \'slack\', ' + + 'actionId: \'chat.postMessage\', input: { channel: \'C0WINS000\', text: \'Done\' } }` — `connectorId` is ' + + 'the registered connector\'s `name`, `actionId` one of the action keys that connector declares, and ' + + '`input` (optional) the action\'s mapped inputs. Keys written under the node\'s `config` are not read: ' + + 'move `connectorId` / `actionId` / `input` from `config` into the block.', + }]; + } + if (connectorConfig === null || typeof connectorConfig !== 'object' || Array.isArray(connectorConfig)) return []; + const block = connectorConfig as Record; + const out: Array<{ path: PropertyKey[]; message: string }> = []; + for (const key of CONNECTOR_DISPATCH_KEYS) { + const value = block[key]; + if (typeof value !== 'string' || NON_BLANK_STRING(value)) continue; + const write = key === 'connectorId' + ? 'the registered connector\'s `name` (e.g. `connectorId: \'slack\'`)' + : 'one of the action keys that connector declares (e.g. `actionId: \'chat.postMessage\'`)'; + out.push({ + path: ['connectorConfig', key], + message: + `\`connectorConfig.${key}\` holds a string that is blank after trimming, so this \`connector_action\` ` + + 'node names nothing to dispatch to. Its executor refuses a node whose `connectorId` or `actionId` is ' + + 'empty — and a whitespace-only one matches no connector — so a flow carrying it used to register and ' + + `then fail every run that reached the node. Write ${write}, or delete the node until it is configured.`, + }); + } + return out; +} + /** * Parse a structural `end` node's `config` against {@link EndConfigSchema} * (#14945), the second half of the node transform above. @@ -853,8 +951,10 @@ export const FlowEdgeSchema = lazySchema(() => strictObject( * nodes: [ * { id: "start", type: "start", label: "Start", position: {x: 0, y: 0} }, * { id: "check_amount", type: "decision", label: "Check Amount", position: {x: 0, y: 100} }, - * { id: "auto_approve", type: "update_record", label: "Auto Approve", position: {x: -100, y: 200} }, - * { id: "submit_for_approval", type: "connector_action", label: "Submit", position: {x: 100, y: 200} } + * { id: "auto_approve", type: "update_record", label: "Auto Approve", position: {x: -100, y: 200}, + * config: { objectName: "order", filter: { id: "{record.id}" }, fields: { status: "approved" } } }, + * { id: "submit_for_approval", type: "connector_action", label: "Submit", position: {x: 100, y: 200}, + * connectorConfig: { connectorId: "approvals_desk", actionId: "submit", input: { orderId: "{record.id}" } } } * ], * edges: [ * { id: "e1", source: "start", target: "check_amount" }, @@ -1392,6 +1492,21 @@ export const FlowSchema = lazySchema(() => strictObject( }); } + // What a `connector_action` node's executor needs its SIBLING block to carry + // (#20418) — `connectorConfig` present, `connectorId` and `actionId` not + // blank — the rest of the read the executor refuses the node on. Beside the + // `config` judge above rather than in it (the block is not `config`), and in + // this walk rather than in the node transform so a node inside an ADR-0031 + // region body is refused here too; the reason is measured under + // {@link connectorActionConfigRefusals}. + for (const graph of collectFlowGraphs(flow)) { + graph.nodes.forEach((node, index) => { + for (const refusal of connectorActionConfigRefusals(node)) { + ctx.addIssue({ code: 'custom', path: [...graph.path, 'nodes', index, ...refusal.path], message: refusal.message }); + } + }); + } + // Edges (#14964): every reader of `edges[].id` assumes the ids are unique — // a designer, a BPMN export, a flow diff, any traversal that dedupes by id — // while nothing enforced it: two edges carrying one id parsed, shipped diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 3bf05d89674..bef8032743b 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -1474,9 +1474,11 @@ const CONNECTOR_CONFIG_LIFTS = ['connectorId', 'actionId', 'input'] as const; * default: the loader parses the CONVERTED flow, and `connectorConfig` requires * `connectorId` + `actionId` once the block exists. Unlike `eventType` there is * no defensible default for either, so when lifting cannot complete that pair - * the node is left **untouched** — it keeps failing at run time with the same - * clear refusal it produces today, rather than going from "registers, fails - * the step" to "fails to load". + * the node is left **untouched** — materializing a half block would only move + * the refusal onto a key the author never wrote. The flow parse then refuses + * the node for its missing `connectorConfig` block, whose prescription names + * the move out of `config` (#20418); before the parse required the block, such + * a node registered and failed every run at the executor's guard. */ function liftConnectorConfigShape(stack: Dict, emit: Emit): Dict { return mapFlowNodes(stack, (node, path) => { @@ -1548,7 +1550,7 @@ const flowNodeConnectorConfigLift: MetadataConversion = { }, // Completeness guard: no actionId anywhere, so lifting would // create a block the loader rejects — left untouched instead - // (same run-time refusal as today). + // (the flow parse refuses it for the missing block). { id: 'n4', type: 'connector_action', diff --git a/packages/spec/src/migrations/entries/semantic/18.connector-action-config-required.ts b/packages/spec/src/migrations/entries/semantic/18.connector-action-config-required.ts new file mode 100644 index 00000000000..0b25233929d --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.connector-action-config-required.ts @@ -0,0 +1,64 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// The `connector_action` sibling of `wait-node-event-config-required`: a node +// whose one input is a SIBLING block (not `config`) owes that block, and the +// flow parse now refuses what the executor's own read refuses — the block +// absent, or an id in it empty. +// +// Form D: no tracker number anywhere in the author-shown text; the decision is +// stated in words. +// +// No backticks in `surface` — build-upgrade-guide.ts renders it inside a code +// span already, and a nested backtick would close it. +export const entry: SemanticMigration = { + id: 'connector-action-config-required', + surface: + 'The connectorConfig block of every type: \'connector_action\' flow node — the BLOCK, and its ' + + 'connectorId and actionId once it is written. The block was optional on the node and both ids ' + + 'were any string inside it, so a node with no block, or with connectorId or actionId empty or ' + + 'only whitespace, parsed. That is the state of a node authored without its configuration, and ' + + 'of a new connector node from the Studio flow designer, which seeds both ids empty. At any depth, ' + + 'including an ADR-0031 region body. Reachable wherever a flow is authored or stored: ' + + 'defineStack({ flows }) sources, defineFlow(), an exported stack passed to objectstack validate, ' + + 'a flow saved from the Studio flow designer (a connector node added and saved before it is ' + + 'configured), and a flow row already sitting in sys_metadata. Also reached: connectorId, actionId ' + + 'or input written under the node\'s config instead of the block, where the load-time conversion ' + + 'cannot complete the pair and leaves them there', + replacement: + 'Declare what the node dispatches, on the node: `connectorConfig: { connectorId: \'slack\', ' + + 'actionId: \'chat.postMessage\', input: { channel: \'C0WINS000\', text: \'Done\' } }` — ' + + '`connectorId` the registered connector\'s `name`, `actionId` one of the action keys that ' + + 'connector declares, `input` optional. Keys written under the node\'s `config` move into the ' + + 'block. A node you cannot configure yet is deleted until you can: there is no placeholder ' + + 'connector, and a block with empty ids names nothing to dispatch to', + reason: + 'The block is the node\'s whole contract: the connector_action executor reads nothing else and ' + + 'refuses the node when `connectorId` or `actionId` is empty. The build doors checked only the ' + + 'block\'s shape once it was written, so `FlowSchema.parse`, `AutomationEngine.registerFlow` and ' + + '`objectstack validate` all admitted a node with no block, or with an empty id, and every run ' + + 'that reached the node then failed at the executor\'s guard — a guard refusal, never routed to a ' + + '`fault` edge, and no rerun could succeed because the config is metadata. The flow parse now ' + + 'refuses what that read refuses, in the walk that reaches every region body, so all three doors ' + + 'answer alike. A whitespace-only id is refused with the empty one: a connector `name` is a ' + + 'snake_case identifier, so whitespace names nothing a dispatch can reach. ' + + '⚠️ No D2 conversion: the platform cannot know the connector or the action the author left out, ' + + 'and no value it could write would dispatch anything. ' + + '⚠️ 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.connectorConfig` for an absent block, at `nodes.N.connectorConfig.connectorId` or ' + + '`nodes.N.connectorConfig.actionId` for an empty or whitespace-only id, or at the region path ' + + '`nodes.N.config.body.nodes.M.connectorConfig…`, and `objectstack validate` prints the same path ' + + 'under `flows.K.`. For each hit write the block, 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 connector node carrying a complete block parses, ' + + 'registers and dispatches as before, and `input` stays optional.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 6095aa93471..410b6b94366 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -7255,6 +7255,66 @@ const step18: MigrationStep = { + 'fail tsc on upgrade; the fix is choosing a shipped driver, never ' + 'widening a local mirror of the enum.', }, + // The `connector_action` sibling of `wait-node-event-config-required`: a node + // whose one input is a SIBLING block (not `config`) owes that block, and the + // flow parse now refuses what the executor's own read refuses — the block + // absent, or an id in it empty. + // + // Form D: no tracker number anywhere in the author-shown text; the decision is + // stated in words. + // + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a code + // span already, and a nested backtick would close it. + { + id: 'connector-action-config-required', + surface: + 'The connectorConfig block of every type: \'connector_action\' flow node — the BLOCK, and its ' + + 'connectorId and actionId once it is written. The block was optional on the node and both ids ' + + 'were any string inside it, so a node with no block, or with connectorId or actionId empty or ' + + 'only whitespace, parsed. That is the state of a node authored without its configuration, and ' + + 'of a new connector node from the Studio flow designer, which seeds both ids empty. At any depth, ' + + 'including an ADR-0031 region body. Reachable wherever a flow is authored or stored: ' + + 'defineStack({ flows }) sources, defineFlow(), an exported stack passed to objectstack validate, ' + + 'a flow saved from the Studio flow designer (a connector node added and saved before it is ' + + 'configured), and a flow row already sitting in sys_metadata. Also reached: connectorId, actionId ' + + 'or input written under the node\'s config instead of the block, where the load-time conversion ' + + 'cannot complete the pair and leaves them there', + replacement: + 'Declare what the node dispatches, on the node: `connectorConfig: { connectorId: \'slack\', ' + + 'actionId: \'chat.postMessage\', input: { channel: \'C0WINS000\', text: \'Done\' } }` — ' + + '`connectorId` the registered connector\'s `name`, `actionId` one of the action keys that ' + + 'connector declares, `input` optional. Keys written under the node\'s `config` move into the ' + + 'block. A node you cannot configure yet is deleted until you can: there is no placeholder ' + + 'connector, and a block with empty ids names nothing to dispatch to', + reason: + 'The block is the node\'s whole contract: the connector_action executor reads nothing else and ' + + 'refuses the node when `connectorId` or `actionId` is empty. The build doors checked only the ' + + 'block\'s shape once it was written, so `FlowSchema.parse`, `AutomationEngine.registerFlow` and ' + + '`objectstack validate` all admitted a node with no block, or with an empty id, and every run ' + + 'that reached the node then failed at the executor\'s guard — a guard refusal, never routed to a ' + + '`fault` edge, and no rerun could succeed because the config is metadata. The flow parse now ' + + 'refuses what that read refuses, in the walk that reaches every region body, so all three doors ' + + 'answer alike. A whitespace-only id is refused with the empty one: a connector `name` is a ' + + 'snake_case identifier, so whitespace names nothing a dispatch can reach. ' + + '⚠️ No D2 conversion: the platform cannot know the connector or the action the author left out, ' + + 'and no value it could write would dispatch anything. ' + + '⚠️ 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.connectorConfig` for an absent block, at `nodes.N.connectorConfig.connectorId` or ' + + '`nodes.N.connectorConfig.actionId` for an empty or whitespace-only id, or at the region path ' + + '`nodes.N.config.body.nodes.M.connectorConfig…`, and `objectstack validate` prints the same path ' + + 'under `flows.K.`. For each hit write the block, 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 connector node carrying a complete block parses, ' + + 'registers and dispatches as before, and `input` stays optional.', + }, // ADR-0049 enforce-or-remove — the D3 entry of the // `connector-error-mapping-removed` family, which landed in commit 13c48c2a5: // eleven inert authorable keys, one of them spelled like the live