Repository navigation
Commit 9f5eca5
fix(spec/automation)!: refuse a
Fixes #22572
Clause-②: no (narrowing)
## What this does
This is the close-out of the family "the `$` names are the flow engine's
at every binding door", after #22477 (the read side) and #22502
(`outputVariable`, `errorVariable`). Every remaining place a flow binds
a variable by name now refuses a name whose first non-blank character is
`$`. It uses the one rule PR #22569 added, `flowBoundVariableNameSchema`
in `packages/spec/src/automation/flow-bound-variable-name.ts`. The rule
stays package-internal, so there is no new export (`check:api-surface`
is green).
| binding position | contract | how the published JSON Schema states it
|
|:--|:--|:--|
| `loop` `iteratorVariable` / `indexVariable` | `LoopConfigSchema` |
`pattern` (default `item` and `minLength: 1` kept) |
| `map` `iteratorVariable` / `indexVariable` | `MapConfigSchema` |
`pattern` (default `item` kept) |
| object-form `screen` `idVariable` | `ScreenConfigSchema` | `pattern` |
| `screen` field `name` (an in-place addition, see below) |
`ScreenFieldConfigSchema` | `pattern` |
| declared variable `name` | `FlowVariableSchema` | `pattern` |
| `assignment` target, a key of the `assignments` map |
`AssignmentConfigSchema` (key schema) | `propertyNames.pattern` |
| `assignment` target, a top-level key of a bare config |
`AssignmentConfigSchema` (`superRefine`) | not stated: one new declared
site in `dropped-refinements.baseline.json` |
| `assignment` target, a legacy `[{ variable, value }]` item | the
`FlowSchema` arm | not stated (the contract refuses the array form
whole) |
**One sentence at every door.** The refusal is the rule's own message.
It says the `$` names are reserved for the engine, names the key, and
gives the remedy: the same name without the `$`, read as `{{ name }}`.
- **`loop`, `map` and `screen` keys.** These are refused through
`flowNodeConfigRefusals`, so `FlowSchema.parse`, `registerFlow`,
`objectstack validate` and the run itself (`parseNodeConfig`) all refuse
them at `nodes.N.config.KEY`.
- **`FlowVariableSchema.name`.** `FlowSchema` refuses it directly, at
`variables.N.name`.
- **`assignment` targets.** No executor contract parses an `assignment`
config, because `getBuiltinNodeConfigContracts()` has no `assignment`
entry. So `FlowSchema.parse` never applied `AssignmentConfigSchema`. A
new arm in the `FlowSchema` superRefine walks `collectFlowGraphs`,
region bodies included. It judges `flowAssignmentTargets(config)`, which
covers the three shapes the executor binds (`service-automation`
`builtin/logic-nodes.ts`) and only those.
**The first non-blank character, at every binding key.** The `screen`
and `script` executors trim a name before they bind it
(`cfg.idVariable.trim()`, `cfg.outputVariable?.trim()`). So `idVariable:
' $id'` passed a first-character rule. The rule's regex is now
`^\s*(?:[^$\s][\s\S]*)?$` (with `\$error|` for `errorVariable`).
JavaScript's `\s` is exactly the set `String.prototype.trim` removes, so
the rule refuses a name exactly when its trimmed form starts with `$`.
This applies to `outputVariable` and `errorVariable` too. #22502's
changeset is still unreleased on the same `18.0.0-next` line.
## Additions beyond the card's list (bounded in-place fixes; all four
conditions hold)
The fixes below meet all four conditions: the same defect class, a
mechanical fix (compose the one rule), the claim's own file with no
other claim on it, and the same gate family.
1. **`ScreenFieldConfigSchema.name`.** Its own describe says "the flow
variable the value binds to". The resume that submits a flat screen
writes each field's value under its `name`. A `$` name there makes the
screen impossible to submit (measured below).
2. **The leading blank**, as above. Measured: under the first-character
rule, `idVariable: ' $id'` registered. The paused screen then named
`$id`, and its resume answered `INVALID_SIGNAL`.
The claim's file surface needs two more entries:
`ScreenFieldConfigSchema` (in `builtin-node-config.zod.ts`, already
listed) and `packages/spec/dropped-refinements.baseline.json`. That file
is a forced debt of the bare-shape key rule: `build-schemas.ts` refuses
the build until the site is declared, and its header total moves from
704 to 705. The four regenerated reference pages are within "as
`check:generated` decides".
## The PM's mechanism assumptions, measured at `origin/main`
`0f77ff5202`
1. **The sites.** Every one accepted a `$` name, measured with each
contract's `safeParse` and `FlowSchema.safeParse` against the built
dist:
| position | at `0f77ff5202` | input | result |
|:--|:--|:--|:--|
| `LoopConfigSchema.iteratorVariable` | `control-flow.zod.ts:220`,
`z.string().min(1).default('item')` | `'$row'` | accepted |
| `LoopConfigSchema.indexVariable` | `:222` | `'$i'` | accepted |
| `MapConfigSchema.iteratorVariable` |
`builtin-node-config.zod.ts:1059`, `z.string().default('item')` (where
#19939 S1 left the block) | `'$row'` | accepted |
| `MapConfigSchema.indexVariable` | `:1061` | `'$i'` | accepted |
| `ScreenConfigSchema.idVariable` | `:862` | `'$id'` | accepted |
| `ScreenFieldConfigSchema.name` | `:700` (not on the card) | `'$f'` |
accepted |
| `AssignmentConfigSchema`, the `assignments` map key | `:1163`,
`z.record(z.string().min(1), ...)` | `'$y'` | accepted |
| `AssignmentConfigSchema`, a bare top-level key | `.catchall()` sees
values, never keys | `'$y'` | accepted |
| `FlowVariableSchema.name` | `flow.zod.ts:218`, `z.string()` | `'$x'` |
accepted |
| `FlowSchema` with `variables: [{ name: '$x' }]` and `assignment` `{
assignments: { $y: 1 } }` | | | accepted |
2. **Defaults.** Both `iteratorVariable` defaults stay `item` (pinned).
The published JSON carries `pattern` beside `default: "item"`.
`check:authorable-surface` is green, and
`authorable-defaults/automation.json` is unchanged.
3. **The engine's own names.** No shipped flow or fixture binds a `$`
name on these positions (item 4). The run-time effect was measured with
`AutomationEngine` at `0f77ff5202`, through a scratch test that is not
committed:
- `loop` with `iteratorVariable: '$record'` and `indexVariable:
'$runId'`: after the loop, a screen titled `record is {{ $record }},
runId is {{ $runId }}` rendered `record is B, runId is 1`.
- `map` with `iteratorVariable: '$record'`: `record is B`.
- `assignment` with `{ assignments: { $record: 'clobbered' } }`: `record
is clobbered`.
- a declared `$record` with `defaultValue: 'mine'`: the engine's own
seeding overwrote it, and the title showed the trigger record.
- object-form `screen` with `idVariable: '$id'`: the run paused, and the
resume the console sends (`{ $id: id }`, `FlowRunner.tsx`
`onObjectFormSaved` at the pinned objectui `20c6d351ad`) answered
`INVALID_SIGNAL`.
- flat `screen` field `name: '$x'`: the same `INVALID_SIGNAL`.
- After this change, `registerFlow` refuses all six flows: a `ZodError`
from `canonicalizeStoredFlow` at the binding path.
4. **The reach: zero newly refused bindings.**
- **The real parse.** All 35 flows the examples ship (`app-crm` 1,
`app-todo` 4, `app-showcase` 30) were run through
`FlowSchema.safeParse`. All parse OK, and the before and after listings
are byte-identical.
- **An AST scan.** The scanner reads TypeScript object-literal
properties, plus a text arm for JSON and YAML. On a control fixture it
found 11 of 11 planted positives. It covered `examples` (234 files),
`packages/platform-objects` (167 files; it ships no flow),
`packages/qa/dogfood` (280), the rest of `packages`, `skills`, `apps`
and `scripts`, and `hotcrm` at `1d7148bf2d` (570 files; a read-only
clone, deleted afterwards). It found no `$`-led binding on any position.
Its only hits were `errorVariable: '$error'`, which stays legal, and
filter operator keys.
- **The pinned objectui `20c6d351ad`.** None. The control
(`errorVariable: '$error'`) hit.
5. **The enumeration pin.** The pin finds binding keys by name. It walks
the JSON Schema of every Zod export of `automation/index.ts` (72
schemas) for any property spelled `*Variable`, at any depth. Each one
must publish the rule's `pattern`, be in the rule's vocabulary, and have
a row in the pin's site table. A floor keeps the walk from passing over
nothing.
- **The honest limit.** A binding position not spelled `*Variable`
cannot be found this way: a field's `name`, a variable's `name`, an
`assignments` key. Those three are hand-listed. They are the rule's
exported vocabulary `FLOW_BINDING_KEYS`, which the pin holds equal to
the site table.
## ADR-0087 disposition
- **D3 entry.**
`packages/spec/src/migrations/entries/semantic/18.flow-binding-name-dollar-refused.ts`,
on protocol 18. Its step-18 rationale fragment has `order: 93`, after
#22502's `92`. `registry.ts` was regenerated with
`gen:migration-registry`. It is still tracked, because PR #22706 had not
landed at `f59a73c395`.
- **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.
- **Changeset.** `.changeset/22572-flow-binding-name-dollar-refused.md`:
`@objectstack/spec` `major` in pre mode, with `Clause-②: no (narrowing)`
and the `registered flow-binding-name-dollar-refused` marker.
`check-adr-0087-registration` reads `[major+BREAKING+clause-②-narrowing]
registered flow-binding-name-dollar-refused (new here)`.
## Tests (at `af752544fb`)
- **New pins**, in
`packages/spec/src/automation/flow-bound-variable-name.test.ts`. The
file went from 26 to 115 tests. The pin table has 18 binding sites, and
each site:
- is refused by its contract, with the remedy;
- is refused by `FlowSchema` at exactly its path, and nowhere else;
- refuses the engine's names `$record`, `$runId`, `$loopItems` and `$`;
- refuses a leading blank (`' $x'`, a tab, a newline);
- accepts the controls `x`, `a$b` and `' x'`.
The file also pins:
- the defaults;
- `defineStack` refusing a declared `$total` with `{ code:
'STACK_SCHEMA_INVALID', status: 422 }` at `flows.0.variables.0.name`;
- the remedy reading clean (`{{ row.name }}` in a loop body);
- `flowAssignmentTargets` over the three shapes, and an assignment
inside a region body;
- the vocabulary pin, the discovery pin and the published-pattern pin;
- the D3 entry and its rationale fragment.
- **`@objectstack/spec`**, `vitest run --project local --maxWorkers=2`:
**642 files, 19286 passed, 1 todo**. `pnpm --filter @objectstack/spec
typecheck` exits 0, with `check:test-typecheck: OK`.
- **`@objectstack/service-automation`**, the whole suite against the
rebuilt spec: **185 files, 2368 passed**. Its typecheck exits 0.
- **`@objectstack/lint`**, the whole suite: **135 files, 6321 passed**.
Its typecheck exits 0.
- **The CLI guidance test**, `vitest run --project integration
test/migrate-meta-engine-guidance.test.ts`, after the CLI closure build
(`turbo run build --filter=@objectstack/cli^...`): **3 passed**. It
holds the new entry's printed guidance.
- **`pnpm --filter @objectstack/spec check:generated`**: all 15
generated artifacts are up to date, after a rebuild at the final source.
## Ablation
Each ablation ran against the committed fix, with the mutation and
restore going through `scripts/ablation-replace.mjs` in wrap mode. The
spec tests import `src`, so no dist leg applies.
1. **The rule dropped from `MapConfigSchema.indexVariable`.** The anchor
went from 1 to 0, and the blob from `cbe18e1969b2` to `ca9af2dd1eb3`.
- Result: **4 failed, 95 passed**. Red: the map `indexVariable`
contract, flow-door and engine-name pins, and the discovery pin with
`MapConfigSchema.indexVariable publishes no pattern`.
- Restore: the blob equals HEAD (`cbe18e1969b2`), and `git diff HEAD` is
empty.
2. **The leading-blank half dropped** (`[^$\s]` back to `[^$]`). The
blob went from `b7564636d07e` to `ae5b2c3c1277`.
- Result: **18 failed, 97 passed**: all 16 leading-blank rows, plus both
published-pattern pins.
- Restore: the blob equals HEAD (`b7564636d07e`), and `git diff HEAD` is
empty.
## Gates (at `af752544fb`)
- **The derived gates.** `node scripts/pm/dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack` (no paths, merge base `f59a73c39`)
derived 114 commands. 112 were run, all exit 0. `check:skill-examples`
first exited 3 (PREREQUISITE NOT MET: `client-react` was not built).
After building `client` and `client-react`, its re-run read `262 prose
examples type-check`.
- **The reconciliation.** `--ran` reads `114 derived, 112 run, 0
NOT-MEASURED, 2 UNRUN`.
- **NOT MEASURED: `check:dual-build-cjs-loads`.** Reason: it needs a
whole-workspace build, which the dispatch rules out.
- **NOT MEASURED: `check:type-check-debt`.** Reason: its script is
`--re-measure`, which the dispatch rules out.
- **NOT MEASURED: the workspace typecheck lane.** Reason: it needs a
whole-workspace build. The type surface is byte-unchanged
(`check:api-surface` is green), because a `regex` check changes no
TypeScript type.
- **The artifact-roster block.** It is run separately. Its three
PR-context guards are wired to this PR after it opened, and their
readings are in the report on #22572.
## Acceptance notes
- **The two strays triage named** stay noted: the `service-automation`
executor `configSchema` descriptors, and the `engine.ts`
`buildSubflowResumeSignal` comment. This PR touches no
`service-automation` file.
- **A body-less legacy `loop`.** Its values are not judged at the flow
door. This is the contract map's `parsedWhen`, unchanged since #21982.
Its `iteratorVariable` is never read; the executor sets `$loopItems` and
`$loopIndex` itself.
- **`map.input` and `subflow.input` keys** name the callee's variables,
so they are not bindings in this flow. A callee can no longer declare a
`$` variable, so a `$` key there binds nothing.
- **`api/automation-api.zod.ts` `ScreenSpec.idVariable`** is a response
shape the engine produces, not an authoring door. It carries no rule.
- **`content/docs/automation/flows.mdx`** (hand-written) still names
only `outputVariable` beside `errorVariable` in its paragraph on where
the `$` names belong. The paragraph is still true, and it is outside the
claim's surface. It is not widened here.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF)_
---------
Co-authored-by: Claude <noreply@anthropic.com>$ name at every remaining flow binding — loop / map iterator and index, screen idVariable and field name, declared variables, assignment targets (#22746)1 parent c74d843 commit 9f5eca5
13 files changed
Lines changed: 706 additions & 69 deletions
File tree
- .changeset
- content/docs/references
- api
- automation
- packages/spec
- src
- automation
- migrations
- entries/semantic
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
135 | 135 | | |
136 | 136 | | |
137 | 137 | | |
138 | | - | |
| 138 | + | |
139 | 139 | | |
140 | 140 | | |
141 | 141 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
208 | 208 | | |
209 | 209 | | |
210 | 210 | | |
211 | | - | |
212 | | - | |
| 211 | + | |
| 212 | + | |
213 | 213 | | |
214 | 214 | | |
215 | 215 | | |
| |||
228 | 228 | | |
229 | 229 | | |
230 | 230 | | |
231 | | - | |
| 231 | + | |
232 | 232 | | |
233 | 233 | | |
234 | 234 | | |
| |||
237 | 237 | | |
238 | 238 | | |
239 | 239 | | |
240 | | - | |
| 240 | + | |
241 | 241 | | |
242 | 242 | | |
243 | 243 | | |
| |||
259 | 259 | | |
260 | 260 | | |
261 | 261 | | |
262 | | - | |
| 262 | + | |
263 | 263 | | |
264 | 264 | | |
265 | 265 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
145 | 145 | | |
146 | 146 | | |
147 | 147 | | |
148 | | - | |
149 | | - | |
| 148 | + | |
| 149 | + | |
150 | 150 | | |
151 | 151 | | |
152 | 152 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
59 | | - | |
| 59 | + | |
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
| |||
223 | 223 | | |
224 | 224 | | |
225 | 225 | | |
226 | | - | |
| 226 | + | |
227 | 227 | | |
228 | 228 | | |
229 | 229 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| |||
462 | 462 | | |
463 | 463 | | |
464 | 464 | | |
| 465 | + | |
465 | 466 | | |
466 | 467 | | |
467 | 468 | | |
| |||
0 commit comments