Skip to content

Commit 3e72f93

Browse files
fix(spec/automation)!: refuse a $-named outputVariable, and a $-named errorVariable other than $error, at authoring (#22569)
Fixes #22502 Clause-②: no (narrowing) ## What this does The `$` names are the flow engine's at the binding keys too, so a flow can no longer bind a `$` name that a text slot then refuses to read. - A node's `outputVariable` (`get_record`, `create_record`, `map`, `script`, `subflow`) refuses a name that starts with `$`, the engine's own names included (`outputVariable: '$record'` would overwrite the trigger record). - A `try_catch` node's `errorVariable` refuses every `$` name except `$error`, its default. - The refusal names the remedy: the same name without the `$`, read as `{{ name }}`. For `errorVariable: '$caught'` that is `caught`, read as `{{ caught.message }}`, or deleting the key and reading the default `{{ $error.message }}`. One rule, composed into all six keys: `flowBoundVariableNameSchema` in the new package-internal leaf `packages/spec/src/automation/flow-bound-variable-name.ts` (not in `automation/index.ts`, so there is no new public export; `check:api-surface` is green). - It is a `regex` check, not a `.refine()`. `z.toJSONSchema()` emits it as `pattern`, so the published `json-schema/**` refuses exactly what the parse refuses, and `dropped-refinements.baseline.json` gains no row. - Each executor parses its config against the same contract. So `FlowSchema.parse`, `registerFlow`, `objectstack validate`, `defineStack` and the run itself all refuse such a name through `flowNodeConfigRefusals`, anchored at `nodes.N.config.outputVariable` / `nodes.N.config.errorVariable`, region bodies included. - It reads the `$` prefix the engine's closed list implies, never a copy of the list. `FLOW_ENGINE_VARIABLES` stays package-internal in `flow-text-slot-template.ts`, untouched. A binding must not claim any name in the engine's namespace, whichever name that is. - ⛔ The text-slot judge is not widened (triage `6084430227` rules that option out). ## The PM's mechanism assumptions, measured at `b53b949a15` 1. **`try_catch.errorVariable`** was `z.string().default('$error')` at `control-flow.zod.ts:329` and accepted any string. Confirmed. 2. **`outputVariable` is declared at five sites, each its own `z.string().optional()`. They share no schema:** - `builtin-node-config.zod.ts:445` (`get_record`), `:471` (`create_record`), `:991` (`map`); - `schemaless-node-config.zod.ts:305` (`script`), `:384` (`subflow`). - `flow.zod.ts` and `flow-function.zod.ts` only mention it in comments. Measured with `git grep -nE '^\s+[a-zA-Z]*(Variable|Var)s?\s*:\s*z\.' -- packages/spec/src`. 3. **The closed list** is `FLOW_ENGINE_VARIABLES` in `packages/spec/src/automation/flow-text-slot-template.ts`. It is unexported on purpose, and the module is `export *`-ed from the barrel. So this card reads the prefix rule it implies rather than publishing the list. 4. **In-repo authors** of a `$`-named `errorVariable` / `outputVariable` other than `$error`, measured with `git grep -nP 'Variable\W{0,8}\$[a-zA-Z_]' -- .` over the whole tree (13 hits, 10 of them `$error`). All three non-`$error` hits are test fixtures, and all three are renamed here: - `packages/spec/src/automation/region-normalization.test.ts:127`: `'$err'` to `'err'`; - `packages/spec/src/automation/flow-builtin-node-config-keys.test.ts:344`: `'$err'` to `'err'`; - `packages/services/service-automation/src/throw-arm-error-refresh.test.ts:77`: `'$caught'` to `'caught'`. - Every authored `errorVariable` in `examples/app-showcase` (2), `content/docs` (2) and the `service-automation` README (1) is `$error`, which stays legal. - No example or doc binds a `$`-named `outputVariable`. - The pinned objectui (`47b1f0bb71`) authors `errorVariable: '$error'` only (`apps/console/src/preview-samples.ts:244`). 5. **Engine readers.** No engine reader breaks. - `try-catch-node.ts:105` (`cfg.errorVariable || '$error'`) still gets `$error`. - The executors write the author's `outputVariable` verbatim with `variables.set` (`crud-nodes.ts`, `map-node.ts`, `screen-nodes.ts`, `subflow-node.ts`). - No engine code authors a `$`-named binding. - The only `packages/services/**` edit is the one fixture above. It is not an engine change: under the new contract `registerFlow` refused that flow (reverse check below). ## ADR-0087 disposition - **D3 entry:** `packages/spec/src/migrations/entries/semantic/18.flow-binding-variable-dollar-name-refused.ts` (protocol 18), plus its step-18 rationale fragment (`order: 92`). - **Projections regenerated:** `gen:migration-registry`, `gen:spec-changes`, `gen:upgrade-guide`. - **No D2 conversion:** the bare name may already be bound in the flow, and the reads of the old name sit in every dialect a flow string speaks. - **Guidance:** the printed guidance carries no tracker number. `test/migrate-meta-engine-guidance.test.ts` is green (3/3). - **Changeset:** `.changeset/22502-flow-binding-variable-dollar-name-refused.md`, `@objectstack/spec` `major` in pre mode, with `Clause-②: no (narrowing)` and the `registered flow-binding-variable-dollar-name-refused` marker. `check-adr-0087-registration` reads `[major+BREAKING+clause-②-narrowing] registered flow-binding-variable-dollar-name-refused (new here)`. ## Tests (at `ff2832a7bb`, after the one `origin/main` merge through `os-regen-merge.sh` and its regeneration commit) - **New:** `packages/spec/src/automation/flow-bound-variable-name.test.ts`, 26 cases. - `$x` is refused with the remedy on each of the five `outputVariable` contracts, and every `$` name with it (`$record`, `$error`, `$`, `$$x`). - Controls: `x`, `a$b` and an absent key are accepted. A non-string keeps its type refusal. - `errorVariable: '$caught'` is refused with the remedy, and so are `$record`, `$errors`, `$error.x` and `$`. Controls: the default `$error`, an explicit `$error` and `caught` are accepted. - `FlowSchema` refuses at `nodes.1.config.errorVariable`, and inside a region at `nodes.1.config.try.nodes.0.config.outputVariable`. - `defineStack` refuses with `{ code: 'STACK_SCHEMA_INVALID', status: 422 }`, and the save door at the key. - `textSlotTemplateRefusal('Failed: {{ caught.message }}')` passes, and a flow binding `caught` with that text in its catch region parses clean. - The published JSON Schema carries a `pattern` on all six keys that accepts and refuses the same names. - The D3 entry and its rationale fragment are registered. - `pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/automation src/migrations`: **39 files, 1416 tests passed**. - `pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2` (whole suite): **180 files, 2259 tests passed**. - `pnpm --filter @objectstack/cli exec vitest run --project integration test/migrate-meta-engine-guidance.test.ts` (after its closure build): **3 passed**. The file lives in the `integration` project. A first `--project unit` call selected nothing; that call is not counted as a measurement. - `pnpm --filter @objectstack/spec typecheck` and `pnpm --filter @objectstack/service-automation typecheck` both exit 0, `check:test-typecheck: OK`. - `pnpm --filter @objectstack/spec check:generated`: **all 15 generated artifacts up to date**. **Ablation** (committed fix, mutation and restore through `scripts/ablation-replace.mjs`, wrap mode). The spec tests import `src` directly, so no dist leg applies. - The rule's regex was replaced with one that accepts everything. The anchor fell from 1 to 0, and the blob moved from `5211426e99fb` to `d0cb32780409`. - Result: **16 failed, 10 passed** of 26. Red: every refusal, every door and the `pattern` pin. Green: the controls, the remedy-reads pin, the non-string case and the ledger pins. - Restore was proven: blob equals HEAD `5211426e99fb`, and `git diff HEAD` is empty. **Reverse check of the services fixture:** - `'caught'` was put back to `'$caught'`, then `throw-arm-error-refresh.test.ts` was run against the rebuilt spec. - Result: **1 failed**, a `ZodError` at `nodes.3.config.errorVariable` thrown from `AutomationEngine.canonicalizeStoredFlow` inside `registerFlow`. So the fixture rename was owed. - Restore was proven: blob equals HEAD `b4f38da11225`. ## Gates - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no paths, merge base `6a3f82efa`) derived 116 commands, reconciled with `--ran`. - `pnpm check:dual-build-cjs-loads` is **NOT MEASURED** (reason: it needs a whole-workspace build, which this dispatch rules out). It is declared to CI. - Full readings, including the artifact-roster block, are in the report on #22502. ## Acceptance notes - **The same class, on sibling binding keys, is not in this PR.** `FlowSchema.parse` at this head still accepts a `$` name on: - `loop` / `map` `iteratorVariable` and `indexVariable`; - a `screen` `idVariable`; - a declared flow variable's `name`; - an `assignment` target key. `NotifyConfigSchema` refuses a read of one (`{{ $row.name }}`). The triage ruling scoped this card to `errorVariable` and `outputVariable`, so the siblings are reported for the family close-out rather than widened here. - `service-automation`'s executor descriptors (`configSchema` on `crud-nodes.ts`, `map-node.ts`, `try-catch-node.ts`) still describe these keys as plain strings. The spec contract is the judge at every door, and the descriptor walk stands aside for builtins. Noted, not changed. - `engine.ts`'s `buildSubflowResumeSignal` comment calls the reserved-name check a false positive "on an oddly-named" `outputVariable`. After this narrowing such a name cannot be authored. Whether `engineBuilt` is still needed there for another reason was not measured. Noted, not changed. - The hand-written `content/docs/automation/flows.mdx` gains one paragraph beside the `try_catch` example saying where the `$` names belong. --- _Generated by [Claude Code](https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 86f53a4 commit 3e72f93

17 files changed

Lines changed: 535 additions & 29 deletions
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
'@objectstack/spec': major
3+
---
4+
5+
A flow node's `outputVariable` (`get_record`, `create_record`, `map`, `script`, `subflow`) refuses a variable name that starts with `$`, and a `try_catch` node's `errorVariable` refuses every `$` name except the engine's own `$error`, its default. The refusal names the remedy: the same name without the `$`, read as `{{ name }}`.
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: registered flow-binding-variable-dollar-name-refused -->
10+
11+
**BREAKING**: an accept-set narrowing on a published authoring surface, shipped as `major` on the v18 line (`.changeset/pre.json` is open on `main` in `next` pre mode, so the release is `18.0.0-next.*`).
12+
13+
**Why.** The `$` names are the flow engine's own variables: it binds `$record`, `$runId`, `$flowName`, `$flowLabel` and `$error`, a flat-graph `loop` binds `$loopItems` and `$loopIndex`, and a resume signal may not write any `$` name. A flow text slot already refuses a `{{ }}` hole whose root is a `$` name the engine does not bind, and tells the author to drop the `$`. But the binding keys took any string, so a flow could bind `errorVariable: '$caught'` and then be refused `'Failed: {{ $caught.message }}'`. The two doors of one contract gave two answers. Now both give the text slot's answer.
14+
15+
**What is refused.**
16+
17+
- `outputVariable` on `GetRecordConfigSchema`, `CreateRecordConfigSchema`, `MapConfigSchema`, `ScriptConfigSchema` and `SubflowConfigSchema`: a name that starts with `$`. That includes the engine's own names, since a binding over `$record` would overwrite the trigger record for the rest of the run.
18+
- `errorVariable` on `TryCatchConfigSchema`: a name that starts with `$`, other than `$error`.
19+
- Each key states the rule as a JSON Schema `pattern`, so the published `json-schema/**` refuses what the parse refuses. The parse issue is an `invalid_format` (regex) issue at the key, and its message names the remedy.
20+
- `FlowSchema.parse`, `registerFlow` and `objectstack validate` refuse the flow at `nodes.N.config.outputVariable` / `nodes.N.config.errorVariable`, inside a region body too (`nodes.N.config.try.nodes.M.config.outputVariable`). A stored flow carrying one is skipped at boot with a warn naming it, and each executor's own contract parse refuses the node at run time.
21+
22+
**Unchanged.** `errorVariable` absent (it defaults to `$error`) or an explicit `errorVariable: '$error'`; any name that does not start with `$`, a `$` later in the name (`a$b`) included; an empty string, which every executor reads as no binding; and every hole the text-slot judge already admits. Non-string values keep the type refusal they had.
23+
24+
## FROM → TO
25+
26+
| you wrote | write instead |
27+
|:--|:--|
28+
| `errorVariable: '$caught'` with `message: 'Failed: {{ $caught.message }}'` | `errorVariable: 'caught'` with `'Failed: {{ caught.message }}'`, or drop `errorVariable` and write `{{ $error.message }}` |
29+
| `outputVariable: '$lead'` with `title: '{{ $lead.name }}'` | `outputVariable: 'lead'` with `title: '{{ lead.name }}'` |
30+
31+
**The one-line fix: name the variable without the `$`, and rename every read of it with it.**
32+
33+
No D2 conversion rewrites the name: the bare name may already be bound in the flow, and the reads of the old name sit in every dialect a flow string speaks (a text-slot hole, a CEL expression, a single-brace token in a value position), so the rename is the author's.
34+
35+
**Who is affected, measured.** The last published spec, `@objectstack/spec@17.7.0` (npm `latest`), types all six keys as plain strings and accepts any `$` name. This repository was measured with `git grep` over `packages`, `examples`, `skills`, `apps`, `content` and `docs`: the only `$`-named `errorVariable` / `outputVariable` other than `$error` were three test fixtures, all renamed here (`errorVariable: '$err'` in two `packages/spec` automation tests, `errorVariable: '$caught'` in a `service-automation` test). Every authored `errorVariable` in examples and docs is `$error`, which stays legal, and no example or doc binds a `$`-named `outputVariable`. The pinned `objectui` checkout uses `errorVariable: '$error'` only. Deployed metadata and other repositories were not measured.
36+
37+
### The kit
38+
39+
- **The rule.** `flowBoundVariableNameSchema` in `automation/flow-bound-variable-name.ts`, package-internal (no new public export), composed into the six keys. It reads the `$` prefix the engine's closed list implies, not a copy of the list.
40+
- **The ledger.** The D3 semantic entry `flow-binding-variable-dollar-name-refused` (protocol 18), with no D2 conversion.

‎content/docs/automation/flows.mdx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,12 @@ a key it does not declare, such as `maxRetry` for `maxRetries`, is refused at
858858
`objectstack validate` with a did-you-mean, rather than dropped to a default
859859
that never retries.
860860

861+
`errorVariable` defaults to `$error`, the engine's own variable, read as
862+
`{{ $error.message }}`. A name of your own is written without a `$` —
863+
`errorVariable: 'caught'`, read as `{{ caught.message }}` — because the `$`
864+
names belong to the engine: any other `$` name is refused at
865+
`objectstack validate`, and so is a `$`-named `outputVariable` on any node.
866+
861867
`catch` is optional in the schema, and omitting it is the trap: with no `catch`
862868
the container **fails** when the `try` region fails, so the failure propagates
863869
exactly as if nothing had been wrapped (measured — the no-`catch` run and the

‎content/docs/references/automation/builtin-node-config.mdx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ Value the variable takes: a CEL value envelope `{ dialect: 'cel', source }` eval
148148
| :--- | :--- | :--- | :--- |
149149
| **objectName** | `string` | ✅ | Object to insert into |
150150
| **fields** | `Record<string, any>` | optional | Field values to write on the new record: each key is a field name, each value a CEL value envelope or a literal |
151-
| **outputVariable** | `string` | optional | Flow variable bound to the created record |
151+
| **outputVariable** | `string` | optional | Flow variable bound to the created record — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` |
152152

153153

154154
---
@@ -195,7 +195,7 @@ A value: a CEL value envelope `{ dialect: 'cel', source }` evaluated by the expr
195195
| **filter** | `Record<string, any>` | optional | Field/value pairs to match (operator values like `{"$ne": null}` are preserved) |
196196
| **fields** | `string[]` | optional | Field projection — only these fields are read (default: all) |
197197
| **limit** | `number` | optional | Max records to return; >1 switches to a multi-record query |
198-
| **outputVariable** | `string` | optional | Flow variable the result is bound to |
198+
| **outputVariable** | `string` | optional | Flow variable the result is bound to — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` |
199199

200200

201201
---
@@ -212,7 +212,7 @@ A value: a CEL value envelope `{ dialect: 'cel', source }` evaluated by the expr
212212
| **indexVariable** | `string` | optional | Optional variable holding the current index |
213213
| **itemObject** | `string` | optional | When items are records, the object they belong to (exposes each item as the child's record) |
214214
| **input** | `Record<string, any>` | optional | Params passed to each item's subflow (interpolated per item) |
215-
| **outputVariable** | `string` | optional | Each item's subflow output, collected in order |
215+
| **outputVariable** | `string` | optional | Each item's subflow output, collected in order — bound to a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` |
216216

217217

218218
---

‎content/docs/references/automation/control-flow.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ const result = FlowRegionSchema.parse(data);
244244
| :--- | :--- | :--- | :--- |
245245
| **try** | `{ nodes: object[]; edges?: object[] }` | ✅ | Protected region |
246246
| **catch** | `{ nodes: object[]; edges?: object[] }` | optional | Handler region run when the try region fails |
247-
| **errorVariable** | `string` | optional (default: `"$error"`) | Variable holding the caught error in the catch region — a `TryCatchErrorValue`: `nodeId`, `message`, `code` when the failing node carried a platform-classified error code (ADR-0112 — branch on `$error.code` to tell "the row is already there" from "the store is down"), and `iteration` / `item` when the failure happened inside a loop body |
247+
| **errorVariable** | `string` | optional (default: `"$error"`) | Variable holding the caught error in the catch region — `$error` (the default), or a name without a leading `$` read as `{{ name.message }}`; any other `$` name is the flow engine's own and refused. The value is a `TryCatchErrorValue`: `nodeId`, `message`, `code` when the failing node carried a platform-classified error code (ADR-0112 — branch on `$error.code` to tell "the row is already there" from "the store is down"), and `iteration` / `item` when the failure happened inside a loop body |
248248
| **retry** | `{ maxRetries?: integer; backoffMs?: integer; backoffMultiplier?: number; maxRetryDelayMs?: integer; … }` | optional | Optional retry policy for the try region |
249249

250250
### Nested Shape: `TryCatchConfig.try`

‎content/docs/references/automation/schemaless-node-config.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ const result = DecisionConditionSchema.parse(data);
164164
| :--- | :--- | :--- | :--- |
165165
| **function** | `string` | ✅ | Registered function to call (defineStack(`{ functions }`)). Contractually pure — it returns a value a later declarative node persists |
166166
| **inputs** | `Record<string, any>` | optional | Inputs passed to the function (values interpolate `{token}` templates) |
167-
| **outputVariable** | `string` | optional | Flow variable the function's return value is bound to |
167+
| **outputVariable** | `string` | optional | Flow variable the function's return value is bound to — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` |
168168
| **actionType** | `never` | optional | [REMOVED] `script.config.actionType` was removed in @objectstack/spec 17 — none of its values did what it said. The two built-ins were logger-backed stubs that recorded the intent and delivered nothing under any configuration, and every other value was a second spelling of `config.function`. Replace it per branch: for `email` use a `notify` node (it delivers through the messaging service — the in-app inbox by default, real email once `@objectstack/plugin-email` is installed); for `slack` use a `connector_action` node with the Slack connector, or an `http` node posting to a webhook; for anything else, move the name into `config.function`. Run `os migrate meta --from 16` to list the mechanical edits for the shorthand case into `config.function`; `--write` applies the ones it can prove, and the stub and marker values are removed. |
169169
| **template** | `never` | optional | [REMOVED] `script.config.template` was removed in @objectstack/spec 17 — it fed only the logger-backed `email`/`slack` stubs, which never rendered or sent a message, so no template id was ever resolved. Delete the key. A `notify` node carries its own `title`/`message`, and stored templates live in the messaging service (`sys_notification_template`), not on the node. 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. |
170170
| **recipients** | `never` | optional | [REMOVED] `script.config.recipients` was removed in @objectstack/spec 17 — the addresses were logged, never messaged: the `email`/`slack` branches it fed delivered nothing. Use a `notify` node, whose `recipients` (user ids, field refs or addresses) reach the messaging service for real. 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. |
@@ -182,7 +182,7 @@ const result = DecisionConditionSchema.parse(data);
182182
| :--- | :--- | :--- | :--- |
183183
| **flowName** | `string` | ✅ | Flow invoked as this step (it may pause — approval / screen / wait) |
184184
| **input** | `Record<string, any>` | optional | Values passed to the subflow's input variables (interpolate `{token}` templates) |
185-
| **outputVariable** | `string` | optional | Parent flow variable the subflow's output is bound to |
185+
| **outputVariable** | `string` | optional | Parent flow variable the subflow's output is bound to — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` |
186186

187187

188188
---

‎docs/protocol-upgrade-guide.md‎

Lines changed: 4 additions & 1 deletion
Large diffs are not rendered by default.

‎packages/services/service-automation/src/throw-arm-error-refresh.test.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,10 @@ const ctxWith = (data: any): any => ({
3838

3939
describe('#14955 — the throw arm refreshes `$error` like the returned-failure arm', () => {
4040
it('a thrown failure with no fault edge of its own names ITSELF on the run-wide `$error`', async () => {
41-
// `tc` binds its caught error to `$caught`, deliberately NOT to `$error`,
41+
// `tc` binds its caught error to `caught`, deliberately NOT to `$error`,
4242
// so the catch region reads the ENGINE's run-wide variable rather than
43-
// `try_catch`'s own rebuilt binding.
43+
// `try_catch`'s own rebuilt binding. (Not `$caught`: a `$` name other
44+
// than `$error` is the engine's, and the contract refuses it — #22502.)
4445
const engine = new AutomationEngine(makeLogger());
4546
let seenError: any;
4647
let seenNodeScoped: any;
@@ -74,7 +75,7 @@ describe('#14955 — the throw arm refreshes `$error` like the returned-failure
7475
{
7576
id: 'tc', type: 'try_catch', label: 'Guarded',
7677
config: {
77-
errorVariable: '$caught',
78+
errorVariable: 'caught',
7879
try: { nodes: [{ id: 'boom', type: 'store_down', label: 'Boom' }], edges: [] },
7980
catch: { nodes: [{ id: 'look', type: 'probe', label: 'Look' }], edges: [] },
8081
},

0 commit comments

Comments
 (0)