Skip to content

Commit 5fb1746

Browse files
feat(lint)!: refuse a flow CEL root the flow does not bind, naming current_user for the run-user aliases (#22609)
Fixes #22565 Clause-②: no (narrowing) ## What this does `objectstack validate` now refuses a flow CEL root the flow does not bind. - Triage direction B (`6092499200`), with its two follow-up answers: **A′** (`6094431558`), every entrance the stack declares is read; **S** (`6094850599`), the runtime publish gate stands down for this judgment. - ⛔ Not A: `service-automation`'s `celScope` is untouched. - `user.id == "u1"`, `ctx.user.id == "u1"` and `os.user.id == "u1"` are refused with a remedy naming `current_user`, plus its guard for a user-less run. - The wider family rides, as triage allowed: the same judge refuses any root nothing binds (`foo.bar == 1`) at no extra cost. Its remedy names the root, what could have bound it, and the nearest in-scope name. - One judge covers every flow CEL site `validateStackExpressions` visits: - a node `config.condition` (the start trigger gate included) and an edge `condition`; - a `decision` branch `expression` and a screen field `visibleWhen`; - the CEL value envelopes of `assignment` and of `create_record` / `update_record` `fields`. ## Premise, measured at `99801d831f` - **The door:** `validateStackExpressions` answered `[]` for an edge `user.id == "u1"`, `ctx.user.id`, `os.user.id` and `foo.bar == 1`. - **The run:** these were evaluated through a verbatim copy of `celScope` over `ExpressionEngine`. - `user`, `ctx`, `os`, `foo`, `input`, `params` and `data` each fault `Unknown variable: X`. - `current_user`, `record`, `previous` and `vars` evaluate, and a comprehension or `cel.bind` variable binds. - `$runId` does not parse. ## The bound set (one per flow) - **Engine roots:** `record`, `previous`, `vars`, `current_user`. - A `$` name has no CEL spelling, so the `$` roots stay the text-slot rule's, and no second list of engine `$` names exists here. - **The flow's own bindings:** `collectFlowVariableNames`, composed. It covers declared variables, `outputVariable` / `errorVariable` / `iteratorVariable` / `indexVariable`, assignment targets and node ids. Two more are added: screen field names, and a `loop`'s default `item`. - **Record fields:** every field, injected columns included, of every object the stack declares. The run flattens whichever record its entrance hands it. - An object-less flow action hands only the row `id`, so `id` is bound for a flow one launches. ## Where the door stands down (`provable: false`, with an `openedBy` that names why) - **A node whose run can bind a name the reader cannot see:** - `script` and `connector_action` hand their code the live variable map; - `wait`, `subflow` and `map` take a resume with `resumeAuthority: 'any'`, which folds the caller's bag in; - a screen with no field list; - any type outside the measured closed set. - **An entrance the stack declares that hands the flow a record whose keys are not in hand.** `flowCelEntrances` reads each entrance once per stack, from the stack's own declarations: - **a record trigger:** the start `config.objectName` names an undeclared object; - **a time-relative sweep:** the start `config.timeRelative.object` (`TimeRelativeTriggerSchema`, its own key) names an undeclared object; - **the inbound hook:** the trigger kind is `api` (`resolveFlowTriggerKind`: `type: 'api'` or `config.triggerType: 'api'`). The hook hands the request body in as the record (`trigger-api`); - **an action:** an `ActionSchema` entry with `type: 'flow'` and `target` set to this flow, in `actions[]` (its `objectName`) or in `objects[].actions[]` (the owning object), on an undeclared object; - **a `map` node:** `MapConfigSchema`'s `flowName` names this flow, and its `itemObject` is undeclared or absent; - **a parent that is itself open:** a `subflow` node (`SubflowConfigSchema`'s `flowName`) or a `map` node names this flow inside an open flow. This is read to a fixpoint. - **The API run door needs no stand-down (measured):** - Both REST trigger routes and the declarative endpoint build their context with `buildAutomationContext`: `params` and `recordId`, no `record`. - The engine never loads a record from `object` + `recordId`, and a schedule run hands no record. - **The runtime publish gate (S):** on a flow write, this one judgment stands down. Every other expression verdict there is unchanged, and a pin shows one still refusing there. - The gate's per-write snapshot (`buildRuntimeWriteSnapshotSet`) carries the written flow alone, beside `objects` / `permissions` / `books` / `datasets`, so no entrance is visible. - The `openedBy` reason names that snapshot. - #22636 carries the widening, a snapshot that carries flows and actions. When it lands, this stand-down goes; nothing of it rides this PR. ## Reach (the real judge at `e99141ce2a`) | corpus | flows | refusals | opened by an entrance | |:--|--:|--:|:--| | `examples/**` (`app-crm`, `app-todo`, `app-multi-package`, `app-showcase`) | 35 | 0 | 1: `showcase_inbound_task_webhook` (the inbound hook, `type: 'api'`) | | `packages/platform-objects` | 0 | 0 | — (declares no flows) | | `objectstack-ai/hotcrm` at `f0afcbda07` (read-only clone, deleted after) | 32 | 0 | 0 | - No flow is opened by an action, a `map` / `subflow` parent, or a trigger / sweep object. - Every flow-launching action targets a declared object: app-crm 2, app-showcase 1, hotcrm 14. - The opened flow reads only `record.*` today; its webhook body keys are arbitrary, so standing down is right. ## Pins (`packages/lint/src/validate-expressions.flow-cel-root.test.ts`) - **The triage pins, unchanged:** - edge `user.id == "u1"` is refused with the `current_user` remedy; - the same condition passes on a flow declaring a variable `user`; - the remedy `current_user.id == "u1"` passes. - **Each alias spelling, each CEL site, and one control per binding kind**, as before. - **One pin per entrance**, each pairing the undeclared case (stands down) with a declared control (still judged): record trigger, time-relative sweep, inbound hook (by `type` and by `triggerType`), action (a stack action and an object's own action; a non-flow action is no entrance), object-less action (`id` bound), `map` (undeclared and absent `itemObject`), and an open-parent `subflow` with a two-level fixpoint. - **The runtime stand-down**, through `runRuntimeAuthoringRules` on a flow write: - no unbound-root finding there, while the brace-trap verdict on another edge still refuses there; - the build door refuses the same root on the same flow. ## Ablations, from committed state `e99141ce2a` Each ran through `scripts/ablation-replace.mjs`, with the anchor hit once and the restore blob-equal to HEAD with `git diff HEAD` empty: 1. **The judge removed** (`const unbound = unboundFlowCelRoots(source, rootScope);` → an empty list): the `user.id` triage pin goes red, `1 failed | 2 passed`, and both controls stay green. 2. **The entrance reader removed** (`flowCelEntrances(...)` → an empty map): all 7 entrance pins go red, `7 failed | 1 passed`. The one pass calls the reader directly. 3. **The runtime stand-down removed** (`runtimeGateWrite` → `false`): the gate pin goes red, `1 failed | 2 passed`. ## Tests (through `scripts/pm/os-verify-lock.sh`, at `e99141ce2a`) - `@objectstack/lint` (closure rebuilt): - `vitest run`: 135 files, 6186 tests passed; - `typecheck` (`tsc --noEmit` plus the test-layer program, whose `src/**/*` includes the pin file): OK. - `@objectstack/metadata-protocol` (the runtime publish door), against the rebuilt lint `dist`: 224 files, 28314 tests passed (3 files and 19 tests skipped). - `@objectstack/cli`, built: the 10 files that carry flows through the authoring doors, 128 tests in all. - 8 in the unit / integration projects (107 tests); - the 2 nightly-tier e2e files under `OS_TEST_TIERS=nightly` (21 tests). - `service-automation`: NOT MEASURED, reason: it does not depend on `@objectstack/lint`. ## Gates (at `e99141ce2a`) - `dispatch-gates --commands` (no paths) derived 91 families for the final 8-path set: 90 run, all exit 0; 1 NOT MEASURED. - `check:dual-build-cjs-loads` is the NOT MEASURED one, reason: no whole-workspace build in this container. - `--ran` with the coded record: "91 derived famil(ies) accounted for — 90 run, 1 NOT-MEASURED", 0 UNRUN. - **Artifact rosters (49):** 46 run locally. The 3 PR-context guards were wired to this PR after this body landed; the report carries all the exit codes. ## Deviations (the seat accepted these in `6094065925`) 1. **Record fields cover every stack object's, not only the trigger object's.** The run flattens whichever record its entrance hands it. A′ now reads those entrances, so the widening is paired with the entrance stand-downs above. 2. **`packages/lint/src/lint-flow-patterns.test.ts` (#19939 pass 3's surface) gains 5 lines in its `valueFlow` fixture**, declaring `source` and `me`. Two pins asserted a clean build-door verdict for a fixture that binds neither name. The hunk is disjoint from pass 3's. 3. **`docs/protocol-upgrade-guide.md` and `spec-changes.json` are not regenerated.** Per both generators' headers, the PR stage compares no committed copy. 4. **The four non-`$` engine roots are a local constant**, because the spec's `FLOW_SCOPE_CLAIMED_IDENTIFIERS` is package-internal. ## Acceptance notes - **Projection drift.** The committed projections lag the registry until #22485. Noted, with no carrier. - **Drift hazard.** The closed node-type set and the engine roots are hand-measured. A parity pin in `service-automation` would catch an executor that starts binding a new name. Noted, with no carrier. - **Severity.** On a judged flow, a flow-variable near-miss of a trigger field now reads as an `error`. On an open flow the advisory is unchanged and still pinned. ## Kit - `packages/lint/src/flow-cel-root-scope.ts`: the judge, the entrance reader `flowCelEntrances`, and `perWriteSnapshotEntrance`. - `packages/lint/src/validate-expressions.ts`: the calls at each flow CEL site, the entrance index read once per stack, and the runtime-gate stand-down. - Fixtures that now declare the bindings they read: `validate-expressions.test.ts` and `lint-flow-patterns.test.ts` (one builder). - The ADR-0087 D3 semantic entry `flow-cel-unbound-root-refused` (step 18) and its `STEP18_RATIONALE` fragment. Both now name the entrances and the runtime gate's stand-down. `registry.ts` is regenerated. - `.changeset/22565-flow-cel-unbound-root-refused.md`: `@objectstack/lint` and `@objectstack/spec` at `major` (pre mode), with the `registered` ADR-0087 marker, FROM → TO and the measured reach. --- _Generated by [Claude Code](https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0ec4268 commit 5fb1746

8 files changed

Lines changed: 1276 additions & 6 deletions
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
'@objectstack/lint': major
3+
'@objectstack/spec': major
4+
---
5+
6+
A flow CEL expression that reads a root the flow does not bind is refused at `objectstack validate`. The run fails such an expression with `Unknown variable: X` (X being the root) on every evaluation, and the build door used to pass it. The reachable case is the run user: `user.id == "u1"`, `ctx.user.id == "u1"` and `os.user.id == "u1"` — the spellings formulas, row-level security and the client accept — are refused, naming `current_user`, the one spelling flow CEL binds.
7+
8+
Clause-②: no (narrowing)
9+
10+
<!-- adr-0087: registered flow-cel-unbound-root-refused -->
11+
12+
**BREAKING**: an accept-set narrowing on a published authoring surface (`objectstack validate`'s `expression-invalid` rule).
13+
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.
15+
16+
**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:
17+
18+
- the engine's roots: `record`, `previous`, `vars`, `current_user`;
19+
- 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`;
20+
- 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`).
21+
22+
The user spellings are told to write `current_user`, with the guard for a flow that can run without a user; any other root is named with what could have bound it, and the nearest in-scope name. The error is `expression-invalid`, `error`. Where the flattened-scope near-miss advisory ("`X` is not a field of … If `X` is a flow variable this is safe to ignore") named the same root, the refusal replaces it.
23+
24+
**Not judged — the door stands down where the run can bind a name it cannot see.**
25+
26+
- **A node:** a `script` or `connector_action` node (each hands its code the live variable map), a `wait`, `subflow` or `map` node (a resume folds the caller's `variables` bag in under their plain names), a screen with no field list, or a node type outside the builtin set.
27+
- **An entrance the stack declares that hands the flow a record whose keys are not in hand:**
28+
- a record trigger (`config.objectName`) or a time-relative sweep (`config.timeRelative.object`) on an object the stack does not declare;
29+
- an `api` trigger, whose inbound hook hands the request body in as the record;
30+
- a `type: 'flow'` action launching the flow on an undeclared object;
31+
- a `map` node feeding the flow whose `itemObject` is undeclared or absent;
32+
- a `subflow` or `map` parent that is itself such a flow.
33+
- **The runtime publish gate:** a flow write there is not judged for unbound roots yet. Its per-write snapshot carries no actions or other flows, so it cannot see those entrances. Every other expression verdict at that door is unchanged.
34+
35+
A `$` name has no CEL spelling (`$runId == "x"` does not parse), so the `$` roots stay the text-slot rule's.
36+
37+
## FROM → TO
38+
39+
| you wrote | write instead |
40+
|:--|:--|
41+
| `condition: 'user.id == "u1"'` | `condition: 'current_user.id == "u1"'` (`current_user != null && current_user.id == "u1"` where the flow can run without a user) |
42+
| `condition: 'ctx.user.id == record.owner'` | `condition: 'current_user.id == record.owner'` |
43+
| `assignments: { who: { dialect: 'cel', source: 'os.user.id' } }` | `assignments: { who: { dialect: 'cel', source: 'current_user.id' } }` |
44+
| `condition: 'deals.size() > 0'` with nothing binding `deals` | declare it (`variables: [{ name: 'deals', type: 'list' }]`) or bind it with a node (`outputVariable: 'deals'`) |
45+
46+
**The one-line fix: write `current_user` for the run user, and bind or correct every other root the refusal names.**
47+
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+
50+
### The kit
51+
52+
- **The judge.** `flow-cel-root-scope.ts` in `@objectstack/lint`, called by `validateStackExpressions` at each flow CEL site. One bound set per flow, read once like the shadowing pass's variable set, and one entrance index per stack (`flowCelEntrances`).
53+
- **The ledger.** The D3 semantic entry `flow-cel-unbound-root-refused` (protocol 18). There is no D2 conversion: none of these roots ever evaluated in a flow, so there is no behaviour to carry over.

0 commit comments

Comments
 (0)