Skip to content

Commit 0f77ff5

Browse files
fix(lint)!: flow CEL record is bound only where an entrance hands the flow a record (#22677) (#22730)
Fixes #22677 Clause-②: no (narrowing) ## What `record` leaves the build door's always-bound roots (`ENGINE_BOUND_ROOTS` in `packages/lint/src/flow-cel-root-scope.ts`). Since PR #22674 (#22642) the run binds `record` only to the record its entrance handed it, or to a flow's own `record` variable, so `objectstack validate` passed a `record.X` that faults at run time with `Unknown variable: record`. Now: - `flowCelEntrances` reads a second set beside "opened": the flows an entrance hands ANY record. `record` goes into those flows' bound set. - A flow that declares a `record` variable (or assigns one) binds it through `collectFlowVariableNames`, as any other name. - Everywhere else `record.X` is refused as `expression-invalid`, `error`, with a `record`-specific message that names the remedy: read the variable by its name or through `vars` (`record.status` becomes `status` / `vars.status`), or give the flow the entrance that hands it its record. - `previous`, `vars` and `current_user` stay engine roots. The generic refusal's closing clause now says so, and adds that `record` is bound when an entrance hands the run one. - The runtime publish gate's stand-down (`perWriteSnapshotEntrance`) is unchanged and covers `record` too, until #22636. `validate-expressions.ts` is unchanged. ## The two sets, measured on the engine at `f66fdc7973` | entrance | hands a record (`record` bound) | opens the flow (stands down) | |:--|:--|:--| | record trigger: kind `record_change` (start `config.triggerType` is a `record-*` string) | yes, the row | when start `config.objectName` is undeclared | | time-relative sweep: kind `time_relative` | yes, each swept row | when `config.timeRelative.object` is undeclared | | inbound hook: kind `api` | yes, the request body | always | | `type: 'flow'` action targeting the flow | yes, on any object or none (an object-less action hands an empty record, still a record) | when the action's object is undeclared | | `map` node naming the flow | yes, when `config.itemObject` is set | when `itemObject` is undeclared or absent | | `subflow` / `map` parent | yes, when the parent is handed one (fixpoint) | when the parent is open (fixpoint) | | start `config.objectName` alone, a REST trigger route, a declared endpoint, a cron schedule | no | (unchanged) | Sources: - `seedRunVariables` binds `context.record` as `record` and nothing else does. - `deriveTriggerBinding` routes `record_change` only for a `record-*` `triggerType`. - `subflow-node.ts` / `map-node.ts` spread the parent's context. - `dispatchFlowAction` always passes a record. A parent's own `record` VARIABLE is not handed on, because a child gets its parent's context, not its variables. That case is pinned. `previous` has no such problem on main: `seedRunVariables` sets it on every run (`context?.previous ?? null`), so it is never unbound. It stays in `ENGINE_BOUND_ROOTS`. ## Corrected pending release notes: the DELIBERATE CORRECTION class `check-empty-changeset` is red **by design**, naming exactly two pending, unreleased notes that this PR changes the behaviour of: - `.changeset/22565-flow-cel-unbound-root-refused.md` (#22565); - `.changeset/22642-flow-cel-record-binding.md` (#22642). Each is corrected in place, and neither may be restored from the base. ### `.changeset/22565-flow-cel-unbound-root-refused.md`: three `record` sentences, verbatim 1. **Why** paragraph - FROM: "the trigger record's fields flattened beside them, and `record`, `previous`, `vars` and `current_user` bound by the engine." - TO: "the trigger record's fields flattened beside them, `record` bound to the record the run was handed (where an entrance hands it one), and `previous`, `vars` and `current_user` bound by the engine." 2. **What is refused**, first bullet - FROM: "- the engine's roots: `record`, `previous`, `vars`, `current_user`;" - TO: "- the engine's roots: `previous`, `vars`, `current_user`;" - followed by a new bullet: "- `record`, where an entrance the stack declares hands the flow a record — a record trigger (a `record-*` `triggerType`), a time-relative sweep, the inbound hook, a `type: 'flow'` action, a `map` node with an `itemObject`, or a `subflow` / `map` parent handed one — or where the flow binds a variable named `record`;" 3. **Who is affected** - FROM: "every root those flows read is an engine root, a binding of the flow's own or a field of its trigger object." - TO: "every root those flows read is an engine root, `record` on a flow an entrance hands a record, a binding of the flow's own or a field of its trigger object." ### `.changeset/22642-flow-cel-record-binding.md`: one sentence, verbatim, nothing else in the file (seat order `6102049574`) - FROM: "`objectstack validate` does not refuse such a `record` read yet; the run fails it, naming the root and the source." - TO: "`objectstack validate` refuses such a `record` read too, naming the root (see `@objectstack/lint`'s note)." ### The D3 entry The landed D3 entry `flow-cel-unbound-root-refused` gets a text-only change to `reason`, and nothing else in the entry changes: - FROM: "the trigger record's fields flattened beside them, and record, previous, vars and current_user bound by the engine." - TO: "the trigger record's fields flattened beside them, record bound to the record the run was handed (where an entrance the stack declares hands the flow one, or the flow binds a variable named record), and previous, vars and current_user bound by the engine." `registry.ts` is still tracked, and it was regenerated with `gen:migration-registry` (a 4-line diff in the same region). ## New changeset and ADR-0087 disposition `.changeset/22677-flow-cel-record-entrance.md`: - `@objectstack/lint: major`. That is the pre-mode level #22565's note took for the same rule's narrowing; `check-changeset-no-major` exempts it in pre mode. - `Clause-②: no (narrowing)`. - Marker `not-required (already-registered flow-cel-unbound-root-refused, flow-cel-record-variables-alias-retired)`. No new ledger entry is owed: - a `record` read on a flow no entrance hands a record is a root the flow does not bind, which is exactly the surface of the unreleased D3 entry `flow-cel-unbound-root-refused`, and its corrected `reason` now says so; - the runtime half is #22642's own D3 entry; - both exist at the merge base, and neither has shipped. The first version of this note ended with a line saying the #22642 note's sentence no longer held. That line was dropped at `0e51b65d09`, once the #22642 sentence itself was corrected. ## Fixture census, taken before any fixture was edited These are the fixtures whose flow reads `record.X` with no record entrance. Each is a start node with `config.objectName` and no `record-*` trigger, except the `lint-flow-patterns` one, which has neither. Lint suite with only the judge change: 34 failed in 3 files. - `packages/lint/src/validate-expressions.test.ts`: 29 tests, 9 fixtures. The start node at each line below got `triggerType: 'record-after-update'` (text only): - `:47` passes a clean stack; - `:81` unknown record field; - `:478` dotted spelling; - `:2760` the `flowWith` helper for the region tests; - `:3125` null-guard flow conditions. Its #4811 comment "the engine binds both roots unconditionally" is corrected to "a record-triggered run binds both roots"; - `:4031` `withCondition`, 15 tests of #5378; - `:4456` `flowWith` of #15662, 4 tests; - `:4623` `flowWith` of #17495, 2 tests; - `:4754` `stackWith` of #15742, 2 tests. - `packages/lint/src/lint-flow-patterns.test.ts:2787`: `valueFlow`, 2 tests. It prescribes `record.title` on an autolaunched flow with no entrance. It became a `record_change` flow on a declared `task` object, so the judgment stays live rather than standing down. - `packages/lint/src/validate-expressions.flow-cel-root.test.ts`: 3 of #22565's own pins: - the "engine roots" control dropped `record` (a new control reads it on a record-triggered flow); - the CEL-type-name control reads `type(vars)`; - the generic message pin follows the corrected closing clause. - `@objectstack/metadata-protocol` (226 files, built closure, lint `dist` carrying the new judge): 0. Its flows go through the runtime gate, which stands down. - CLI files that run flows through `validate`: 0. That is 5 `unit` files and 3 `integration` files that spawn the source CLI. ## Reach with the real judge `validateStackExpressions` and `flowCelEntrances` from this worktree. Positive control: a no-entrance flow that reads `record.status` is counted, with 1 refusal. | scope | flows | read `record` | newly refused | |:--|:--|:--|:--| | `examples/**` (`app-crm`, `app-todo`, `app-multi-package`, `app-showcase`) | 35 | 14 (12 `record_change`, 1 `time_relative`, 1 `api`) | 0 | | `packages/platform-objects` (source scan) | 0 | 0 | 0 | | `objectstack-ai/hotcrm` at `1d7148b` (read-only sparse clone, deleted after) | 32 | 14 (all `record_change`) | 0 | No shipped example refuses, and no example was changed. ## Pins and ablations The new block in `validate-expressions.flow-cel-root.test.ts` pins: - `validate` refuses `record.X` with no entrance (`where`, the message head, and the `vars` remedy); - a start `objectName` alone is refused; - control: a record-triggered flow passes; - control: a `subflow` child of a record-handing parent passes, including two levels; - red control: a `subflow` child of a non-handing parent is refused; - a parent's `record` variable is not handed on; - controls for a time-relative sweep, an action on a declared object, an object-less action, a `map` with `itemObject`, and a declared `record` variable; - the reader's `bound` per entrance; - the runtime publish gate does not refuse `record.X` on a flow write while `validate` refuses it on the same flow. Ablations ran through `scripts/ablation-replace.mjs` at `641f940df7`, with the fix committed. Each one restored blob-equal to HEAD (`1f19fd39e079`), with `git diff HEAD` empty. - `'record'` put back in `ENGINE_BOUND_ROOTS`: 5 failed / 72 passed. All 5 are the refusal-side pins (the refusal, `objectName` alone, the red subflow control, the parent's variable, the runtime gate pin). - The record-bound read dropped (`names` without `handed`): 8 failed / 69 passed. These are the record-trigger and `subflow` controls, plus the time-relative, both action and `map` controls, the reader pin, and the record-trigger row of the binding-kind controls. The declared-variable control stays green, as it should. ## Verification Run at `8a65474075` (the merge of `origin/main` `762db996ad` through `os-regen-merge.sh`) unless stated. - `@objectstack/lint` suite: 135 files, 6318 passed; `typecheck` exit 0 (`tsc --noEmit` and `check:test-typecheck` OK). Lint and its typecheck were also green at `641f940df7`. - `@objectstack/spec`: - `check:migration-registry`: registry current; - `check:generated`: all 15 artifacts current, after a `spec` build. The first run, against a stale `dist`, said PREREQUISITE NOT MET, so it does not count. - `@objectstack/metadata-protocol` suite, with its closure built: 223 files passed and 3 skipped, 28101 tests passed. CLI flow-validate files: `--project unit`, 5 files, 78 passed; `--project integration` (they spawn the source CLI), 3 files, 15 passed. Both were measured on the pre-merge tree. The judge source is byte-identical at `8a65474075`, and the merge touched neither package's flow path. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 91 commands at `8a65474075`, and 89 ran: - 88 exit 0; - `check-empty-changeset` exit 1 — the DELIBERATE CORRECTION above, red by design. - `--ran` reconcile: 91 derived, 89 run, 2 unrun. Those 2 are NOT MEASURED by order: - `check:dual-build-cjs-loads` needs a whole-workspace build; - `check:type-check-debt` is `--re-measure`. - Artifact rosters: 46 ran without PR context: - 45 exit 0; - `check:published-readme-exports` exit 3 PREREQUISITE NOT MET: 7 unbuilt packages, none touched here. NOT MEASURED. - The 3 PR-context guards were run against this PR at `8a65474075`: `check-closing-target-claim`, `check-partof-closing-keyword` and `check-single-claim-paths` each exit 0. Patch round at `0e51b65d09`, a text-only change to the two `.changeset/` files named by `git diff --name-only 8a65474 0e51b65`. No code changed, so no suite was re-run: - `node scripts/check-empty-changeset.mjs --base origin/main`: exit 1, naming exactly `.changeset/22565-flow-cel-unbound-root-refused.md` and `.changeset/22642-flow-cel-record-binding.md`. This is the DELIBERATE CORRECTION class, red by design; - `node scripts/check-changeset-no-major.mjs --base origin/main`: exit 0 (the `major` is exempt in pre mode); - `node scripts/check-changeset-fixed.mjs`: exit 0; - `pnpm check:nul-bytes`: exit 0. ## Acceptance notes 1. **The `opened`-set gap.** A `record-*` trigger with no start `config.objectName` makes the record-change trigger register its hook with no object, so the hook fires on any object's write. Such a run can be handed a record whose keys are not in hand, yet `flowCelEntrances` opens a flow only when `objectName` is named and undeclared. This is a read-only inference with no public-door reproduction. Carrier: #22636, the same judge's next card. 2. **A declared `record` variable** binds `record` at the build door, as every declared variable does (`collectFlowVariableNames`; #22565's judge-wide design). At run time a declared variable is bound only with a supplied input or a `defaultValue` (`seedDeclaredVariables`). 3. **The clause "none of these roots ever evaluated in a flow"** in the #22565 note, the D3 `reason` and the registry's step text is left as it is (seat order `6102049574`, question 2 answered A). #22642's own D3 entry, `flow-cel-record-variables-alias-retired`, carries `record`'s behaviour change with its FROM → TO. --- _Generated by [Claude Code](https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a360cee commit 0f77ff5

9 files changed

Lines changed: 268 additions & 48 deletions

‎.changeset/22565-flow-cel-unbound-root-refused.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@ Clause-②: no (narrowing)
1111

1212
**BREAKING**: an accept-set narrowing on a published authoring surface (`objectstack validate`'s `expression-invalid` rule).
1313

14-
**Why.** Flow CEL evaluates in one scope per run: the flow's variables spread to top level, the trigger record's fields flattened beside them, and `record`, `previous`, `vars` and `current_user` bound by the engine. Formulas, row-level security and the client also bind the run's user as `user`, `ctx.user` and `os.user`; flow CEL does not, because a top-level `user` would collide with a variable or a lookup field of that name. So an author who wrote the alias by habit shipped a flow that faulted at its first evaluation, and nothing warned.
14+
**Why.** Flow CEL evaluates in one scope per run: the flow's variables spread to top level, the trigger record's fields flattened beside them, `record` bound to the record the run was handed (where an entrance hands it one), and `previous`, `vars` and `current_user` bound by the engine. Formulas, row-level security and the client also bind the run's user as `user`, `ctx.user` and `os.user`; flow CEL does not, because a top-level `user` would collide with a variable or a lookup field of that name. So an author who wrote the alias by habit shipped a flow that faulted at its first evaluation, and nothing warned.
1515

1616
**What is refused.** At every flow CEL site the build door reads — a node's `config.condition` (the start node's trigger gate included), an edge `condition`, a `decision` branch `expression`, a screen field `visibleWhen`, and the CEL value envelopes of an `assignment` and of a `create_record` / `update_record` `fields` map — a root that is none of:
1717

18-
- the engine's roots: `record`, `previous`, `vars`, `current_user`;
18+
- the engine's roots: `previous`, `vars`, `current_user`;
19+
- `record`, where an entrance the stack declares hands the flow a record — a record trigger (a `record-*` `triggerType`), a time-relative sweep, the inbound hook, a `type: 'flow'` action, a `map` node with an `itemObject`, or a `subflow` / `map` parent handed one — or where the flow binds a variable named `record`;
1920
- a name the flow binds: a declared variable, an `outputVariable`, an `iteratorVariable` (a `loop`'s default `item` included), an `indexVariable`, an `errorVariable`, an `assignment` target, a node id, a screen field's `name`;
2021
- a field (or a registry-injected column) of any object the stack declares — the run flattens whichever record its entrance hands it (an object-less flow action hands only the row `id`).
2122

@@ -45,7 +46,7 @@ A `$` name has no CEL spelling (`$runId == "x"` does not parse), so the `$` root
4546

4647
**The one-line fix: write `current_user` for the run user, and bind or correct every other root the refusal names.**
4748

48-
**Who is affected, measured.** The last published spec line, 17.x, judged a flow expression's syntax only, so it accepts every refused source. Measured with the judge over every flow CEL site: the example apps (`app-crm`, `app-todo`, `app-multi-package`, `app-showcase`: 35 flows, 45 sites), `packages/platform-objects` (no flows) and `objectstack-ai/hotcrm` at `f0afcbda07` (32 flows, 65 sites) — 0 refusals; every root those flows read is an engine root, a binding of the flow's own or a field of its trigger object. One flow of the 67 is left unjudged by an entrance (`showcase_inbound_task_webhook`, an `api` trigger). Deployed metadata and other repositories were not measured.
49+
**Who is affected, measured.** The last published spec line, 17.x, judged a flow expression's syntax only, so it accepts every refused source. Measured with the judge over every flow CEL site: the example apps (`app-crm`, `app-todo`, `app-multi-package`, `app-showcase`: 35 flows, 45 sites), `packages/platform-objects` (no flows) and `objectstack-ai/hotcrm` at `f0afcbda07` (32 flows, 65 sites) — 0 refusals; every root those flows read is an engine root, `record` on a flow an entrance hands a record, a binding of the flow's own or a field of its trigger object. One flow of the 67 is left unjudged by an entrance (`showcase_inbound_task_webhook`, an `api` trigger). Deployed metadata and other repositories were not measured.
4950

5051
### The kit
5152

‎.changeset/22642-flow-cel-record-binding.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,4 @@ In a flow whose run holds no record, read a variable by its name:
3535
- this repository at `0ec4268972` (`examples/**`, `packages/platform-objects`, `packages/qa/dogfood`): 64 flows, 51 CEL slots, no flow in `packages/platform-objects`; the 2 flows that read `record` are an `api` hook flow and a record-change flow;
3636
- `objectstack-ai/hotcrm` at `f0afcbda07` (`src/`, `test/`): 45 flows, 57 CEL slots; the 12 flows that read `record` are all record-change flows.
3737

38-
Deployed metadata and other repositories were not measured. `objectstack validate` does not refuse such a `record` read yet; the run fails it, naming the root and the source.
38+
Deployed metadata and other repositories were not measured. `objectstack validate` refuses such a `record` read too, naming the root (see `@objectstack/lint`'s note).
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
'@objectstack/lint': major
3+
---
4+
5+
A flow CEL expression that reads `record` on a flow no entrance hands a record is refused at `objectstack validate`. Since #22642 the run binds `record` only to the record it was handed, so such a read fails the run with `Unknown variable: record`, and the build door still counted `record` as bound on every run and passed it.
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (already-registered flow-cel-unbound-root-refused, flow-cel-record-variables-alias-retired) A record read on a flow no entrance hands a record is a root the flow does not bind, which is the surface of the D3 entry flow-cel-unbound-root-refused (protocol 18, not yet released); this diff corrects that entry's reason to say so. The runtime half is the D3 entry flow-cel-record-variables-alias-retired (#22642). Both exist at the merge base and neither has shipped, so this narrowing adds no entry. -->
10+
11+
**BREAKING**: an accept-set narrowing on a published authoring surface (`objectstack validate`'s `expression-invalid` rule).
12+
13+
**What is refused.** `record` is no longer one of the roots the build door binds on every run (`previous`, `vars` and `current_user` still are). It is bound where:
14+
15+
- an entrance the stack declares hands the flow a record: a record trigger (start `config.triggerType` is a `record-*` token), a time-relative sweep (start `config.timeRelative`), the inbound hook (trigger kind `api`), a `type: 'flow'` action that targets the flow (on any object, or on none), a `map` node with a `config.itemObject` that names the flow, or a `subflow` / `map` parent that is itself handed a record;
16+
- or the flow binds a variable named `record` itself (a declared variable or an `assignment` target).
17+
18+
A start `config.objectName` alone is no entrance: with no `record-*` trigger nothing hands the run a row. A parent's own `record` variable is not handed on either: a child gets its parent's context, not its variables. Everywhere else `record.X` is refused as `expression-invalid`, `error`, with the remedy below.
19+
20+
**Not judged.** The stand-downs of the unbound-root rule are unchanged. At the runtime publish gate this judgment still stands down: its per-write snapshot carries no actions and no other flows, so it cannot see an entrance, and a flow write there is not refused for `record` until that snapshot carries them.
21+
22+
## FROM → TO
23+
24+
| you wrote | write instead |
25+
|:--|:--|
26+
| `record.assignee` on a flow no entrance hands a record, meaning a flow variable `assignee` | `assignee`, or `vars.assignee` |
27+
| `record.status == "open"` on a flow meant to run when a row is written | the trigger that hands it the row: start `config: { objectName: 'acct', triggerType: 'record-after-update' }` |
28+
29+
**The one-line fix: read a flow variable by its name or through `vars`, and read `record` only on a flow an entrance hands one.**
30+
31+
**Who is affected, measured.** With this judge over every flow CEL site: the example apps (`app-crm`, `app-todo`, `app-multi-package`, `app-showcase`: 35 flows, 14 that read `record`), `packages/platform-objects` (no flows) and `objectstack-ai/hotcrm` at `1d7148b` (32 flows, 14 that read `record`): 0 newly refused. Every flow that reads `record` there is a record-change, time-relative or inbound-hook flow. Deployed metadata and other repositories were not measured.

0 commit comments

Comments
 (0)