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
43 changes: 43 additions & 0 deletions .changeset/21898-flow-builtin-node-config-values-refused.md
Original file line number Diff line number Diff line change
@@ -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)

<!-- adr-0087: registered flow-builtin-node-config-values-refused -->

**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.<key>`), `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.
5 changes: 4 additions & 1 deletion packages/lint/src/validate-expressions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2353,14 +2353,17 @@ 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: [{
name: 'f',
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"]);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>,
patch: Record<string, unknown>,
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<string, unknown>, 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');
Expand All @@ -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');
Expand Down Expand Up @@ -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');
});
Expand Down Expand Up @@ -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');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>).template = 'crm.large_deal_won';
const result = await engine.execute('notify_flow');
expect(result.success).toBe(false);
expect(result.error).toContain('`template`');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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: [
Expand Down
Loading
Loading