Skip to content

Commit 78f841b

Browse files
feat(spec)!: the build doors refuse an undeclared key on a script / subflow node config, with its location (#22129)
Part of #21982 Clause-②: no (narrowing) **Draft, patch round 1 done** (claim revision `6050287134`). This PR lands the `script` / `subflow` half of the card. The card stays open for the remainder named below. ## What changes The build doors now refuse a key that a `script` or `subflow` node's executor contract does not declare. They refuse it at its location, in the existing family of flow slot refusal codes. - **The doors:** `FlowSchema.parse` / `defineFlow()`, `defineStack`, `objectstack validate`, `objectstack compile`, an artifact's parse, the metadata save door's `flow` type schema, and `registerFlow`, which parses `FlowSchema` first. - **The code:** the existing `node-config-refused-by-contract`, `params: { nodeType, key }`. There is one refusal per undeclared key, anchored at the key (`nodes.N.config.bogusKey`), and its message is the contract's own sentence. - **The edit:** `packages/spec/src/automation/flow-node-config-refusals.ts`, the builtin branch of `flowNodeConfigRefusals`. It judges key membership where `builtinKeysJudged(nodeType)` holds. That is a builtin contract whose type is in the spec's own schemaless class, `SCHEMALESS_NODE_CONFIG_SCHEMAS`. - **Why the narrow lift:** a schemaless descriptor publishes no `configSchema`, so `registerFlow`'s undeclared-key walk skips it. Its executor still parses the strict contract, so it refuses the node at every run. - **Unchanged:** `getBuiltinNodeConfigContracts()` keeps its 13 entries, and no new code joins `FLOW_SLOT_REFUSAL_CODES`. - **Premise corrected:** `builtinValueJudged`'s docblock said "registration refuses an undeclared key against the descriptor". That was false for exactly these two types. The docblock now says which door owns which key. - **Comments made true:** the `FlowSchema` header in `flow.zod.ts` and the `node-config-refused-by-contract` docblock in `flow-node-expression-paths.ts` now name each arm that judges key membership: approval whole (#21850), builtin values (#21898), and `script` / `subflow` keys. - No `service-automation` source line moves, and there is no second key check anywhere. ## Built-ins: which get the key refusal, and why (measured at `15ec50e528`) | type | in `getBuiltinNodeConfigContracts` | descriptor `configSchema` | contract strict | executor parses it | key refusal here | |:--|:--|:--|:--|:--|:--| | `script` | yes | none | `strictObject` | yes (`screen-nodes.ts` `parseNodeConfig`) | **yes** | | `subflow` | yes | none | `strictObject` | yes (`subflow-node.ts` `parseNodeConfig`) | **yes** | | `decision` | no | none | `strictObject` | no: its executor reads `conditions[]` raw | no. An undeclared key fails no run. `decisionShapeRefusals` judges the `conditions` shape, not keys. | | `wait`, `connector_action` | no | none | their contracts are the FlowNode sibling blocks `waitEventConfig` / `connectorConfig`, `strictObject` inside `FlowNodeSchema` | they read the sibling block, not `config` | no. `FlowSchema` already refuses an unknown key in those blocks. | | `get_record`, `create_record`, `update_record`, `delete_record`, `notify`, `http`, `screen`, `map`, `loop`, `parallel`, `try_catch` | yes | yes | `strictObject` (all 11) | yes | no: the remainder, below | | `assignment` | no | yes (keyValue map) | no: open top-level variable names | no single contract | no | ## The remainder: the card stays open for it Triage's direction step 2 covers every builtin whose executor contract is strict. All 13 are. This PR takes the two schemaless ones, by the seat's ruling `6050287134`. - **Remainder 1, the 11 descriptor-`configSchema` builtins at the build doors:** `get_record`, `create_record`, `update_record`, `delete_record`, `notify`, `http`, `screen`, `map`, `loop`, `parallel` and `try_catch`. - They have the same gap at the build doors. Measured at this branch's round-0 head `f281d801f` on `examples/app-showcase`, flow `showcase_task_completed`, node `notify`, with `config.bogusKey: 1`: - `objectstack validate` exits 0; - `objectstack compile` exits 0, and the artifact carries `"bogusKey":1`; - `registerFlow` refuses the same node: "Flow 'p' rejected: 1 undeclared config key(s). … unknown config key `bogusKey` at config.bogusKey". - So boot drops the flow with a warning, and the build never says so. - **Remainder 2, an open design choice, not decided here:** once the spec arm covers those 11 types, who judges a builtin's undeclared key at `registerFlow`? The spec arm pre-empts registration's descriptor walk, and with it that walk's pinned prescriptions (`service-automation` `config-unknown-keys.test.ts`). ## Census first (triage step 1): no writer found | corpus | read at | `script` nodes | `subflow` nodes | with a key outside the contract | |:--|:--|:--|:--|:--| | this repo: `examples/**`, `packages/platform-objects/**`, `packages/apps/**`, `packages/create-objectstack/**` (templates), `skills/**`, `content/docs/**` | `15ec50e528` | 6 | 2 | 0 | | hotcrm, whole tree | `c9678036d9` | 0 | 5 | 0 | | objectui flow designer `FLOW_NODE_CONFIG` | pin `a58626c88d` (same file at objectui `main` `9990f9e122`) | form writes `function`, `inputs`, `outputVariable` | form writes `flowName`, `input`, `outputVariable` | 0 (its `timeoutMs` field writes the node; the five retired `script` keys sit behind a `showWhen` no field satisfies) | | objectui designer seeds `defaultNodeExtras` | `a58626c88d` | empty `config` | empty `config` | 0 | | objectui console preview samples | `a58626c88d` | 4 | 0 | 0 | - **Method:** a TypeScript-AST scan for object literals carrying `id` and `type: 'script'` / `'subflow'`, reading the keys of their `config`. Code fences in `.md` / `.mdx` and `.json` files were parsed too. - **Control:** over this whole repo, tests included, the same scan finds 103 nodes and flags 17. All 17 are fixtures: - 9 in the D2 conversion fixtures (`conversions/registry.ts`); - 5 in `lint` tests; - 3 test-double keys in `service-automation` `engine.test.ts`, repaired below. ## Doors, measured - **`objectstack validate` / `compile`.** Built CLI at round-0 head `f281d801f`, `examples/app-showcase` node `summarize` (`script`), one edit: `function: 'summarizeCompletedTask' , bogusKey: 1,`. - Control: validate exit 0, compile exit 0, and the artifact has no `bogusKey`. - With `bogusKey`: validate **exit 1**, compile **exit 2**, and no artifact is written. Both print the refusal at path `nodes, 1, config, bogusKey`. - The mutation was made with `scripts/ablation-replace.mjs`: anchor 1 → 0, then restored to the HEAD blob, `git diff HEAD` empty. - **`registerFlow`** (real builtin executors): - `script` / `subflow` with `bogusKey` are refused at `nodes.1.config.bogusKey`; - both controls register; - a `script` `functionName` alias registers: it is converted before the parse; - `http` `bogusKey` is still refused by the descriptor walk. - **Pinned in `flow-builtin-node-config-keys.test.ts`** (19 tests): - the refusal at `FlowSchema` (also inside a region body), `defineStack` (`STACK_SCHEMA_INVALID` 422 at `flows.1.nodes.1.config.bogusKey`), `ObjectStackDefinitionSchema`, the save door's `flow` type schema and an artifact parse; - the controls: no extra key; a descriptor type's key still left to registration (`http`, `create_record`, `screen`); `decision`; a retired `script` key keeps its tombstone path. - **Reverse verification** at round 0, `builtinKeysJudged` mutated to `return false`: 12 of 19 went red and the 7 controls held. It was restored to the HEAD blob. ## Cross-lane fixtures repaired (claim revision `6050287134`) - **`service-automation`, test only:** - The doubles in `engine.test.ts` ("should execute unconditional branches in parallel", "should fail when parameter type is wrong") and in `input-schema-retry-parity.test.ts` now register under the type `probe_step`, executor and nodes alike, never the builtin `script`. - The `function: 'noop'` filler went with them. - `inputSchema` reads top-level config keys, which a real `script` executor refuses. - **`lint`:** `validateStackExpressions` keeps the pre-conversion tolerance it declares. - The filter in `validate-expressions.ts` also hands the judge's undeclared-key refusal for a `script` node's `functionName` alias to the callable check, which already reads that alias. - A new pin holds that every other undeclared `script` key is still refused there (`bogusKey`, on a canonical and on an alias source). - The changeset gains `'@objectstack/lint': patch`. **Red → green.** Round 0 at `f281d801f` had 4 red in `service-automation` and 2 red in `lint`. All six now pass at `ef0dfb44d`: - `engine.test.ts` › "should execute unconditional branches in parallel" ✓ - `engine.test.ts` › "should fail when parameter type is wrong" ✓ - `input-schema-retry-parity.test.ts` › "never executes a node whose config mis-types its declared inputSchema — on ANY attempt" ✓ - `input-schema-retry-parity.test.ts` › "still retries a VALID flow normally …" ✓ - `validate-expressions.test.ts` › "accepts a script node that names a callable via the functionName alias" ✓ - `validate-expressions.test.ts` › "a `script` with no `function` is ONE finding, the callable check's …" ✓ ## ADR-0087 - **D3 entry:** `18.flow-script-subflow-config-undeclared-keys-refused.ts`. - **Rationale fragment:** step 18 `order: 87`, re-read on `origin/main` `8fc50b764` (the merged base): its highest order is 86, and open PRs #22103 and #22094 hold 86 and 85 at their heads. - **Registry:** `registry.ts` was regenerated by `gen:migration-registry`. - **Changeset:** `@objectstack/spec` `minor`, BREAKING, with the `registered` marker, plus `@objectstack/lint` `patch`. `.changeset/pre.json` is absent on `origin/main`. ## Merge - `origin/main` `8fc50b764` was merged by `scripts/pm/os-regen-merge.sh` as merge commit `521e16f1f`, with parents `f281d801f` and `8fc50b764`. There were no conflicts. - Step 2 took `main`'s side of the generated artifacts that `main` moved, and there was nothing more to commit. - After a spec build on the merged tree, `check:generated` reported all 15 artifacts up to date. The delta against `main` was exactly this PR's 5 round-0 files. - `gen:schema` was not run. - Round 1's edits are commit `ef0dfb44d` on top. ## Verification at `ef0dfb44d` - **Spec:** - `check:generated`: all 15 artifacts up to date. - The pin files `flow-builtin-node-config-keys.test.ts` and `flow-builtin-node-config-values.test.ts`: 63/63. - Round 0's whole spec suite at `f281d801f`: 624 files, 18628 tests passed. - **lint:** the whole suite, 123 files, 5689/5689. - **service-automation:** the whole suite, 175 files, 2120/2120. The first attempt collided with a concurrent gate run that left `@objectstack/spec/automation` unresolvable for 17 files; it was re-run alone. - **Typecheck:** `@objectstack/lint` exit 0 and `@objectstack/service-automation` exit 0, both including `check:test-typecheck`, over a closure rebuilt with declarations. - **eslint, narrowed** (`--no-inline-config --format json`) over the diff's 10 `.ts` files: 10 files, 0 errors, 0 warnings. The population is read from the json count. `parserOptions.project` and `projectService` are null for each file, so there is no type-aware linting and no untouched file's verdict can move. - **Gates:** `dispatch-gates --commands --repo objectstack-ai/objectstack` derives 92 commands from the merged head. All ran, with exit codes captured before any pipe. The `--ran` reconciliation reads 91 run, 1 NOT-MEASURED, 0 UNRUN (`check:dts-closure` recorded at its re-run). - 91 exit 0. - `check:dual-build-cjs-loads`: exit 3, PREREQUISITE NOT MET (packages outside this worktree's build closure have no `dist`). It is read from CI, as are round 0's `cli` published-subpath pins. - `check:dts-closure` first exited 1, naming exactly the 19 closure packages built with `OS_SKIP_DTS=1` for the test runs. Re-run after the closure was rebuilt with declarations, it exits 0: 169/169 declaration files across 71 built packages. --- _Generated by [Claude Code](https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ef1fcb2 commit 78f841b

11 files changed

Lines changed: 566 additions & 39 deletions
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/lint': patch
4+
---
5+
6+
A `script` or `subflow` flow node whose `config` carries a key its executor contract does not declare is refused at parse, with a location, in the contract's own words: a `script` `bogusKey`, a `subflow` `timeoutMs` written inside `config`, and the like no longer pass the build doors and registration and then fail every run.
7+
8+
Clause-②: no (narrowing)
9+
10+
<!-- adr-0087: registered flow-script-subflow-config-undeclared-keys-refused -->
11+
12+
**BREAKING**: an accept-set narrowing on a published authoring surface, shipped as `minor` under the launch-window convention for accept-set narrowings.
13+
14+
**Why.** The `script` and `subflow` executors parse the node's `config` against a strict contract (`ScriptConfigSchema`, `SubflowConfigSchema`) before they act, and refuse the node on an undeclared key. No door before the run judged one: `registerFlow`'s undeclared-key check reads the node type descriptor's `configSchema`, and these two descriptors publish none, while the build doors' executor-contract arm judged required keys and present values but not key membership. So a `script` node carrying `bogusKey` passed `FlowSchema.parse`, `objectstack validate` and `objectstack compile` (compile copied it into `dist/objectstack.json`), registered, and failed every run that reached the node: ``script 'n': config does not satisfy the script contract — config: Unrecognized key(s) on this script node config: `bogusKey` ``.
15+
16+
**What is refused.** A `script` node, at any depth, whose config carries a key other than `function`, `inputs` and `outputVariable`, or a `subflow` node whose config carries a key other than `flowName`, `input` and `outputVariable`. The refusal is the existing closed-set code `node-config-refused-by-contract`, `params: { nodeType, key }`, one per undeclared key, anchored at the key (`nodes.N.config.bogusKey`), 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.
17+
18+
**What stays as it was.**
19+
20+
- Every other builtin node type: its undeclared keys are judged at registration against its descriptor's `configSchema`, with that check's own prescriptions, and the build doors do not judge them.
21+
- `decision`: it publishes no descriptor `configSchema` either, but its executor parses no contract, so an undeclared key fails no run and stays unjudged.
22+
- A retired `script` key (`actionType`, `template`, `recipients`, `variables`, `script`) keeps its tombstone path.
23+
- A spelling an ADR-0087 D2 conversion still rewrites at load (`functionName` and `input` on a `script`, `flow` on a `subflow`) is converted before the judge at every door that converts first (`defineStack`, `os validate`, `os compile`, `registerFlow`). Met by a direct `FlowSchema.parse` or `defineFlow()`, it is refused like any other undeclared key, as its missing canonical key already was.
24+
25+
## FROM → TO
26+
27+
| you wrote | write instead |
28+
|:--|:--|
29+
| a typo of a declared key (`funtion`, `outputVariabel`) | the declared key: `function`, `inputs`, `outputVariable` on a `script`; `flowName`, `input`, `outputVariable` on a `subflow` |
30+
| a value the function or child flow should receive, as its own config key (`config: { function: 'f', taskId: '{record.id}' }`) | inside the input map: `config: { function: 'f', inputs: { taskId: '{record.id}' } }` (`input` on a `subflow`) |
31+
| a `subflow` `config.timeoutMs` | on the node: `{ id, type: 'subflow', timeoutMs: 30000, config: { … } }` |
32+
| a key nothing reads | delete it |
33+
34+
**The one-line fix: rename, move or delete the key the refusal names.** The runtime never ran such a node, so the fix changes nothing a working flow does.
35+
36+
**Who is affected, measured.** At `15ec50e528`, every `script` and `subflow` node authored in this repository's examples, platform objects, apps, scaffolding templates, skills and docs (8 nodes: 6 `script`, 2 `subflow`) carries only declared keys, and so does every one in hotcrm at `c9678036d9` (5 `subflow`, no `script`). The Studio flow designer at the pinned objectui `a58626c88d` writes only declared keys for both types (its `timeoutMs` field writes the node, not `config`), and seeds a new node with an empty `config`. 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.
37+
38+
**`@objectstack/lint`.** `validateStackExpressions` keeps the pre-conversion tolerance it declares: on a raw source, a `script` node's `functionName` alias stays the callable check's to read, not an undeclared-key error, while every other undeclared `script` key is refused there as at the build doors.
39+
40+
### The kit
41+
42+
- **The refusal.** The key half of the executor-contract arm of `flowNodeConfigRefusals` in `automation/flow-node-config-refusals.ts`, judged for the builtins in the spec's schemaless class (`SCHEMALESS_NODE_CONFIG_SCHEMAS`) that have an executor contract; no new code joins `FLOW_SLOT_REFUSAL_CODES`, and `getBuiltinNodeConfigContracts()` keeps its 13 entries.
43+
- **The ledger.** The D3 semantic entry `flow-script-subflow-config-undeclared-keys-refused` (protocol 18). No key is removed, so there is no tombstone, and there is no D2 conversion: the platform cannot know what an undeclared key was meant to be.

‎packages/lint/src/validate-expressions.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4535,6 +4535,21 @@ describe('node config an executor requires (#20316)', () => {
45354535
expect(found.map((i) => i.where)).toEqual(["flow 'config_flow' · node 'n' (script) callable"]);
45364536
expect(errorsOf(stackWith({ type: 'script', config: { functionName: 'recalc_totals' } }))).toHaveLength(0);
45374537
});
4538+
4539+
it('a `script` key its contract does not declare is still refused — only the `functionName` alias is the callable check\'s', () => {
4540+
const config = { function: 'recalc_totals', bogusKey: 1 };
4541+
expect(errorsOf(stackWith({ type: 'script', config })).map((i) => [i.where, i.message, i.source])).toEqual([
4542+
["flow 'config_flow' · node 'n' (script) config.bogusKey", flowNodeConfigRefusals('script', config)[0].message, ''],
4543+
]);
4544+
// On a raw alias source the judge names `function`, `functionName` and
4545+
// `bogusKey`; the pass hands the first two to the callable check and
4546+
// keeps the third.
4547+
const aliased = { functionName: 'recalc_totals', bogusKey: 1 };
4548+
expect(flowNodeConfigRefusals('script', aliased).map((r) => r.path).sort()).toEqual(['bogusKey', 'function', 'functionName']);
4549+
expect(errorsOf(stackWith({ type: 'script', config: aliased })).map((i) => i.where)).toEqual([
4550+
"flow 'config_flow' · node 'n' (script) config.bogusKey",
4551+
]);
4552+
});
45384553
});
45394554

