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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/23543-f3-retire-wait-connector-config-lift.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
'@objectstack/spec': minor
'@objectstack/service-automation': patch
---

feat(spec)!: the loose `config` keys of a `wait` or `connector_action` flow node are refused, naming the `waitEventConfig` / `connectorConfig` key they belong on, and `flow-node-wait-event-config-lift` and `flow-node-connector-config-lift` leave the load path

Clause-②: yes (narrowing)

**BREAKING (authoring):** neither node type has a `config` contract: the `wait` executor reads only `waitEventConfig` and the `connector_action` executor only `connectorConfig`. Through 17 the loader lifted settings written under the node's `config` into that block with a notice. At 18 both ADR-0087 D2 conversions are retired from the load path (`retiredAfter: '17.7.0'`), and the flow parse refuses each such key where it is written, at every door that judges a flow (`defineStack`, `os validate`, `os build`, and `os lint` when the config calls it), a node inside a loop / parallel / try-catch region body included: "A `wait` node's `config.duration` was removed in @objectstack/spec 18 (ADR-0087 D2) … Move it to `waitEventConfig.timerDuration` …". The refused keys are exactly the ones the lifts moved: `eventType`, `timerDuration`, `duration`, `signalName`, `signal` and `timeoutMs` on `wait`; `connectorId`, `actionId` and `input` on `connector_action`. Any other `config` key on these node types is judged as before.

Migration:

- `wait` node `config.eventType` / `config.timerDuration` / `config.signalName` → the same key under `waitEventConfig`.
- `wait` node `config.duration` → `waitEventConfig.timerDuration`, and `config.signal` → `waitEventConfig.signalName`.
- `wait` node `config.timeoutMs` → `waitEventConfig.timerDuration`, as a string (`60000` → `'60000'`, or `'PT1M'`): it was only ever read as the timer duration.
- `connector_action` node `config.connectorId` / `config.actionId` / `config.input` → the same key under `connectorConfig`.
- If the block already sets the same key, the block's value is the one the node always ran with: delete the loose key.

Run `os migrate meta --from 16` to list the mechanical edits for existing sources; `--write` applies the ones it can prove, and you apply the rest by hand. The D3 entry `flow-sibling-block-loose-config-keys-refused` states the edit for upgraders.

Stored metadata needs no edit. The automation engine's flow rehydration (`registerFlow`), stored-row rehydration and the metadata save door's flow verdict still replay both conversions, and each now DELETES a loose key its block already shadows (and a loose key holding `null`) instead of leaving it in place, so a `sys_metadata` flow carrying either shape keeps loading with the block alone. A `connector_action` node whose `connectorId` / `actionId` pair is complete in neither place is refused as before: no conversion can complete it.

`@objectstack/service-automation`: comment-only changes in the `wait` and `connector_action` executors, which already read only their declared blocks.

