From 7840f60770785cd91a3a7288605ee3482bcdcd89 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 22:21:15 +0000 Subject: [PATCH 1/5] fix(spec/automation)!: refuse a `$` name at every remaining flow binding A loop or map iteratorVariable / indexVariable, an object-form screen's idVariable, a screen field's name, a declared flow variable's name and an assignment node's targets now refuse a name that starts with `$`, by the one rule (`flowBoundVariableNameSchema`) outputVariable and errorVariable already compose. ADR-0087 D3 entry `flow-binding-name-dollar-refused`. Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude --- .../22572-flow-binding-name-dollar-refused.md | 46 +++++ .../src/automation/builtin-node-config.zod.ts | 80 ++++++--- .../spec/src/automation/control-flow.zod.ts | 15 +- .../automation/flow-bound-variable-name.ts | 157 +++++++++++++++--- packages/spec/src/automation/flow.zod.ts | 31 +++- .../18.flow-binding-name-dollar-refused.ts | 61 +++++++ packages/spec/src/migrations/registry.ts | 71 ++++++++ 7 files changed, 412 insertions(+), 49 deletions(-) create mode 100644 .changeset/22572-flow-binding-name-dollar-refused.md create mode 100644 packages/spec/src/migrations/entries/semantic/18.flow-binding-name-dollar-refused.ts diff --git a/.changeset/22572-flow-binding-name-dollar-refused.md b/.changeset/22572-flow-binding-name-dollar-refused.md new file mode 100644 index 00000000000..bb18861043d --- /dev/null +++ b/.changeset/22572-flow-binding-name-dollar-refused.md @@ -0,0 +1,46 @@ +--- +'@objectstack/spec': major +--- + +Every remaining place a flow binds a variable by name refuses a name that starts with `$`: a `loop` or `map` node's `iteratorVariable` and `indexVariable`, an object-form `screen` node's `idVariable`, a `screen` field's `name`, a declared flow variable's `name`, and an `assignment` node's targets. The refusal names the remedy: the same name without the `$`, read as `{{ name }}`. + +Clause-②: no (narrowing) + + + +**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.*`). + +**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 refuses a `{{ }}` hole over a `$` name the engine does not bind, and `outputVariable` / `errorVariable` already refuse one. Every other binding took any string, and on the run a `$` binding was worse than unreadable (measured with the engine on this release's `main`): + +- `iteratorVariable: '$record'` on a `loop` or `map` left `$record` holding the last item for the rest of the run, and `indexVariable: '$runId'` left `$runId` holding an index; +- `assignments: { $record: … }` overwrote the trigger record; +- a declared variable named `$record` was overwritten by the engine at run start, so its `defaultValue` never reached the run; +- a `screen` whose `idVariable` or field `name` was a `$` name paused, and then could never be submitted: the resume carrying the value was refused with `INVALID_SIGNAL`. + +**What is refused.** + +- `iteratorVariable` and `indexVariable` on `LoopConfigSchema` and `MapConfigSchema`, `idVariable` on `ScreenConfigSchema`, `name` on `ScreenFieldConfigSchema` and on `FlowVariableSchema`: a name that starts with `$`. 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. +- An `assignment` node's targets, in each shape its executor binds: a key of the `assignments` map (`AssignmentConfigSchema` states it as `propertyNames.pattern`), a top-level key of the bare legacy config, and the `variable` (or `name`, `key`) of a legacy `assignments: [{ variable, value }]` item. +- `FlowSchema.parse`, `registerFlow` and `objectstack validate` refuse the flow where the name was written — `nodes.N.config.iteratorVariable`, `nodes.N.config.fields.M.name`, `nodes.N.config.assignments.NAME`, `variables.N.name` — inside a region body too. A stored flow carrying one is skipped at boot with a warn naming it, and the `loop`, `map` and `screen` executors' own contract parse refuses the node at run time. + +**Unchanged.** Any name that does not start with `$`, a `$` later in the name (`a$b`) included; the defaults (`iteratorVariable` is still `item`); an empty string where the key took one; a body-less legacy `loop`, whose `iteratorVariable` nothing reads; and every hole the text-slot judge already admits. Non-string values keep the type refusal they had. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| `iteratorVariable: '$row'` with `title: '{{ $row.name }}'` | `iteratorVariable: 'row'` with `title: '{{ row.name }}'` | +| `idVariable: '$account'` | `idVariable: 'account_id'`, read as `{{ account_id }}` | +| `variables: [{ name: '$total', type: 'number' }]` | `variables: [{ name: 'total', type: 'number' }]`, read as `{{ total }}` | +| `assignments: { $total: … }` | `assignments: { total: … }` | + +**The one-line fix: name the variable without the `$`, and rename every read of it with it.** + +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. + +**Who is affected, measured.** The last published spec, `@objectstack/spec@17.7.0` (npm `latest`), types every one of these keys as a plain string. Measured on `main` at this change's base: the 35 flows `examples/app-crm`, `examples/app-todo` and `examples/app-showcase` ship all parse clean under the new rule, and a TypeScript-AST scan of `examples` (234 files), `packages/platform-objects` (167, which ships no flow), `packages/qa/dogfood` (280), the rest of `packages` and the `hotcrm` app found no `$`-led binding on any of these positions. The pinned `objectui` checkout binds none either. Deployed metadata and other repositories were not measured. + +### The kit + +- **The rule.** `flowBoundVariableNameSchema` in `automation/flow-bound-variable-name.ts`, package-internal (no new public export), now composed into every binding position. A `$` binding is refused with one sentence whichever door judges it. +- **The ledger.** The D3 semantic entry `flow-binding-name-dollar-refused` (protocol 18), with no D2 conversion. diff --git a/packages/spec/src/automation/builtin-node-config.zod.ts b/packages/spec/src/automation/builtin-node-config.zod.ts index 1593f97ad7f..cbe18e1969b 100644 --- a/packages/spec/src/automation/builtin-node-config.zod.ts +++ b/packages/spec/src/automation/builtin-node-config.zod.ts @@ -102,9 +102,12 @@ import { valueSlotTemplateRefusals } from './flow-value-slot-template'; // the single-brace tokens they still carry, composed into the screen and end // contracts below rather than re-spelled here. import { textSlotTemplateRefusal } from './flow-text-slot-template'; -// [#22502] The `$` names are the flow engine's at the binding keys too — the -// one rule, composed into every `outputVariable` below rather than re-spelled. -import { flowBoundVariableNameSchema } from './flow-bound-variable-name'; +// [#22502, #22572] The `$` names are the flow engine's at the binding keys too +// — the one rule, composed into every key below that binds a variable by name +// (`outputVariable`, a screen's `idVariable` and field `name`, `map`'s +// `iteratorVariable` / `indexVariable`, an `assignment` target) rather than +// re-spelled. +import { flowAssignmentTargets, flowBoundVariableNameRefusal, flowBoundVariableNameSchema } from './flow-bound-variable-name'; // The one key a screen field's option is addressed by as text — the option // collision refusal below reads it, as `translateFlow` and the extractor do. import { flowScreenFieldOptionKey } from './flow-screen-option-key'; @@ -696,8 +699,13 @@ export const ScreenFieldConfigSchema = lazySchema(() => strictObject({ object: 'reference', }, }, { - /** Field name — an item with an empty name is dropped. */ - name: z.string().describe('Field name (the flow variable the value binds to)'), + /** + * Field name — an item with an empty name is dropped. The submitted value + * binds to a flow variable of this name, so it is never a `$` name: those + * are the engine's, and the resume carrying one is refused (#22572). + */ + name: flowBoundVariableNameSchema('screenFieldName') + .describe('Field name (the flow variable the value binds to) — a name without a leading `$` (the `$` names are the flow engine\'s own), read as `{{ name }}`'), /** Display label. */ label: z.string().optional().describe('Display label'), /** Input type (text, number, select, …). */ @@ -858,9 +866,9 @@ export const ScreenConfigSchema = lazySchema(() => strictObject({ /** Object-form screen: render this object's full create/edit form instead. */ objectName: z.string().optional() .describe("Render this object's full create/edit form instead of a flat field list"), - /** Object form only: variable bound to the saved record's id. */ - idVariable: z.string().optional() - .describe("Object form only: variable bound to the saved record's id"), + /** Object form only: variable bound to the saved record's id — never a `$` name: those are the engine's (#22572). */ + idVariable: flowBoundVariableNameSchema('idVariable').optional() + .describe("Object form only: variable bound to the saved record's id — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}`"), /** * Object form only: create or edit. * @@ -1055,10 +1063,12 @@ export const MapConfigSchema = lazySchema(() => strictObject({ .describe('Template/variable resolving to the array to process (an inline array is accepted)'), /** The per-item subflow (execute-time required). */ flowName: z.string().describe('Subflow run for each item — it may pause (e.g. an approval)'), - /** Variable the current item is bound to inside each item's scope. */ - iteratorVariable: z.string().default('item').describe('Variable holding the current item'), - /** Optional variable the zero-based index is bound to. */ - indexVariable: z.string().optional().describe('Optional variable holding the current index'), + /** Variable the current item is bound to inside each item's scope — never a `$` name: those are the engine's (#22572). */ + iteratorVariable: flowBoundVariableNameSchema('iteratorVariable').default('item') + .describe('Variable holding the current item — a name without a leading `$` (the `$` names are the flow engine\'s own), read as `{{ name }}`'), + /** Optional variable the zero-based index is bound to — never a `$` name (#22572). */ + indexVariable: flowBoundVariableNameSchema('indexVariable').optional() + .describe('Optional variable holding the current index — a name without a leading `$` (the `$` names are the flow engine\'s own)'), /** When items are records, the object they belong to (exposes each item as the child's record). */ itemObject: z.string().optional() .describe("When items are records, the object they belong to (exposes each item as the child's record)"), @@ -1159,22 +1169,42 @@ export const ASSIGNMENT_ARRAY_FORM_PRESCRIPTION = * refused here instead; `assignments` one level down keeps its own guard, * because the two are different parsers at different depths and neither * covers the other. + * + * Its targets are bindings, so none is a `$` name (#22572): those are the flow + * engine's, and `assignments: { $record: … }` would overwrite the trigger + * record for the rest of the run. The canonical map's KEY schema is the rule + * (`flowBoundVariableNameSchema`), so the published JSON Schema states it as + * `propertyNames.pattern`. A bare config's top-level keys are refused by the + * same rule in the `superRefine` below — a `.catchall()` sees values, never + * keys, so no key schema reaches them, and that one site is a declared + * dropped refinement. A FLOW is judged at its own door, which reads every shape + * the executor binds, the legacy array included (`FlowSchema`'s `assignment` + * arm, {@link flowAssignmentTargets}): this contract is applied by no door. */ export const AssignmentConfigSchema = lazySchema(() => refuseCatchallProtoKey(z.object({ /** Variable name → value; the canonical authoring surface. */ assignments: refuseRecordProtoKey( - z.record(z.string().min(1), AssignmentValueSchema, { + z.record(flowBoundVariableNameSchema('assignmentTarget').min(1), AssignmentValueSchema, { // The array form is a TYPE error on this slot; the message is the // prescription, carried on the record's own `invalid_type` issue because // an object-level refinement never runs once a property has failed its // type (Zod aborts the object) — measured, not assumed. - error: (issue) => (Array.isArray(issue.input) ? ASSIGNMENT_ARRAY_FORM_PRESCRIPTION : undefined), + // + // [#22572] A refused KEY arrives as the record's own `invalid_key` + // issue, whose default message ("Invalid key in record") drops the key + // schema's sentence into a nested list; lift that sentence — the `$` + // rule's remedy — onto the issue the author reads. + error: (issue) => { + if (issue.code === 'invalid_key') return (issue as { issues?: ReadonlyArray<{ message?: string }> }).issues?.[0]?.message; + return Array.isArray(issue.input) ? ASSIGNMENT_ARRAY_FORM_PRESCRIPTION : undefined; + }, }), // [objectstack#18847] `__proto__` ONLY. This slot's key type carries no - // grammar (`z.string().min(1)`), so `constructor` and `prototype` are - // legal flow-variable names today and are left legal — only `__proto__` - // is structurally unreachable by any key schema (see - // `refuseRecordProtoKey`'s docblock), so it alone is refused here. + // grammar beyond the `$` rule (`flowBoundVariableNameSchema(…).min(1)`), + // so `constructor` and `prototype` are legal flow-variable names today and + // are left legal — only `__proto__` is structurally unreachable by any key + // schema (see `refuseRecordProtoKey`'s docblock), so it alone is refused + // here. 'assignments', ).optional() .describe('Variables to set: each key is a variable name, each value a CEL value envelope or a literal'), @@ -1184,7 +1214,19 @@ export const AssignmentConfigSchema = lazySchema(() => refuseCatchallProtoKey(z. // literal (an envelope-shaped object included — nothing evaluates it here), // and one still spelling the retired `{…}` template dialect is refused like // in the map, so the bare shape is no way around the retirement. - .catchall(LEGACY_ASSIGNMENT_VALUE), + .catchall(LEGACY_ASSIGNMENT_VALUE) + // [#22572] …and each such KEY is a binding, judged by the `$` rule the map's + // key schema is: the bare shape is no way around it either. The executor + // binds top-level keys only when there is no `assignments` map or array, so + // those are the keys judged here; the map's keys and the array form are the + // `assignments` slot's own. + .superRefine((config, ctx) => { + for (const target of flowAssignmentTargets(config)) { + if (target.path[0] === 'assignments' && target.path.length > 1) continue; + const message = flowBoundVariableNameRefusal('assignmentTarget', target.name); + if (message !== undefined) ctx.addIssue({ code: 'custom', path: [...target.path], message }); + } + }), // [#19151] `__proto__` ONLY, and for the same structural reason // the `assignments` slot above refuses it: `handleCatchall`'s // `if (key === "__proto__") continue;` runs above `_catchall.run`, so no diff --git a/packages/spec/src/automation/control-flow.zod.ts b/packages/spec/src/automation/control-flow.zod.ts index b4bff147e54..e115566ea96 100644 --- a/packages/spec/src/automation/control-flow.zod.ts +++ b/packages/spec/src/automation/control-flow.zod.ts @@ -86,8 +86,9 @@ import { strictObject } from '../shared/strict-object'; import { FlowNodeSchema, FlowEdgeSchema } from './flow.zod'; import type { FlowNodeParsed, FlowEdgeParsed } from './flow.zod'; import { FLOW_REGION_SLOTS_BY_TYPE } from './region-slots'; -// [#22502] The `$` names are the flow engine's at the binding keys too — the -// one rule, composed into `try_catch`'s `errorVariable` below. +// [#22502, #22572] The `$` names are the flow engine's at the binding keys +// too — the one rule, composed into `loop`'s `iteratorVariable` / +// `indexVariable` and `try_catch`'s `errorVariable` below. import { ENGINE_ERROR_VARIABLE, flowBoundVariableNameSchema } from './flow-bound-variable-name'; /** @@ -216,10 +217,12 @@ export const LoopConfigSchema = lazySchema(() => strictObject( description: 'Template/variable resolving to the array to iterate (an inline array is accepted)', xExpression: 'template', }), - /** Variable name the current item is bound to inside the body. */ - iteratorVariable: z.string().min(1).default('item').describe('Loop variable holding the current item'), - /** Optional variable name the zero-based index is bound to inside the body. */ - indexVariable: z.string().optional().describe('Optional loop variable holding the current index'), + /** Variable name the current item is bound to inside the body — never a `$` name: those are the engine's (#22572). */ + iteratorVariable: flowBoundVariableNameSchema('iteratorVariable').min(1).default('item') + .describe('Loop variable holding the current item — a name without a leading `$` (the `$` names are the flow engine\'s own), read as `{{ name }}`'), + /** Optional variable name the zero-based index is bound to inside the body — never a `$` name (#22572). */ + indexVariable: flowBoundVariableNameSchema('indexVariable').optional() + .describe('Optional loop variable holding the current index — a name without a leading `$` (the `$` names are the flow engine\'s own)'), /** * Maximum iterations to run — a guard against runaway collections. Clamped to * {@link LOOP_MAX_ITERATIONS_CEILING}; a collection longer than this fails the diff --git a/packages/spec/src/automation/flow-bound-variable-name.ts b/packages/spec/src/automation/flow-bound-variable-name.ts index 5211426e99f..62d5bea68e9 100644 --- a/packages/spec/src/automation/flow-bound-variable-name.ts +++ b/packages/spec/src/automation/flow-bound-variable-name.ts @@ -3,24 +3,42 @@ /** * @module automation/flow-bound-variable-name * - * **The `$` names are the flow engine's at the BINDING doors too** (#22502). + * **The `$` names are the flow engine's at EVERY binding door** (#22502, + * #22572). * - * A flow names a variable it binds in two config keys this module governs: a - * node's `outputVariable` (`get_record`, `create_record`, `map`, `script`, - * `subflow`) and a `try_catch` node's `errorVariable`. Both took any string, - * so a flow could bind `$caught` — and then not read it: a text slot refuses a - * `{{ }}` hole whose root is a `$` name the engine does not bind - * (`flow-text-slot-template.ts`, #22477), and its remedy is to drop the `$`. - * One contract's two doors disagreed about whether an author may own a `$` - * name. + * A flow names the variables it binds in eight places, and this module governs + * all of them ({@link FLOW_BINDING_KEYS}): + * + * - a node's `outputVariable` (`get_record`, `create_record`, `map`, + * `script`, `subflow`) and a `try_catch` node's `errorVariable` (#22502); + * - a `loop` or `map` node's `iteratorVariable` and `indexVariable`, which + * bind the current item and its index in the enclosing scope; + * - an object-form `screen` node's `idVariable`, and a `screen` field's + * `name` — the resume that submits the screen writes the saved record's id, + * and each field's value, under those names; + * - a declared flow variable's `name` (`FlowSchema.variables[].name`); + * - an `assignment` node's targets, in the three shapes its executor reads + * ({@link flowAssignmentTargets}). + * + * Each took any string, so a flow could bind `$row` — and then not read it: a + * text slot refuses a `{{ }}` hole whose root is a `$` name the engine does not + * bind (`flow-text-slot-template.ts`, #22477), and its remedy is to drop the + * `$`. One contract's two doors disagreed about whether an author may own a `$` + * name. Measured on the run, it was worse than unreadable: a `$` binding over + * one of the engine's own names replaces it for the rest of the run + * (`iteratorVariable: '$record'` leaves the trigger record holding the last + * item, `indexVariable: '$runId'` the run id holding an index), a declared + * `$record` is overwritten by the engine at run start, and a `screen` whose + * `idVariable` or field is a `$` name can never be submitted — the resume that + * carries it is refused (`INVALID_SIGNAL`). * * They agree now, on the rule that judge already reads: the `$` names are * reserved for the engine (a resume signal may not write one either — - * `IAutomationService.resume`'s `INVALID_SIGNAL`). So a binding key refuses a - * name that starts with `$`, and its remedy is the same name without the `$`, - * read as `{{ name }}`. The one `$` name an author may write is the one the - * engine itself binds there: `errorVariable: '$error'`, the key's default — - * the caught error the engine publishes as `$error` either way. + * `IAutomationService.resume`'s `INVALID_SIGNAL`). So a binding refuses a name + * that starts with `$`, and its remedy is the same name without the `$`, read + * as `{{ name }}`. The one `$` name an author may write is the one the engine + * itself binds there: `errorVariable: '$error'`, the key's default — the + * caught error the engine publishes as `$error` either way. * * ## The prefix rule, not a second list * @@ -36,11 +54,20 @@ * The rule is a `regex` check, not a `.refine()`: `z.toJSONSchema()` emits a * regex as `pattern`, so the published schema refuses exactly what the parse * refuses, where a refinement would be dropped from it - * (`shared/refinement-projection.ts`). Each executor parses its config against - * the same contract (`parseNodeConfig`), so `FlowSchema.parse`, + * (`shared/refinement-projection.ts`). A key composes it as its string schema + * (`pattern`); the canonical `assignments` map composes it as its KEY schema + * (`propertyNames.pattern`). Each executor parses its config against the same + * contract (`parseNodeConfig`), so `FlowSchema.parse`, * `AutomationEngine.registerFlow`, `objectstack validate` and the run itself * all refuse such a name through `flowNodeConfigRefusals`, anchored at the key. * + * Two positions are not a key of a contract a door applies, and are judged by + * {@link flowBoundVariableNameRefusal} instead — the same rule, the same + * sentence: an `assignment` node's targets (no executor contract parses its + * config, so `FlowSchema` walks {@link flowAssignmentTargets}), and the bare + * legacy `assignment` config's top-level keys inside `AssignmentConfigSchema` + * itself, where no key schema reaches (a `.catchall()` sees values only). + * * ## ⛔ Package-internal — NOT a public export * * Deliberately absent from `automation/index.ts`: its callers are the node @@ -53,8 +80,33 @@ import { z } from 'zod'; /** The name the engine binds a `try_catch`'s caught error to when `errorVariable` is absent. */ export const ENGINE_ERROR_VARIABLE = '$error'; -/** The config keys that bind a flow variable by name, and that this module governs. */ -export type FlowBindingKey = 'outputVariable' | 'errorVariable'; +/** + * Each binding position this module governs, and the clause its refusal says + * about it. `errorVariable` alone admits a `$` name (`$error`), so its clause + * is written in {@link boundVariableNameRefusal}. + */ +const BINDING_CLAUSES = { + outputVariable: 'an `outputVariable` may not bind one', + errorVariable: '', + iteratorVariable: 'a `loop` or `map` `iteratorVariable` may not bind one', + indexVariable: 'a `loop` or `map` `indexVariable` may not bind one', + idVariable: + 'an object-form `screen` `idVariable` may not bind one (the resume that submits the screen could not write it)', + variableName: 'a flow may not declare a variable under one', + screenFieldName: + 'a `screen` field, whose value binds to a variable of its `name`, may not be named with one (the resume that ' + + 'submits the screen could not write it)', + assignmentTarget: 'an `assignment` node may not set one', +} as const; + +/** The binding positions this module governs — a key, or a position that names a variable. */ +export type FlowBindingKey = keyof typeof BINDING_CLAUSES; + +/** + * Every binding position, in a fixed order — the vocabulary the enumeration + * pin (`flow-bound-variable-name.test.ts`) holds equal to the sites it judges. + */ +export const FLOW_BINDING_KEYS: readonly FlowBindingKey[] = Object.keys(BINDING_CLAUSES) as FlowBindingKey[]; /** * Any string whose first character is not `$` — the empty string included, @@ -63,7 +115,7 @@ export type FlowBindingKey = 'outputVariable' | 'errorVariable'; */ const NOT_DOLLAR_LED = '[^$][\\s\\S]*'; -/** Why a `$`-led name is refused at a binding key, with its remedy. */ +/** Why a `$`-led name is refused at a binding position, with its remedy. */ function boundVariableNameRefusal(key: FlowBindingKey, name: string): string { const bare = name.replace(/^\$+/, ''); const read = key === 'errorVariable' ? `${bare}.message` : bare; @@ -73,7 +125,7 @@ function boundVariableNameRefusal(key: FlowBindingKey, name: string): string { const rule = key === 'errorVariable' ? `a \`try_catch\` \`errorVariable\` may name only the engine's own \`${ENGINE_ERROR_VARIABLE}\`, its default, ` + 'and a flow text slot refuses to read any other `$` name. ' - : 'an `outputVariable` may not bind one, and a flow text slot refuses to read it. '; + : `${BINDING_CLAUSES[key]}, and a flow text slot refuses to read it. `; const remedy = bare === '' ? 'Name the variable without the `$` and read it as a `{{ }}` hole over that name' : `Name it \`${bare}\` and read it as \`{{ ${read} }}\``; @@ -84,9 +136,9 @@ function boundVariableNameRefusal(key: FlowBindingKey, name: string): string { } /** - * The string schema of a binding key: a name that does not start with `$`, - * and for `errorVariable` the engine's own `$error` as well. The caller adds - * `.optional()` or `.default(…)` and its `.describe()`. + * The string schema of a binding position: a name that does not start with + * `$`, and for `errorVariable` the engine's own `$error` as well. The caller + * adds `.min(1)`, `.optional()` or `.default(…)` and its `.describe()`. */ export function flowBoundVariableNameSchema(key: FlowBindingKey) { const engineOwn = key === 'errorVariable' ? `${ENGINE_ERROR_VARIABLE.replace('$', '\\$')}|` : ''; @@ -94,3 +146,62 @@ export function flowBoundVariableNameSchema(key: FlowBindingKey) { error: (issue) => boundVariableNameRefusal(key, String(issue.input)), }); } + +const RULE_BY_KEY = new Map>(); + +/** + * The rule's refusal of `name` at `key` — the very sentence the key's schema + * gives — or `undefined` when the rule admits it. For the positions no key + * schema reaches (see the module docblock). + */ +export function flowBoundVariableNameRefusal(key: FlowBindingKey, name: string): string | undefined { + let rule = RULE_BY_KEY.get(key); + if (rule === undefined) { + rule = flowBoundVariableNameSchema(key); + RULE_BY_KEY.set(key, rule); + } + const result = rule.safeParse(name); + return result.success ? undefined : result.error.issues[0]?.message; +} + +/** One variable an `assignment` node binds, at the config path the author wrote it. */ +export interface FlowAssignmentTarget { + /** The path under the node's `config`: `['assignments', 'total']`, `['assignments', 0, 'variable']`, `['total']`. */ + readonly path: readonly (string | number)[]; + /** The variable name the executor binds. */ + readonly name: string; +} + +/** + * Every variable an `assignment` node's `config` binds, in the three shapes its + * executor reads (`service-automation` `builtin/logic-nodes.ts`), and only + * those — a key the executor never binds is no binding: + * + * - `assignments` an array — the legacy `[{ variable, value }]` form: each + * item's first non-null `variable`, `name` or `key`, when it is a non-empty + * string; + * - `assignments` any other object — the canonical map: each own key; + * - otherwise (no `assignments`, or a value that is neither) — the bare legacy + * config: each top-level key. + */ +export function flowAssignmentTargets(config: unknown): FlowAssignmentTarget[] { + if (config === null || typeof config !== 'object' || Array.isArray(config)) return []; + const raw = (config as Record).assignments; + const out: FlowAssignmentTarget[] = []; + if (Array.isArray(raw)) { + raw.forEach((item: unknown, index) => { + if (item === null || typeof item !== 'object') return; + const entry = item as Record; + const key = (['variable', 'name', 'key'] as const).find((k) => entry[k] !== undefined && entry[k] !== null); + const name = key === undefined ? undefined : entry[key]; + if (key !== undefined && typeof name === 'string' && name !== '') out.push({ path: ['assignments', index, key], name }); + }); + return out; + } + if (raw !== null && typeof raw === 'object') { + for (const name of Object.keys(raw)) out.push({ path: ['assignments', name], name }); + return out; + } + for (const name of Object.keys(config)) out.push({ path: [name], name }); + return out; +} diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index c4c6c4453db..5063f322562 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -27,6 +27,10 @@ import { collectFlowGraphs, parseFlowNodeRegions } from './control-flow.zod'; import { predicateSlotRefusal, resolveFlowNodeExpressions } from './flow-node-expression-paths'; import { flowNodeConfigRefusals } from './flow-node-config-refusals'; import { EndConfigSchema } from './builtin-node-config.zod'; +// [#22572] The `$` names are the flow engine's at every binding door — the one +// rule, composed into a declared variable's `name` and judging an `assignment` +// node's targets below rather than re-spelled. +import { flowAssignmentTargets, flowBoundVariableNameRefusal, flowBoundVariableNameSchema } from './flow-bound-variable-name'; import { APPROVAL_NODE_TYPE, APPROVAL_REVISE_NODE_TYPE } from './approval.zod'; export const FlowNodeAction = z.enum([ 'start', // Trigger @@ -224,7 +228,12 @@ export const FlowVariableSchema = lazySchema(() => strictObject( 'mis-declared input/output contract shipped without a diagnostic.', }, { - name: z.string().describe('Variable name').meta({ title: 'Name' }), + // [#22572] A declared variable is a binding: never a `$` name, which is the + // engine's — a declared `$record` is overwritten when the engine seeds its own + // variables at run start, and a text slot refuses to read any other. + name: flowBoundVariableNameSchema('variableName') + .describe('Variable name — a name without a leading `$` (the `$` names are the flow engine\'s own), read as `{{ name }}`') + .meta({ title: 'Name' }), type: z.string().describe('Data type (text, number, boolean, object, list)').meta({ title: 'Type' }), isInput: z.boolean().default(false).describe('Is input parameter').meta({ title: 'Input' }), isOutput: z.boolean().default(false).describe('Is output parameter').meta({ title: 'Output' }), @@ -1519,6 +1528,26 @@ export const FlowSchema = lazySchema(() => strictObject( }); } + // An `assignment` node's targets are bindings (#22572), and the `$` names are + // the flow engine's at every binding door — `assignments: { $record: … }` + // would overwrite the trigger record for the rest of the run. No executor + // contract parses an `assignment` config (its executor reads three + // read-compatible shapes), so `flowNodeConfigRefusals` cannot reach them: + // this arm reads the names the executor binds, in every shape + // (`flowAssignmentTargets`), and refuses each `$` name with the one rule's + // own sentence, anchored where the author wrote it — a region body's node + // included, through `collectFlowGraphs`. + for (const graph of collectFlowGraphs(flow)) { + graph.nodes.forEach((node, index) => { + if ((node as { type?: unknown } | null)?.type !== 'assignment') return; + for (const target of flowAssignmentTargets((node as { config?: unknown }).config)) { + const message = flowBoundVariableNameRefusal('assignmentTarget', target.name); + if (message === undefined) continue; + ctx.addIssue({ code: 'custom', path: [...graph.path, 'nodes', index, 'config', ...target.path], message }); + } + }); + } + // 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 diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-binding-name-dollar-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-binding-name-dollar-refused.ts new file mode 100644 index 00000000000..f9f4959ff36 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-binding-name-dollar-refused.ts @@ -0,0 +1,61 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// #22572 — the family close-out of flow-binding-variable-dollar-name-refused: +// every remaining position where a flow binds a variable by name refuses a +// dollar name, by the same rule — a loop or map iteratorVariable and +// indexVariable, an object-form screen's idVariable, a screen field's name, a +// declared flow variable's name, and an assignment node's targets in each shape +// its executor reads. It narrows a flow's accept set; no key is removed, so +// there is no tombstone and no RETIRED_KEYS_BY_MAJOR row. Semantic-only — 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. +// +// No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it +// inside a code span and a table cell. +export const entry: SemanticMigration = { + id: 'flow-binding-name-dollar-refused', + surface: + 'flows[].nodes[].config.iteratorVariable and config.indexVariable of a loop or map node, ' + + 'flows[].nodes[].config.idVariable and config.fields[].name of a screen node, flows[].variables[].name, ' + + 'and an assignment node\'s targets — a key of config.assignments, a top-level key of a config with no ' + + 'assignments map, or the variable of a legacy assignments array item — when the name starts with a dollar ' + + 'sign, such as $row. Reachable wherever a flow is authored or stored: defineStack flows sources, ' + + 'defineFlow, an exported stack passed to objectstack validate or objectstack compile, a flow saved from the ' + + 'Studio flow designer, and a flow row already in sys_metadata', + replacement: + 'the same name without the dollar sign, read as a hole over that name: iteratorVariable: \'row\' read as ' + + '{{ row.name }}, idVariable: \'account_id\' read as {{ account_id }}, a declared variable or an assignment ' + + 'target named total read as {{ total }}. Rename every read of the old name with it — a text-slot hole, a ' + + 'CEL expression, a single-brace token in a value position', + reason: + 'The dollar-named variables are the flow engine\'s own: it binds $record, $runId, $flowName, $flowLabel and ' + + '$error, a flat-graph loop binds $loopItems and $loopIndex, and a resume signal may not write any dollar ' + + 'name. A flow text slot refuses a hole whose root is a dollar name the engine does not bind and tells the ' + + 'author to drop the dollar sign, and since protocol 18 an outputVariable and an errorVariable refuse one ' + + '(flow-binding-variable-dollar-name-refused). Every other binding still took any string, so one contract ' + + 'still had two answers. Measured on the run, the old answer was worse than unreadable: a loop or map ' + + 'iteratorVariable of $record left the trigger record holding the last item for the rest of the run, an ' + + 'indexVariable of $runId left the run id holding an index, an assignment to $record overwrote it, a ' + + 'declared variable named $record was overwritten by the engine at run start, and a screen whose ' + + 'idVariable or field name was a dollar name could never be submitted — the resume that carried the value ' + + 'was refused as a write to an engine variable. Each binding now gives the text slots\' answer: each key ' + + 'states the rule as a JSON Schema pattern (propertyNames for the assignments map), so the published schema ' + + 'refuses what the parse refuses, and the flow parse, registerFlow and objectstack validate refuse such a ' + + 'name where it was written, naming the same name without the dollar sign — as does the run itself for a ' + + 'loop, map or screen node, whose executor parses its contract. ' + + 'No D2 conversion exists: 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, so the rename is the author\'s. Where such a binding 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 stack source throws ' + + 'StackSchemaInvalidError for the whole stack. 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 position — nodes.N.config.iteratorVariable, ' + + 'nodes.N.config.fields.M.name, nodes.N.config.assignments.NAME, variables.N.name, or the same under a ' + + 'region path such as nodes.N.config.body.nodes.M.config — and the name to write. Rename the binding and ' + + 'every read of it, then (1) objectstack validate is clean, (2) each flow registers at boot with no failed ' + + 'to register flow warn for it, and (3) the flow paths that read the variable — a notification, a screen, ' + + 'a later node — carry its value, with no blank fragment, and a screen that binds one submits.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 87c176a0d81..358d465d56b 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5666,6 +5666,20 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ + 'the platform cannot know what the author meant. Its D3 record is the semantic entry ' + '`flow-approval-node-config-contract-refused`.', }, + { + id: 'flow-binding-name-dollar-refused', + order: 93, + text: + 'And every other binding does too, closing the family: a `loop` or `map` `iteratorVariable` and ' + + '`indexVariable`, an object-form `screen`\'s `idVariable`, a `screen` field\'s `name`, a declared flow ' + + 'variable\'s `name`, and an `assignment` node\'s targets in each shape its executor reads refuse a name ' + + 'that starts with `$`, by the same rule and with the same remedy. A binding over an engine name replaced ' + + 'it for the rest of the run (`iteratorVariable: \'$record\'`), a declared `$record` was overwritten at ' + + 'run start, and a `screen` binding a `$` name could never be submitted, its resume refused. Each key ' + + 'states the rule as a `pattern` (the `assignments` map as `propertyNames`), and the flow parse, ' + + '`registerFlow` and `objectstack validate` refuse such a name where it was written. No D2 conversion ' + + 'exists, for the same reason. Its D3 record is the semantic entry `flow-binding-name-dollar-refused`.', + }, { id: 'flow-binding-variable-dollar-name-refused', order: 92, @@ -13560,6 +13574,63 @@ const step18: MigrationStep = { + 'row that exists only in `sys_metadata`. An approval node the contract accepts parses and ' + 'registers byte-identically to before.', }, + // #22572 — the family close-out of flow-binding-variable-dollar-name-refused: + // every remaining position where a flow binds a variable by name refuses a + // dollar name, by the same rule — a loop or map iteratorVariable and + // indexVariable, an object-form screen's idVariable, a screen field's name, a + // declared flow variable's name, and an assignment node's targets in each shape + // its executor reads. It narrows a flow's accept set; no key is removed, so + // there is no tombstone and no RETIRED_KEYS_BY_MAJOR row. Semantic-only — 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. + // + // No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it + // inside a code span and a table cell. + { + id: 'flow-binding-name-dollar-refused', + surface: + 'flows[].nodes[].config.iteratorVariable and config.indexVariable of a loop or map node, ' + + 'flows[].nodes[].config.idVariable and config.fields[].name of a screen node, flows[].variables[].name, ' + + 'and an assignment node\'s targets — a key of config.assignments, a top-level key of a config with no ' + + 'assignments map, or the variable of a legacy assignments array item — when the name starts with a dollar ' + + 'sign, such as $row. Reachable wherever a flow is authored or stored: defineStack flows sources, ' + + 'defineFlow, an exported stack passed to objectstack validate or objectstack compile, a flow saved from the ' + + 'Studio flow designer, and a flow row already in sys_metadata', + replacement: + 'the same name without the dollar sign, read as a hole over that name: iteratorVariable: \'row\' read as ' + + '{{ row.name }}, idVariable: \'account_id\' read as {{ account_id }}, a declared variable or an assignment ' + + 'target named total read as {{ total }}. Rename every read of the old name with it — a text-slot hole, a ' + + 'CEL expression, a single-brace token in a value position', + reason: + 'The dollar-named variables are the flow engine\'s own: it binds $record, $runId, $flowName, $flowLabel and ' + + '$error, a flat-graph loop binds $loopItems and $loopIndex, and a resume signal may not write any dollar ' + + 'name. A flow text slot refuses a hole whose root is a dollar name the engine does not bind and tells the ' + + 'author to drop the dollar sign, and since protocol 18 an outputVariable and an errorVariable refuse one ' + + '(flow-binding-variable-dollar-name-refused). Every other binding still took any string, so one contract ' + + 'still had two answers. Measured on the run, the old answer was worse than unreadable: a loop or map ' + + 'iteratorVariable of $record left the trigger record holding the last item for the rest of the run, an ' + + 'indexVariable of $runId left the run id holding an index, an assignment to $record overwrote it, a ' + + 'declared variable named $record was overwritten by the engine at run start, and a screen whose ' + + 'idVariable or field name was a dollar name could never be submitted — the resume that carried the value ' + + 'was refused as a write to an engine variable. Each binding now gives the text slots\' answer: each key ' + + 'states the rule as a JSON Schema pattern (propertyNames for the assignments map), so the published schema ' + + 'refuses what the parse refuses, and the flow parse, registerFlow and objectstack validate refuse such a ' + + 'name where it was written, naming the same name without the dollar sign — as does the run itself for a ' + + 'loop, map or screen node, whose executor parses its contract. ' + + 'No D2 conversion exists: 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, so the rename is the author\'s. Where such a binding 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 stack source throws ' + + 'StackSchemaInvalidError for the whole stack. 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 position — nodes.N.config.iteratorVariable, ' + + 'nodes.N.config.fields.M.name, nodes.N.config.assignments.NAME, variables.N.name, or the same under a ' + + 'region path such as nodes.N.config.body.nodes.M.config — and the name to write. Rename the binding and ' + + 'every read of it, then (1) objectstack validate is clean, (2) each flow registers at boot with no failed ' + + 'to register flow warn for it, and (3) the flow paths that read the variable — a notification, a screen, ' + + 'a later node — carry its value, with no blank fragment, and a screen that binds one submits.', + }, // #22502 — the binding half of the rule the text-slot judge reads // (`flow-text-slot-unbound-dollar-root-refused`): the dollar names are the flow // engine's, so a node's outputVariable and a try_catch errorVariable refuse one From e8db6a982800b85fe5c03ea981e053af26eae06c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 22:23:58 +0000 Subject: [PATCH 2/5] test(spec/automation): pin the `$` rule at every binding door, and enumerate them Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude --- .../flow-bound-variable-name.test.ts | 306 +++++++++++++++++- 1 file changed, 303 insertions(+), 3 deletions(-) diff --git a/packages/spec/src/automation/flow-bound-variable-name.test.ts b/packages/spec/src/automation/flow-bound-variable-name.test.ts index 39aaa8022f7..5ae1b981c71 100644 --- a/packages/spec/src/automation/flow-bound-variable-name.test.ts +++ b/packages/spec/src/automation/flow-bound-variable-name.test.ts @@ -9,6 +9,11 @@ * composes it, at the `FlowSchema` door (a region body included), at the stack * door, and in the published JSON Schema; and that the remedy's spelling is one * the text-slot judge reads. + * + * #22572 closes the family: EVERY position a flow binds a variable by name — + * `loop` / `map` `iteratorVariable` and `indexVariable`, a `screen`'s + * `idVariable` and field `name`, a declared variable's `name`, an `assignment` + * node's targets — and the enumeration pin below holds it closed. */ import { describe, expect, it } from 'vitest'; import { z } from 'zod'; @@ -16,11 +21,20 @@ import { z } from 'zod'; import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas'; import { MIGRATIONS_BY_MAJOR } from '../migrations/registry'; import { defineStack } from '../stack.zod'; -import { CreateRecordConfigSchema, GetRecordConfigSchema, MapConfigSchema } from './builtin-node-config.zod'; -import { TryCatchConfigSchema } from './control-flow.zod'; +import { + AssignmentConfigSchema, + CreateRecordConfigSchema, + GetRecordConfigSchema, + MapConfigSchema, + ScreenConfigSchema, + ScreenFieldConfigSchema, +} from './builtin-node-config.zod'; +import { LoopConfigSchema, TryCatchConfigSchema } from './control-flow.zod'; +import { FLOW_BINDING_KEYS, flowAssignmentTargets, type FlowBindingKey } from './flow-bound-variable-name'; import { flowNodeConfigRefusals } from './flow-node-config-refusals'; import { textSlotTemplateRefusal } from './flow-text-slot-template'; -import { FlowSchema } from './flow.zod'; +import { FlowSchema, FlowVariableSchema } from './flow.zod'; +import * as automation from './index'; import { ScriptConfigSchema, SubflowConfigSchema } from './schemaless-node-config.zod'; /** Every contract with an `outputVariable`, with the smallest config it accepts. */ @@ -208,3 +222,289 @@ describe('ADR-0087 — the narrowing is registered as a D3 semantic entry of pro expect(MIGRATIONS_BY_MAJOR[18]!.rationale).toContain(ENTRY_ID); }); }); + +// ─── #22572 — every binding door, and the enumeration pin ──────────────────── + +/** A body with one inert node, so a `loop`'s values are judged (a body-less legacy `loop` reads none of them). */ +const LOOP_BODY = { nodes: [{ id: 'step', type: 'assignment', label: 'Step', config: { assignments: { touched: true } } }], edges: [] }; + +/** The automation export each `outputVariable` contract above is published as. */ +const OUTPUT_VARIABLE_EXPORTS: Readonly> = { + get_record: 'GetRecordConfigSchema', + create_record: 'CreateRecordConfigSchema', + map: 'MapConfigSchema', + script: 'ScriptConfigSchema', + subflow: 'SubflowConfigSchema', +}; + +type Parser = { safeParse(v: unknown): { success: boolean; error?: z.ZodError } }; + +/** One position a flow binds a variable by name. */ +interface BindingSite { + /** The position, as the rule's vocabulary names it. */ + readonly key: FlowBindingKey; + /** The row's name in the test titles. */ + readonly label: string; + /** The automation export that declares a `*Variable` key — what the discovery pin cross-checks. */ + readonly declaredBy?: string; + /** The contract that declares the position: a config binding `name` there, and where its issue lands. */ + readonly contract?: { readonly schema: Parser; readonly config: (name: string) => unknown; readonly path: (name: string) => string }; + /** A whole flow binding `name` there, and where `FlowSchema`'s issue lands. */ + readonly flow: (name: string) => unknown; + readonly flowPath: (name: string) => string; + /** How the remedy reads the bare name: `{{ }}`. */ + readonly read?: (bare: string) => string; +} + +/** `flowWith` plus declared variables. */ +function flowDeclaring(variables: unknown[]) { + return { ...flowWith('assignment', { assignments: { touched: true } }), variables }; +} + +/** + * Every binding position, one row per site — the table the enumeration pin + * holds equal to the rule's vocabulary ({@link FLOW_BINDING_KEYS}) and to the + * `*Variable` keys it discovers in the automation schemas. + */ +const BINDING_SITES: readonly BindingSite[] = [ + ...OUTPUT_VARIABLE_CONTRACTS.map(({ type, schema, base }): BindingSite => ({ + key: 'outputVariable', + label: `${type} outputVariable`, + declaredBy: OUTPUT_VARIABLE_EXPORTS[type], + contract: { schema, config: (name) => ({ ...base, outputVariable: name }), path: () => 'outputVariable' }, + flow: (name) => flowWith(type, { ...base, outputVariable: name }), + flowPath: () => 'nodes.1.config.outputVariable', + })), + { + key: 'errorVariable', + label: 'try_catch errorVariable', + declaredBy: 'TryCatchConfigSchema', + contract: { schema: TryCatchConfigSchema, config: (name) => ({ try: TRY_REGION, errorVariable: name }), path: () => 'errorVariable' }, + flow: (name) => flowWith('try_catch', { try: TRY_REGION, errorVariable: name }), + flowPath: () => 'nodes.1.config.errorVariable', + read: (bare) => `${bare}.message`, + }, + ...(['iteratorVariable', 'indexVariable'] as const).flatMap((key): BindingSite[] => [ + { + key, + label: `loop ${key}`, + declaredBy: 'LoopConfigSchema', + contract: { schema: LoopConfigSchema, config: (name) => ({ collection: 'rows', body: LOOP_BODY, [key]: name }), path: () => key }, + flow: (name) => flowWith('loop', { collection: 'rows', body: LOOP_BODY, [key]: name }), + flowPath: () => `nodes.1.config.${key}`, + }, + { + key, + label: `map ${key}`, + declaredBy: 'MapConfigSchema', + contract: { schema: MapConfigSchema, config: (name) => ({ collection: 'rows', flowName: 'per_row', [key]: name }), path: () => key }, + flow: (name) => flowWith('map', { collection: 'rows', flowName: 'per_row', [key]: name }), + flowPath: () => `nodes.1.config.${key}`, + }, + ]), + { + key: 'idVariable', + label: 'screen idVariable', + declaredBy: 'ScreenConfigSchema', + contract: { schema: ScreenConfigSchema, config: (name) => ({ objectName: 'lead', idVariable: name }), path: () => 'idVariable' }, + flow: (name) => flowWith('screen', { objectName: 'lead', idVariable: name }), + flowPath: () => 'nodes.1.config.idVariable', + }, + { + key: 'screenFieldName', + label: 'screen fields[].name', + contract: { schema: ScreenFieldConfigSchema, config: (name) => ({ name, label: 'N', type: 'text' }), path: () => 'name' }, + flow: (name) => flowWith('screen', { fields: [{ name, label: 'N', type: 'text' }] }), + flowPath: () => 'nodes.1.config.fields.0.name', + }, + { + key: 'variableName', + label: 'flow variables[].name', + contract: { schema: FlowVariableSchema, config: (name) => ({ name, type: 'text' }), path: () => 'name' }, + flow: (name) => flowDeclaring([{ name, type: 'text' }]), + flowPath: () => 'variables.0.name', + }, + { + key: 'assignmentTarget', + label: 'assignment assignments map key', + contract: { schema: AssignmentConfigSchema, config: (name) => ({ assignments: { [name]: 1 } }), path: (name) => `assignments.${name}` }, + flow: (name) => flowWith('assignment', { assignments: { [name]: 1 } }), + flowPath: (name) => `nodes.1.config.assignments.${name}`, + }, + { + key: 'assignmentTarget', + label: 'assignment bare top-level key', + contract: { schema: AssignmentConfigSchema, config: (name) => ({ [name]: 1 }), path: (name) => name }, + flow: (name) => flowWith('assignment', { [name]: 1 }), + flowPath: (name) => `nodes.1.config.${name}`, + }, + { + // No contract row: `AssignmentConfigSchema` refuses the legacy array form + // whole, with the write-the-map prescription; the flow door, which reads + // every shape the executor binds, judges its items. + key: 'assignmentTarget', + label: 'assignment legacy array item variable', + flow: (name) => flowWith('assignment', { assignments: [{ variable: name, value: 1 }] }), + flowPath: () => 'nodes.1.config.assignments.0.variable', + }, +]; + +const CONTRACT_SITES = BINDING_SITES.filter((site) => site.contract !== undefined); + +describe('every binding door refuses a `$` name — #22572 closes the family', () => { + it.each(CONTRACT_SITES)('$label: its contract refuses a dollar-led name at the position, with the remedy', ({ contract, read }) => { + const message = refusalAt(contract!.schema, contract!.config('$x'), contract!.path('$x')); + expect(message).toBeDefined(); + expect(message).toContain('`$x` is a `$` name'); + expect(message).toContain(`Name it \`x\` and read it as \`{{ ${read ? read('x') : 'x'} }}\``); + }); + + it.each(BINDING_SITES)('$label: FlowSchema refuses a dollar-led name there, and nothing else', ({ flow, flowPath, read }) => { + const issues = issuesOf(flow('$x')); + expect(issues.map(({ path }) => path)).toEqual([flowPath('$x')]); + expect(issues[0]!.message).toContain(`Name it \`x\` and read it as \`{{ ${read ? read('x') : 'x'} }}\``); + }); + + it.each(BINDING_SITES)('$label: the engine\'s own names are refused as bindings too', ({ flow, flowPath }) => { + for (const name of ['$record', '$runId', '$loopItems', '$']) { + expect(issuesOf(flow(name)).map(({ path }) => path), name).toEqual([flowPath(name)]); + } + }); + + it.each(BINDING_SITES)('CONTROL: $label: a plain name and a `$` past the first character parse clean', ({ contract, flow }) => { + for (const name of ['x', 'a$b']) { + if (contract) expect(contract.schema.safeParse(contract.config(name)).success, name).toBe(true); + expect(issuesOf(flow(name)), name).toEqual([]); + } + }); + + it('CONTROL: the defaults are kept — `iteratorVariable` is still `item` on a loop and a map, `$error` on a try_catch', () => { + const loop = LoopConfigSchema.safeParse({ collection: 'rows', body: LOOP_BODY }); + const map = MapConfigSchema.safeParse({ collection: 'rows', flowName: 'per_row' }); + expect(loop.success && loop.data.iteratorVariable).toBe('item'); + expect(map.success && map.data.iteratorVariable).toBe('item'); + expect(loop.success && loop.data.indexVariable).toBeUndefined(); + expect(issuesOf(flowWith('try_catch', { try: TRY_REGION, errorVariable: '$error' }))).toEqual([]); + }); + + it('defineStack refuses a declared `$` variable with the stack envelope, at the variable', () => { + const stack = { + manifest: { id: 'com.example.binding', name: 'binding', version: '1.0.0', type: 'app', namespace: 'bnd' }, + objects: [{ name: 'bnd_task', label: 'Task', fields: { title: { type: 'text', label: 'Title' } } }], + flows: [flowDeclaring([{ name: '$total', type: 'number' }])], + }; + let refusal: { code?: unknown; status?: unknown; issues?: Array<{ path: unknown[] }> } | undefined; + try { + defineStack(stack as never); + } catch (e) { + refusal = e as typeof refusal; + } + expect(refusal, 'defineStack must refuse the flow').toBeDefined(); + expect({ code: refusal!.code, status: refusal!.status }).toEqual({ code: 'STACK_SCHEMA_INVALID', status: 422 }); + expect(refusal!.issues!.map((i) => i.path.join('.'))).toEqual(['flows.0.variables.0.name']); + }); + + it('the remedy reads: a loop binding `row` and a body text slot reading `{{ row.name }}` parse clean', () => { + expect(textSlotTemplateRefusal('Due: {{ $row.name }}')).toBeDefined(); + const body = { nodes: [{ id: 'tell', type: 'notify', label: 'Tell', config: { title: 'Due', message: 'Due: {{ row.name }}', recipients: ['ops'] } }], edges: [] }; + expect(issuesOf(flowWith('loop', { collection: 'rows', iteratorVariable: 'row', body }))).toEqual([]); + }); +}); + +describe('assignment targets — the names the executor binds, in each of its three shapes', () => { + it('reads the canonical map\'s keys, a bare config\'s top-level keys, and a legacy array item\'s variable', () => { + const at = (config: unknown) => flowAssignmentTargets(config).map(({ path, name }) => `${path.join('.')}=${name}`); + // The map: its keys, and a top-level key beside it is never bound. + expect(at({ assignments: { a: 1, b: 2 }, c: 3 })).toEqual(['assignments.a=a', 'assignments.b=b']); + // The bare config: every top-level key. + expect(at({ a: 1, b: 2 })).toEqual(['a=a', 'b=b']); + // The legacy array: each item's first non-null `variable` / `name` / `key`, when a non-empty string. + expect(at({ assignments: [{ variable: 'v', value: 1 }, { name: 'n' }, { key: 'k' }, { variable: null, name: 'm' }, { variable: '' }, 5, null] })) + .toEqual(['assignments.0.variable=v', 'assignments.1.name=n', 'assignments.2.key=k', 'assignments.3.name=m']); + for (const config of [null, undefined, [], 'x', 5]) expect(at(config), JSON.stringify(config)).toEqual([]); + }); + + it('a region body\'s assignment is refused at the path the author wrote', () => { + const region = { nodes: [{ id: 'set', type: 'assignment', label: 'Set', config: { assignments: { $total: 1 } } }], edges: [] }; + expect(issuesOf(flowWith('loop', { collection: 'rows', body: region })).map(({ path }) => path)).toEqual([ + 'nodes.1.config.body.nodes.0.config.assignments.$total', + ]); + }); +}); + +describe('the enumeration pin — every flow binding is listed, and every listed one carries the rule', () => { + it('the rule\'s vocabulary and the site table name the same positions', () => { + expect([...new Set(BINDING_SITES.map((site) => site.key))].sort()).toEqual([...FLOW_BINDING_KEYS].sort()); + }); + + it('every `*Variable` key of every automation schema carries the rule — a new one without it turns this red', () => { + // Discovery by NAME: the binding keys the protocol spells `*Variable`, at + // any depth of any schema `automation/index.ts` exports. A binding + // position spelled otherwise (a field's `name`, a variable's `name`, an + // `assignments` key) cannot be found this way; those are the vocabulary + // rows above, held by the previous pin. + const discovered: Array<{ exportName: string; key: string; pattern?: string }> = []; + for (const [exportName, value] of Object.entries(automation)) { + if (value === null || (typeof value !== 'object' && typeof value !== 'function') || !('_zod' in value)) continue; + const json = z.toJSONSchema(value as z.ZodType, { io: 'input', unrepresentable: 'any' }); + const walk = (node: unknown): void => { + if (Array.isArray(node)) return node.forEach(walk); + if (node === null || typeof node !== 'object') return; + const properties = (node as { properties?: Record }).properties; + for (const [key, property] of Object.entries(properties ?? {})) { + if (/Variable$/.test(key)) discovered.push({ exportName, key, pattern: property?.pattern }); + } + Object.values(node).forEach(walk); + }; + walk(json); + } + for (const { exportName, key, pattern } of discovered) { + const at = `${exportName}.${key}`; + expect(FLOW_BINDING_KEYS as readonly string[], at).toContain(key); + expect(pattern, `${at} publishes no pattern — compose flowBoundVariableNameSchema`).toBeDefined(); + const re = new RegExp(pattern!); + expect(re.test('x') && re.test('a$b'), at).toBe(true); + expect(re.test('$x') || re.test('$record'), at).toBe(false); + expect(BINDING_SITES.some((site) => site.declaredBy === exportName && site.key === key), `${at} has no row in BINDING_SITES`).toBe(true); + } + // FLOOR: the walk reaches every `*Variable` row of the table, so a walker + // that silently finds nothing cannot pass the loop above vacuously. + for (const site of BINDING_SITES.filter((s) => s.declaredBy !== undefined)) { + expect(discovered.some((d) => d.exportName === site.declaredBy && d.key === site.key), `${site.label} not discovered`).toBe(true); + } + }); + + it('the positions not spelled `*Variable` carry the rule in the published JSON Schema too', () => { + const json = (schema: unknown) => z.toJSONSchema(schema as z.ZodType, { io: 'input', unrepresentable: 'any' }) as { + properties: Record; + }; + const patterns = { + 'FlowVariable.name': json(FlowVariableSchema).properties.name?.pattern, + 'ScreenFieldConfig.name': json(ScreenFieldConfigSchema).properties.name?.pattern, + 'AssignmentConfig.assignments (keys)': json(AssignmentConfigSchema).properties.assignments?.propertyNames?.pattern, + }; + for (const [at, pattern] of Object.entries(patterns)) { + expect(pattern, at).toBeDefined(); + const re = new RegExp(pattern!); + expect(re.test('x') && re.test('a$b'), at).toBe(true); + expect(re.test('$x'), at).toBe(false); + } + }); +}); + +describe('ADR-0087 — the close-out is registered as a D3 semantic entry of protocol 18', () => { + const ENTRY_ID = 'flow-binding-name-dollar-refused'; + + it('names every remaining binding position, with no D2 conversion', () => { + const entries = MIGRATIONS_BY_MAJOR[18]!.semantic.filter((e) => e.id === ENTRY_ID); + expect(entries).toHaveLength(1); + for (const position of ['iteratorVariable', 'indexVariable', 'idVariable', 'fields[].name', 'variables[].name', 'assignment']) { + expect(entries[0]!.surface, position).toContain(position); + } + expect(entries[0]!.conversionIds ?? []).toEqual([]); + }); + + it('names the step-18 rationale fragment for it', () => { + expect(MIGRATIONS_BY_MAJOR[18]!.rationale).toContain(ENTRY_ID); + }); +}); From d35961fcd8b81ce9e6e018e6e1e6f7f978be3977 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 22:26:23 +0000 Subject: [PATCH 3/5] chore(spec): declare the bare assignment config's key rule as a dropped refinement The `$` rule on a bare legacy `assignment` config's top-level keys is a superRefine (a catchall sees values, never keys), so the published JSON Schema cannot state it; the ledger names the new site. Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude --- packages/spec/dropped-refinements.baseline.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/spec/dropped-refinements.baseline.json b/packages/spec/dropped-refinements.baseline.json index 8df921dd578..3e28e0e27a9 100644 --- a/packages/spec/dropped-refinements.baseline.json +++ b/packages/spec/dropped-refinements.baseline.json @@ -3,7 +3,7 @@ "measured": { "zod": "4.4.3", "publishedSchemasWithDroppedRefinements": 226, - "droppedRefinementSites": 704, + "droppedRefinementSites": 705, "refinementSitesThatDidProject": 369, "refinementSitesWithNoJsonFormToCompare": 0 }, @@ -462,6 +462,7 @@ }, "automation/AssignmentConfig": { "sites": [ + "out", "out.assignments.out.valueType", "out.catchall" ] From c647be15ea482e46b10d34430d381985faa555ce Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 22:28:06 +0000 Subject: [PATCH 4/5] docs(spec): regenerate the automation references for the binding-name rule Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude --- content/docs/references/api/automation-api.mdx | 2 +- .../docs/references/automation/builtin-node-config.mdx | 10 +++++----- content/docs/references/automation/control-flow.mdx | 4 ++-- content/docs/references/automation/flow.mdx | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/content/docs/references/api/automation-api.mdx b/content/docs/references/api/automation-api.mdx index 4189c815a87..75a2db85a60 100644 --- a/content/docs/references/api/automation-api.mdx +++ b/content/docs/references/api/automation-api.mdx @@ -135,7 +135,7 @@ const result = AutomationApiErrorCode.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **name** | `string` | ✅ | Variable name | +| **name** | `string` | ✅ | Variable name — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` | | **type** | `string` | ✅ | Data type (text, number, boolean, object, list) | | **isInput** | `boolean` | optional (default: `false`) | Is input parameter | | **isOutput** | `boolean` | optional (default: `false`) | Is output parameter | diff --git a/content/docs/references/automation/builtin-node-config.mdx b/content/docs/references/automation/builtin-node-config.mdx index e669a6309c8..6317182867e 100644 --- a/content/docs/references/automation/builtin-node-config.mdx +++ b/content/docs/references/automation/builtin-node-config.mdx @@ -208,8 +208,8 @@ A value: a CEL value envelope `{ dialect: 'cel', source }` evaluated by the expr | :--- | :--- | :--- | :--- | | **collection** | `string \| any[]` | ✅ | Template/variable resolving to the array to process (an inline array is accepted) | | **flowName** | `string` | ✅ | Subflow run for each item — it may pause (e.g. an approval) | -| **iteratorVariable** | `string` | optional (default: `"item"`) | Variable holding the current item | -| **indexVariable** | `string` | optional | Optional variable holding the current index | +| **iteratorVariable** | `string` | optional (default: `"item"`) | Variable holding the current item — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` | +| **indexVariable** | `string` | optional | Optional variable holding the current index — a name without a leading `$` (the `$` names are the flow engine's own) | | **itemObject** | `string` | optional | When items are records, the object they belong to (exposes each item as the child's record) | | **input** | `Record` | optional | Params passed to each item's subflow, keyed by its input variables: each value a CEL value envelope `{ dialect: 'cel', source }` evaluated per item (the item variable is in scope), or a literal written as it is — a `{…}` template token is refused | | **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 }}` | @@ -228,7 +228,7 @@ A value: a CEL value envelope `{ dialect: 'cel', source }` evaluated by the expr | **fields** | `{ name: string; label?: string; type?: string; required?: boolean; … }[]` | optional | Input fields collected on this screen | | **waitForInput** | `boolean` | optional | Pause to show the screen even with no fields; false forces a server pass-through | | **objectName** | `string` | optional | Render this object's full create/edit form instead of a flat field list | -| **idVariable** | `string` | optional | Object form only: variable bound to the saved record's id | +| **idVariable** | `string` | optional | Object form only: variable bound to the saved record's id — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` | | **mode** | `Enum<'create' \| 'edit'>` | optional (default: `"create"`) | Object form only: create (default) or edit; 'edit' needs a recordId to name its target | | **recordId** | `string` | optional | Object form only: id of the record to edit (required for mode: 'edit' to be useful) | | **defaults** | `Record` | optional | Object form only: prefilled values | @@ -237,7 +237,7 @@ A value: a CEL value envelope `{ dialect: 'cel', source }` evaluated by the expr | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **name** | `string` | ✅ | Field name (the flow variable the value binds to) | +| **name** | `string` | ✅ | Field name (the flow variable the value binds to) — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` | | **label** | `string` | optional | Display label | | **type** | `string` | optional | Input type | | **required** | `boolean` | optional | Whether a value is required to submit | @@ -259,7 +259,7 @@ A value: a CEL value envelope `{ dialect: 'cel', source }` evaluated by the expr | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **name** | `string` | ✅ | Field name (the flow variable the value binds to) | +| **name** | `string` | ✅ | Field name (the flow variable the value binds to) — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` | | **label** | `string` | optional | Display label | | **type** | `string` | optional | Input type | | **required** | `boolean` | optional | Whether a value is required to submit | diff --git a/content/docs/references/automation/control-flow.mdx b/content/docs/references/automation/control-flow.mdx index 9d7d1ae9fb2..f10e8477a28 100644 --- a/content/docs/references/automation/control-flow.mdx +++ b/content/docs/references/automation/control-flow.mdx @@ -145,8 +145,8 @@ const result = FlowRegionSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **collection** | `string \| any[]` | ✅ | Template/variable resolving to the array to iterate (an inline array is accepted) | -| **iteratorVariable** | `string` | optional (default: `"item"`) | Loop variable holding the current item | -| **indexVariable** | `string` | optional | Optional loop variable holding the current index | +| **iteratorVariable** | `string` | optional (default: `"item"`) | Loop variable holding the current item — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` | +| **indexVariable** | `string` | optional | Optional loop variable holding the current index — a name without a leading `$` (the `$` names are the flow engine's own) | | **maxIterations** | `integer` | optional | Hard cap on iterations (clamped to the engine ceiling) | | **body** | `{ nodes: object[]; edges?: object[] }` | optional | Loop body region (omit for legacy flat-graph loops) | diff --git a/content/docs/references/automation/flow.mdx b/content/docs/references/automation/flow.mdx index e59becd23f1..2bd0350b40f 100644 --- a/content/docs/references/automation/flow.mdx +++ b/content/docs/references/automation/flow.mdx @@ -56,7 +56,7 @@ const result = FlowSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **name** | `string` | ✅ | Variable name | +| **name** | `string` | ✅ | Variable name — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` | | **type** | `string` | ✅ | Data type (text, number, boolean, object, list) | | **isInput** | `boolean` | optional (default: `false`) | Is input parameter | | **isOutput** | `boolean` | optional (default: `false`) | Is output parameter | @@ -223,7 +223,7 @@ const result = FlowSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **name** | `string` | ✅ | Variable name | +| **name** | `string` | ✅ | Variable name — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` | | **type** | `string` | ✅ | Data type (text, number, boolean, object, list) | | **isInput** | `boolean` | optional (default: `false`) | Is input parameter | | **isOutput** | `boolean` | optional (default: `false`) | Is output parameter | From 90e18ee6e7fb69e646d352c57c5d1447799fdb95 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 23:01:16 +0000 Subject: [PATCH 5/5] fix(spec/automation)!: judge a binding name's first non-blank character The `screen` and `script` executors trim a binding name before they bind it, so `idVariable: ' $id'` passed a first-character rule and its screen then named `$id`, refused on resume. The rule now refuses a name whose first non-blank character is `$`, at every binding key. Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude --- .../22572-flow-binding-name-dollar-refused.md | 6 +++-- .../flow-bound-variable-name.test.ts | 19 ++++++++++---- .../automation/flow-bound-variable-name.ts | 25 +++++++++++++------ .../18.flow-binding-name-dollar-refused.ts | 20 ++++++++++----- packages/spec/src/migrations/registry.ts | 24 ++++++++++++------ 5 files changed, 66 insertions(+), 28 deletions(-) diff --git a/.changeset/22572-flow-binding-name-dollar-refused.md b/.changeset/22572-flow-binding-name-dollar-refused.md index bb18861043d..90723dcd1e4 100644 --- a/.changeset/22572-flow-binding-name-dollar-refused.md +++ b/.changeset/22572-flow-binding-name-dollar-refused.md @@ -15,15 +15,17 @@ Clause-②: no (narrowing) - `iteratorVariable: '$record'` on a `loop` or `map` left `$record` holding the last item for the rest of the run, and `indexVariable: '$runId'` left `$runId` holding an index; - `assignments: { $record: … }` overwrote the trigger record; - a declared variable named `$record` was overwritten by the engine at run start, so its `defaultValue` never reached the run; -- a `screen` whose `idVariable` or field `name` was a `$` name paused, and then could never be submitted: the resume carrying the value was refused with `INVALID_SIGNAL`. +- a `screen` whose `idVariable` or field `name` was a `$` name paused, and then could never be submitted: the resume carrying the value was refused with `INVALID_SIGNAL`; +- a leading blank hid the `$` from a first-character rule, while the `screen` and `script` executors trim a name before they bind it: under that rule `idVariable: ' $id'` registered, the paused screen named `$id`, and its resume was refused the same way; a `script`'s `outputVariable: ' $record'` passes the protocol-18 rule too and is trimmed to `$record` by its executor (read from the executor, not run). **What is refused.** - `iteratorVariable` and `indexVariable` on `LoopConfigSchema` and `MapConfigSchema`, `idVariable` on `ScreenConfigSchema`, `name` on `ScreenFieldConfigSchema` and on `FlowVariableSchema`: a name that starts with `$`. 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. +- Every binding key — `outputVariable` and `errorVariable` included — now judges the first NON-BLANK character, so `' $x'` is refused like `'$x'`. The pattern's `\s` is exactly the set `String.prototype.trim` removes. - An `assignment` node's targets, in each shape its executor binds: a key of the `assignments` map (`AssignmentConfigSchema` states it as `propertyNames.pattern`), a top-level key of the bare legacy config, and the `variable` (or `name`, `key`) of a legacy `assignments: [{ variable, value }]` item. - `FlowSchema.parse`, `registerFlow` and `objectstack validate` refuse the flow where the name was written — `nodes.N.config.iteratorVariable`, `nodes.N.config.fields.M.name`, `nodes.N.config.assignments.NAME`, `variables.N.name` — inside a region body too. A stored flow carrying one is skipped at boot with a warn naming it, and the `loop`, `map` and `screen` executors' own contract parse refuses the node at run time. -**Unchanged.** Any name that does not start with `$`, a `$` later in the name (`a$b`) included; the defaults (`iteratorVariable` is still `item`); an empty string where the key took one; a body-less legacy `loop`, whose `iteratorVariable` nothing reads; and every hole the text-slot judge already admits. Non-string values keep the type refusal they had. +**Unchanged.** Any name whose first non-blank character is not `$`, a `$` later in the name (`a$b`) included; the defaults (`iteratorVariable` is still `item`); an empty string where the key took one; a body-less legacy `loop`, whose `iteratorVariable` nothing reads; and every hole the text-slot judge already admits. Non-string values keep the type refusal they had. ## FROM → TO diff --git a/packages/spec/src/automation/flow-bound-variable-name.test.ts b/packages/spec/src/automation/flow-bound-variable-name.test.ts index 5ae1b981c71..9f1d1412785 100644 --- a/packages/spec/src/automation/flow-bound-variable-name.test.ts +++ b/packages/spec/src/automation/flow-bound-variable-name.test.ts @@ -371,8 +371,17 @@ describe('every binding door refuses a `$` name — #22572 closes the family', ( } }); + it.each(BINDING_SITES)('$label: a leading blank does not hide the `$` — a trimming executor would bind it', ({ flow, flowPath }) => { + // `screen`'s `idVariable` and `script`'s `outputVariable` are trimmed by + // their executors before they bind, so the rule judges the first non-blank + // character, at every site alike. + for (const name of [' $x', '\t$record', '\n$x']) { + expect(issuesOf(flow(name)).map(({ path }) => path), JSON.stringify(name)).toEqual([flowPath(name)]); + } + }); + it.each(BINDING_SITES)('CONTROL: $label: a plain name and a `$` past the first character parse clean', ({ contract, flow }) => { - for (const name of ['x', 'a$b']) { + for (const name of ['x', 'a$b', ' x']) { if (contract) expect(contract.schema.safeParse(contract.config(name)).success, name).toBe(true); expect(issuesOf(flow(name)), name).toEqual([]); } @@ -463,8 +472,8 @@ describe('the enumeration pin — every flow binding is listed, and every listed expect(FLOW_BINDING_KEYS as readonly string[], at).toContain(key); expect(pattern, `${at} publishes no pattern — compose flowBoundVariableNameSchema`).toBeDefined(); const re = new RegExp(pattern!); - expect(re.test('x') && re.test('a$b'), at).toBe(true); - expect(re.test('$x') || re.test('$record'), at).toBe(false); + expect(re.test('x') && re.test('a$b') && re.test(' x'), at).toBe(true); + expect(re.test('$x') || re.test('$record') || re.test(' $x'), at).toBe(false); expect(BINDING_SITES.some((site) => site.declaredBy === exportName && site.key === key), `${at} has no row in BINDING_SITES`).toBe(true); } // FLOOR: the walk reaches every `*Variable` row of the table, so a walker @@ -486,8 +495,8 @@ describe('the enumeration pin — every flow binding is listed, and every listed for (const [at, pattern] of Object.entries(patterns)) { expect(pattern, at).toBeDefined(); const re = new RegExp(pattern!); - expect(re.test('x') && re.test('a$b'), at).toBe(true); - expect(re.test('$x'), at).toBe(false); + expect(re.test('x') && re.test('a$b') && re.test(' x'), at).toBe(true); + expect(re.test('$x') || re.test(' $x'), at).toBe(false); } }); }); diff --git a/packages/spec/src/automation/flow-bound-variable-name.ts b/packages/spec/src/automation/flow-bound-variable-name.ts index 62d5bea68e9..b7564636d07 100644 --- a/packages/spec/src/automation/flow-bound-variable-name.ts +++ b/packages/spec/src/automation/flow-bound-variable-name.ts @@ -35,8 +35,8 @@ * They agree now, on the rule that judge already reads: the `$` names are * reserved for the engine (a resume signal may not write one either — * `IAutomationService.resume`'s `INVALID_SIGNAL`). So a binding refuses a name - * that starts with `$`, and its remedy is the same name without the `$`, read - * as `{{ name }}`. The one `$` name an author may write is the one the engine + * whose first non-blank character is `$`, and its remedy is the same name + * without the `$`, read as `{{ name }}`. The one `$` name an author may write is the one the engine * itself binds there: `errorVariable: '$error'`, the key's default — the * caught error the engine publishes as `$error` either way. * @@ -109,15 +109,24 @@ export type FlowBindingKey = keyof typeof BINDING_CLAUSES; export const FLOW_BINDING_KEYS: readonly FlowBindingKey[] = Object.keys(BINDING_CLAUSES) as FlowBindingKey[]; /** - * Any string whose first character is not `$` — the empty string included, - * which every executor reads as "no binding". `[\s\S]` rather than `.` so a - * name carrying a line break is judged on its first character too. + * Any string whose first NON-BLANK character is not `$` — the empty and the + * blank string included, which every executor reads as "no binding". + * + * Judged past leading whitespace because two executors trim before they bind + * (`screen`'s `idVariable`, `script`'s `outputVariable`): `' $id'` passed a + * first-character rule, and its screen then named `$id`, refused on resume + * (#22572, measured). + * `\s` is exactly the set `String.prototype.trim` removes (ECMA-262 + * WhiteSpace ∪ LineTerminator, `shared/refinement-projection.ts`), so the + * pattern refuses a name whose trimmed form is `$`-led, and nothing a trim + * would not turn into one. `[\s\S]` rather than `.` so a name carrying a + * line break is judged too. */ -const NOT_DOLLAR_LED = '[^$][\\s\\S]*'; +const NOT_DOLLAR_LED = '[^$\\s][\\s\\S]*'; /** Why a `$`-led name is refused at a binding position, with its remedy. */ function boundVariableNameRefusal(key: FlowBindingKey, name: string): string { - const bare = name.replace(/^\$+/, ''); + const bare = name.trim().replace(/^\$+/, ''); const read = key === 'errorVariable' ? `${bare}.message` : bare; const lead = `\`${name}\` is a \`$\` name, and the \`$\` names are reserved for the flow engine's own variables (a resume ` @@ -142,7 +151,7 @@ function boundVariableNameRefusal(key: FlowBindingKey, name: string): string { */ export function flowBoundVariableNameSchema(key: FlowBindingKey) { const engineOwn = key === 'errorVariable' ? `${ENGINE_ERROR_VARIABLE.replace('$', '\\$')}|` : ''; - return z.string().regex(new RegExp(`^(?:${engineOwn}${NOT_DOLLAR_LED})?$`), { + return z.string().regex(new RegExp(`^\\s*(?:${engineOwn}${NOT_DOLLAR_LED})?$`), { error: (issue) => boundVariableNameRefusal(key, String(issue.input)), }); } diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-binding-name-dollar-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-binding-name-dollar-refused.ts index f9f4959ff36..74fd67e41b9 100644 --- a/packages/spec/src/migrations/entries/semantic/18.flow-binding-name-dollar-refused.ts +++ b/packages/spec/src/migrations/entries/semantic/18.flow-binding-name-dollar-refused.ts @@ -7,10 +7,12 @@ import type { SemanticMigration } from '../../types.js'; // dollar name, by the same rule — a loop or map iteratorVariable and // indexVariable, an object-form screen's idVariable, a screen field's name, a // declared flow variable's name, and an assignment node's targets in each shape -// its executor reads. It narrows a flow's accept set; no key is removed, so -// there is no tombstone and no RETIRED_KEYS_BY_MAJOR row. Semantic-only — 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. +// its executor reads — judged on the first non-blank character at every binding +// key, since two executors trim a name before they bind it. It narrows a flow's +// accept set; no key is removed, so there is no tombstone and no +// RETIRED_KEYS_BY_MAJOR row. Semantic-only — 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. // // No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it // inside a code span and a table cell. @@ -21,7 +23,9 @@ export const entry: SemanticMigration = { + 'flows[].nodes[].config.idVariable and config.fields[].name of a screen node, flows[].variables[].name, ' + 'and an assignment node\'s targets — a key of config.assignments, a top-level key of a config with no ' + 'assignments map, or the variable of a legacy assignments array item — when the name starts with a dollar ' - + 'sign, such as $row. Reachable wherever a flow is authored or stored: defineStack flows sources, ' + + 'sign, such as $row; and on every binding key, the outputVariable and errorVariable of protocol 18 included, ' + + 'a name whose first non-blank character is a dollar sign, such as \' $id\'. Reachable wherever a flow is ' + + 'authored or stored: defineStack flows sources, ' + 'defineFlow, an exported stack passed to objectstack validate or objectstack compile, a flow saved from the ' + 'Studio flow designer, and a flow row already in sys_metadata', replacement: @@ -40,7 +44,11 @@ export const entry: SemanticMigration = { + 'indexVariable of $runId left the run id holding an index, an assignment to $record overwrote it, a ' + 'declared variable named $record was overwritten by the engine at run start, and a screen whose ' + 'idVariable or field name was a dollar name could never be submitted — the resume that carried the value ' - + 'was refused as a write to an engine variable. Each binding now gives the text slots\' answer: each key ' + + 'was refused as a write to an engine variable. A leading blank hid the dollar sign from the first-character ' + + 'rule the outputVariable and errorVariable keys took, while the screen and script executors trim a name ' + + 'before they bind it, so idVariable: \' $id\' registered and its screen could not be submitted either. Each ' + + 'binding now gives the text slots\' answer, ' + + 'judged on the first non-blank character: each key ' + 'states the rule as a JSON Schema pattern (propertyNames for the assignments map), so the published schema ' + 'refuses what the parse refuses, and the flow parse, registerFlow and objectstack validate refuse such a ' + 'name where it was written, naming the same name without the dollar sign — as does the run itself for a ' diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 358d465d56b..d52db71105e 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5675,7 +5675,9 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ + 'variable\'s `name`, and an `assignment` node\'s targets in each shape its executor reads refuse a name ' + 'that starts with `$`, by the same rule and with the same remedy. A binding over an engine name replaced ' + 'it for the rest of the run (`iteratorVariable: \'$record\'`), a declared `$record` was overwritten at ' - + 'run start, and a `screen` binding a `$` name could never be submitted, its resume refused. Each key ' + + 'run start, and a `screen` binding a `$` name could never be submitted, its resume refused. Every ' + + 'binding key, `outputVariable` and `errorVariable` included, now judges the first non-blank character, ' + + 'since two executors trim a name before they bind it (`idVariable: \' $id\'` named `$id` on its screen). Each key ' + 'states the rule as a `pattern` (the `assignments` map as `propertyNames`), and the flow parse, ' + '`registerFlow` and `objectstack validate` refuse such a name where it was written. No D2 conversion ' + 'exists, for the same reason. Its D3 record is the semantic entry `flow-binding-name-dollar-refused`.', @@ -13579,10 +13581,12 @@ const step18: MigrationStep = { // dollar name, by the same rule — a loop or map iteratorVariable and // indexVariable, an object-form screen's idVariable, a screen field's name, a // declared flow variable's name, and an assignment node's targets in each shape - // its executor reads. It narrows a flow's accept set; no key is removed, so - // there is no tombstone and no RETIRED_KEYS_BY_MAJOR row. Semantic-only — 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. + // its executor reads — judged on the first non-blank character at every binding + // key, since two executors trim a name before they bind it. It narrows a flow's + // accept set; no key is removed, so there is no tombstone and no + // RETIRED_KEYS_BY_MAJOR row. Semantic-only — 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. // // No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it // inside a code span and a table cell. @@ -13593,7 +13597,9 @@ const step18: MigrationStep = { + 'flows[].nodes[].config.idVariable and config.fields[].name of a screen node, flows[].variables[].name, ' + 'and an assignment node\'s targets — a key of config.assignments, a top-level key of a config with no ' + 'assignments map, or the variable of a legacy assignments array item — when the name starts with a dollar ' - + 'sign, such as $row. Reachable wherever a flow is authored or stored: defineStack flows sources, ' + + 'sign, such as $row; and on every binding key, the outputVariable and errorVariable of protocol 18 included, ' + + 'a name whose first non-blank character is a dollar sign, such as \' $id\'. Reachable wherever a flow is ' + + 'authored or stored: defineStack flows sources, ' + 'defineFlow, an exported stack passed to objectstack validate or objectstack compile, a flow saved from the ' + 'Studio flow designer, and a flow row already in sys_metadata', replacement: @@ -13612,7 +13618,11 @@ const step18: MigrationStep = { + 'indexVariable of $runId left the run id holding an index, an assignment to $record overwrote it, a ' + 'declared variable named $record was overwritten by the engine at run start, and a screen whose ' + 'idVariable or field name was a dollar name could never be submitted — the resume that carried the value ' - + 'was refused as a write to an engine variable. Each binding now gives the text slots\' answer: each key ' + + 'was refused as a write to an engine variable. A leading blank hid the dollar sign from the first-character ' + + 'rule the outputVariable and errorVariable keys took, while the screen and script executors trim a name ' + + 'before they bind it, so idVariable: \' $id\' registered and its screen could not be submitted either. Each ' + + 'binding now gives the text slots\' answer, ' + + 'judged on the first non-blank character: each key ' + 'states the rule as a JSON Schema pattern (propertyNames for the assignments map), so the published schema ' + 'refuses what the parse refuses, and the flow parse, registerFlow and objectstack validate refuse such a ' + 'name where it was written, naming the same name without the dollar sign — as does the run itself for a '