45404555
/**

‎packages/lint/src/validate-expressions.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,8 +1714,15 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
17141714
// #4343): this pass may be handed a pre-conversion source, and that
17151715
// check reads what such a source spells — the `functionName` alias,
17161716
// the retired dispatch keys — and names each, where the judge would
1717-
// only see `function` absent.
1718-
.filter((configRefusal) => !(nodeType === 'script' && configRefusal.path === 'function'));
1717+
// only see `function` absent. Since the judge also refuses a key a
1718+
// `script`'s contract does not declare (#21982), it would name that
1719+
// same `functionName` alias undeclared too, so that one refusal is
1720+
// the callable check's as well: the pass keeps the pre-conversion
1721+
// tolerance it declares. Every other undeclared key stays refused.
1722+
.filter((configRefusal) => !(nodeType === 'script' && (
1723+
configRefusal.path === 'function'
1724+
|| (configRefusal.code === 'node-config-refused-by-contract' && configRefusal.path === 'functionName')
1725+
)));
17191726
for (const configRefusal of configRefusals) {
17201727
issues.push({
17211728
where: `${at} · node '${node.id}' (${nodeType}) config.${configRefusal.path}`,

‎packages/services/service-automation/src/engine.test.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2337,8 +2337,11 @@ describe('AutomationEngine - Parallel Branch Execution', () => {
23372337
// sequential engine satisfies just as well.
23382338
const trace: string[] = [];
23392339

2340+
// A test double under a type of its own, never the builtin `script`:
2341+
// `delay` is this double's key, and the `script` contract refuses an
2342+
// undeclared key at the flow parse that `registerFlow` runs first.
23402343
engine.registerNodeExecutor({
2341-
type: 'script',
2344+
type: 'probe_step',
23422345
async execute(node) {
23432346
trace.push(`enter:${node.id}`);
23442347
const delay = (node.config as any)?.delay ?? 0;
@@ -2355,8 +2358,8 @@ describe('AutomationEngine - Parallel Branch Execution', () => {
23552358
type: 'autolaunched',
23562359
nodes: [
23572360
{ id: 'start', type: 'start', label: 'Start' },
2358-
{ id: 'branch_a', type: 'script', label: 'Branch A', config: { function: 'noop', delay: 10 } },
2359-
{ id: 'branch_b', type: 'script', label: 'Branch B', config: { function: 'noop', delay: 10 } },
2361+
{ id: 'branch_a', type: 'probe_step', label: 'Branch A', config: { delay: 10 } },
2362+
{ id: 'branch_b', type: 'probe_step', label: 'Branch B', config: { delay: 10 } },
23602363
{ id: 'end', type: 'end', label: 'End' },
23612364
],
23622365
edges: [
@@ -2442,8 +2445,11 @@ describe('AutomationEngine - Node Input Schema Validation', () => {
24422445
});
24432446

24442447
it('should fail when parameter type is wrong', async () => {
2448+
// A test double under a type of its own: `inputSchema` reads TOP-LEVEL
2449+
// config keys, and the builtin `script` contract refuses an undeclared
2450+
// `count` at the flow parse before this check could run.
24452451
engine.registerNodeExecutor({
2446-
type: 'script',
2452+
type: 'probe_step',
24472453
async execute() {
24482454
return { success: true };
24492455
},
@@ -2457,9 +2463,9 @@ describe('AutomationEngine - Node Input Schema Validation', () => {
24572463
{ id: 'start', type: 'start', label: 'Start' },
24582464
{
24592465
id: 'validated',
2460-
type: 'script',
2466+
type: 'probe_step',
24612467
label: 'Validated',
2462-
config: { function: 'noop', count: 'not_a_number' },
2468+
config: { count: 'not_a_number' },
24632469
inputSchema: {
24642470
count: { type: 'number', required: true },
24652471
},

‎packages/services/service-automation/src/input-schema-retry-parity.test.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,11 @@ function countingFlowEngine(opts: {
5555
const engine = new AutomationEngine(createTestLogger());
5656
const runs = { count: 0 };
5757

58+
// A test double under a type of its own, never the builtin `script`:
59+
// `inputSchema` reads TOP-LEVEL config keys, and the `script` contract
60+
// refuses an undeclared key at the flow parse `registerFlow` runs first.
5861
engine.registerNodeExecutor({
59-
type: 'script',
62+
type: 'probe_step',
6063
async execute() {
6164
runs.count++;
6265
return opts.executeResult ? opts.executeResult(runs.count) : { success: true };
@@ -72,11 +75,9 @@ function countingFlowEngine(opts: {
7275
{ id: 'start', type: 'start', label: 'Start' },
7376
{
7477
id: 'work',
75-
type: 'script' as any,
78+
type: 'probe_step',
7679
label: 'Work',
77-
// `function` is the key the script executor contract requires;
78-
// the flow parse refuses a script node without it (#20316).
79-
config: { function: 'noop', ...opts.config },
80+
config: { ...opts.config },
8081
inputSchema: opts.inputSchema,
8182
},
8283
{ id: 'end', type: 'end', label: 'End' },

0 commit comments

Comments
 (0)