<!-- adr-0087: registered flow-sibling-block-loose-config-keys-refused -->
4 changes: 2 additions & 2 deletions content/docs/automation/flows.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,11 @@ 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 — its node type owns its shape. On every built-in node type with an executor contract, a key that contract does not declare is refused at the flow parse, at the key's path; on a plugin node type, a key the published `configSchema` does not declare is rejected at `registerFlow()`. An `approval` node's config is judged whole against its declared contract at the flow parse, and the built-in executors `parse()` the value against their Zod contract before running |
| `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 |
| `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. The same three keys written under the node's `config` are refused (since 18), each naming its `connectorConfig` key |
| `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` | ✅ 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 |
| `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. Event keys written under the node's `config` instead (`eventType`, `timerDuration` or `duration`, `signalName` or `signal`, `timeoutMs`) are refused (since 18), each naming its `waitEventConfig` key |
| `boundaryConfig` | `object` | ✅ on `boundary_event` | BPMN boundary-event descriptor (interop). `FlowSchema` refuses a `boundary_event` node without it |

<Callout type="info">
Expand Down
4 changes: 2 additions & 2 deletions packages/lint/src/lint-flow-patterns.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ describe('lintFlowPatterns — approval revise loop (ADR-0044)', () => {
) => {
const stack = approvalFlow(edges) as any;
stack.flows[0].nodes = stack.flows[0].nodes.map((n: any) =>
n.id === 'wait' ? { id: 'wait', type: 'wait', config: { eventType: 'signal' } } : n,
n.id === 'wait' ? { id: 'wait', type: 'wait', waitEventConfig: { eventType: 'signal' } } : n,
);
return stack;
};
Expand Down Expand Up @@ -2089,7 +2089,7 @@ describe('per-iteration containment (#13681 / #14394)', () => {
{ id: 'n4', type: 'delete_record', label: 'Delete', config: { objectName: 'task', filter: { case_id: CEL('currentCase.id') } } },
{ id: 'n5', type: 'http', label: 'Call', config: { url: 'https://example.test' } },
{ id: 'n6', type: 'notify', label: 'Notify', config: { title: 'x' } },
{ id: 'n7', type: 'connector_action', label: 'Connector', config: { connectorId: 'c', action: 'a' } },
{ id: 'n7', type: 'connector_action', label: 'Connector', connectorConfig: { connectorId: 'c', actionId: 'a' } },
{ id: 'n8', type: 'script', label: 'Script', config: { function: 'f' } },
{ id: 'n9', type: 'subflow', label: 'Subflow', config: { flowName: 'child' } },
{ id: 'n10', type: 'map', label: 'Map', config: { collection: { dialect: 'cel', source: 'items' } } },
Expand Down
4 changes: 2 additions & 2 deletions packages/lint/src/validate-expressions.flow-cel-root.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ describe('controls — one per binding kind the run has', () => {
describe('the door stands down where the run can bind a name it cannot see', () => {
it.each<[string, AnyRec]>([
['a `script` (its function gets the live variable map)', { id: 'fn', type: 'script', config: { function: 'enrich' } }],
['a `connector_action` (its handler gets the live variable map)', { id: 'push', type: 'connector_action', config: { connectorId: 'slack', actionId: 'post' } }],
['a `wait` (a resume folds the caller\'s bag)', { id: 'hold', type: 'wait', config: { eventType: 'signal', signalName: 'go' } }],
['a `connector_action` (its handler gets the live variable map)', { id: 'push', type: 'connector_action', connectorConfig: { connectorId: 'slack', actionId: 'post' } }],
['a `wait` (a resume folds the caller\'s bag)', { id: 'hold', type: 'wait', waitEventConfig: { eventType: 'signal', signalName: 'go' } }],
['a `subflow` (a resume folds the caller\'s bag)', { id: 'child', type: 'subflow', config: { flowName: 'other' } }],
['a `map` (a resume folds the caller\'s bag)', { id: 'each', type: 'map', config: { collection: { dialect: 'cel', source: 'rows' }, flowName: 'other' } }],
['a plugin node type', { id: 'ai', type: 'ai_classify', config: {} }],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,12 @@ describe('connector_action (baseline node)', () => {

// #4045 — this proves the CONVERSION, not executor tolerance. The executor
// reads only the declared `connectorConfig` block; the config-authored trio
// reaches it because `registerFlow` applies
// `flow-node-connector-config-lift`, which lifts it. This is the exact
// shape the descriptor's former configSchema mis-taught the schema-driven
// Studio form to write.
// reaches it because `registerFlow` — the rehydration seam a stored flow
// meets — replays `flow-node-connector-config-lift`, which lifts it. That
// entry left the authoring load path at 18 (#23543 F3): `defineStack` /
// `os validate` refuse this shape now, and only a stored row is lifted.
// This is the exact shape the descriptor's former configSchema mis-taught
// the schema-driven Studio form to write.
it('accepts the mis-taught config.{connectorId,actionId,input} shape via the lift', async () => {
let received: Record<string, unknown> | undefined;
engine.registerConnector(fakeConnector(), {
Expand Down Expand Up @@ -163,6 +165,47 @@ describe('connector_action (baseline node)', () => {
expect(received).toEqual({ message: 'hi' });
});

// #23543 F3 — a stored row whose complete block shadows a loose key: the
// flow parse refuses every loose key under `config` since 18, so the lift
// deletes the shadowed one at this seam, and the row registers and runs
// with the block's input alone — never the stale loose one.
it('registers a stored row whose block shadows a loose `config.input`, and dispatches the block\'s input', async () => {
let received: Record<string, unknown> | undefined;
engine.registerConnector(fakeConnector(), {
async echo(input) {
received = input;
return { ok: true };
},
});

engine.registerFlow('shadowed_flow', {
name: 'shadowed_flow',
label: 'Shadowed Flow',
type: 'autolaunched',
nodes: [
{ id: 'start', type: 'start', label: 'Start' },
{
id: 'call',
type: 'connector_action',
label: 'Shadowed',
connectorConfig: { connectorId: 'fake', actionId: 'echo', input: { message: 'block' } },
config: { input: { message: 'stale' } },
},
{ id: 'end', type: 'end', label: 'End' },
],
edges: [
{ id: 'e1', source: 'start', target: 'call' },
{ id: 'e2', source: 'call', target: 'end' },
],
});

const node = (await engine.getFlow('shadowed_flow'))!.nodes.find((n) => n.id === 'call')!;
expect(node.config).toEqual({});
const result = await engine.execute('shadowed_flow');
expect(result.success).toBe(true);
expect(received).toEqual({ message: 'block' });
});

// #4045 — `input` is optional in the spec block because the executor
// dispatches with `input ?? {}` and the designer's keyValue editor omits an
// empty map entirely. Under the old required `input`, this exact designer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ export function registerConnectorNodes(engine: AutomationEngine, ctx: PluginCont
// `config.<key>` and REPLACING the client's hand-written
// connectorConfig form (with its connector/action pickers). So the
// schema steered online authoring to keys the executor never
// reads, and the node refused to dispatch. Stored flows that
// carry the mis-taught shape are healed at load by the ADR-0087 D2
// conversion 'flow-node-connector-config-lift'.
// reads, and the node refused to dispatch. Since 18 the flow parse
// refuses those loose keys at authoring, and stored flows that
// carry the mis-taught shape are healed at rehydration by the
// retired ADR-0087 D2 conversion 'flow-node-connector-config-lift'.
}),
async execute(node, variables, context) {
const cfg = node.connectorConfig;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,9 @@ describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)',
*
* These go through `registerFlow`, which is the seam that applies the conversion
* — so they prove the graduation end-to-end on a legacy source, not merely that
* the executor stopped looking. Both spellings under test (`duration`, `signal`)
* the executor stopped looking. Since 18 (#23543 F3) that seam is the STORED
* flow's: the conversion left the authoring load path, and an authored loose key
* is refused by the flow parse, so these read as "a stored row still runs". Both spellings under test (`duration`, `signal`)
* are ones the spec never declared; they existed only as the tail of the `??`
* chains this change deleted.
*/
Expand Down Expand Up @@ -896,10 +898,10 @@ describe('wait config graduation — legacy loose `config` still works via the c
expect(ran).toEqual(['after']);
});

// Unlike the two above, this one passes with the conversion unregistered too —
// it pins the EXECUTOR's side of the precedence (a declared value is what gets
// read), not the conversion's. Kept because that is the half a future
// "simplification" of the lift could silently invert.
// It pins the EXECUTOR's side of the precedence (a declared value is what gets
// read) — the half a future "simplification" of the lift could silently
// invert — and, since #23543 F3, the lift's: the shadowed loose key is
// deleted at this seam, or the stored row would not parse at all.
it('reads the declared value, not its loose counterpart, when both are present', async () => {
const { ctx, scheduled } = fakeJobCtx();
registerWaitNode(engine, ctx);
Expand All @@ -919,6 +921,8 @@ describe('wait config graduation — legacy loose `config` still works via the c
],
});

const pause = (await engine.getFlow('wait_flow'))!.nodes.find((n) => n.id === 'pause')!;
expect(pause.config).toEqual({});
const before = Date.now();
await engine.execute('wait_flow');
// 1h (declared) — not 9h (loose). Same precedence the deleted `??` had.
Expand Down
7 changes: 4 additions & 3 deletions packages/services/service-automation/src/builtin/wait-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,10 @@ export function registerWaitNode(engine: AutomationEngine, ctx: PluginContext):
// The loose `config.*` back door this used to also read — six keys, two of
// them (`duration`, `signal`) spellings the spec never declared — graduated
// into the ADR-0087 D2 conversion layer as
// `flow-node-wait-event-config-lift` (#4045). It is rewritten at load,
// including the `registerFlow` rehydration seam, so there is nothing left
// to fall back to here (PD #12: no consumer-side fallbacks).
// `flow-node-wait-event-config-lift` (#4045). Since 18 the flow parse
// refuses those keys at authoring, and a stored flow's are rewritten at
// the `registerFlow` rehydration seam, so there is nothing left to fall
// back to here (PD #12: no consumer-side fallbacks).
//
// ── REVERSAL (#17928) ────────────────────────────────────────────────
// Until this change the two lines below read `(node.waitEventConfig ?? {})`
Expand Down
6 changes: 3 additions & 3 deletions packages/services/service-automation/src/run-summary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -943,7 +943,7 @@ describe('uncountable effects (#4354 follow-up)', () => {
name: 'f', label: 'f', type: 'autolaunched',
nodes: [
{ id: 'start', type: 'start', label: 'S' },
{ id: 'push', type: 'connector_action', label: 'P', config: { connectorId: 'crm', actionId: 'push', input: {} } },
{ id: 'push', type: 'connector_action', label: 'P', connectorConfig: { connectorId: 'crm', actionId: 'push', input: {} } },
{ id: 'end', type: 'end', label: 'E' },
],
edges: [{ id: 'e1', source: 'start', target: 'push' }, { id: 'e2', source: 'push', target: 'end' }],
Expand Down Expand Up @@ -977,7 +977,7 @@ describe('uncountable effects (#4354 follow-up)', () => {
name: 'f', label: 'f', type: 'autolaunched',
nodes: [
{ id: 'start', type: 'start', label: 'S' },
{ id: 'push', type: 'connector_action', label: 'P', config: { connectorId: 'crm', actionId: 'push', input: {} } },
{ id: 'push', type: 'connector_action', label: 'P', connectorConfig: { connectorId: 'crm', actionId: 'push', input: {} } },
{ id: 'end', type: 'end', label: 'E' },
],
edges: [{ id: 'e1', source: 'start', target: 'push' }, { id: 'e2', source: 'push', target: 'end' }],
Expand All @@ -1003,7 +1003,7 @@ describe('uncountable effects (#4354 follow-up)', () => {
name: 'child', label: 'child', type: 'autolaunched',
nodes: [
{ id: 'start', type: 'start', label: 'S' },
{ id: 'push', type: 'connector_action', label: 'P', config: { connectorId: 'crm', actionId: 'push', input: {} } },
{ id: 'push', type: 'connector_action', label: 'P', connectorConfig: { connectorId: 'crm', actionId: 'push', input: {} } },
{ id: 'end', type: 'end', label: 'E' },
],
edges: [{ id: 'e1', source: 'start', target: 'push' }, { id: 'e2', source: 'push', target: 'end' }],
Expand Down
Loading
Loading