Repository navigation
Commit 5aa7f55
fix(app-shell): a record-triggered Start node is judged against the scope the engine binds, with one verdict (objectui#11789) (#11849)
Fixes #11789
Clause-②: no
## What was wrong, measured on `main` (`87f7b6c`)
A record-triggered flow's Start node read **"Valid CEL"** and, right
under it, **"`record` is not a reference in scope at this step."** for
the same entry condition. Two defects were behind that one screen.
1. **The Start node's scope had no `record`.** `resolveFlowScope` in
`flow-scope.ts` pushed the whole-record `record` ref only `if
(!onStart)`. The engine binds it there. `seedRunVariables` in
`@objectstack/service-automation` sets `record`, `$record`, the record's
flattened fields and `previous`, and the start-condition gate then
evaluates against that same variables map. Every shipped spelling
resolves at run time: `record.status` and bare `status`.
2. **Two verdicts for one expression.** The raw CEL editor
(`CelPredicateField`, mounted by `ConditionBuilder`) says "Valid CEL"
from its lint. Its lint knows the CEL scope roots, not the flow's scope
at the node. The scope note comes from somewhere else, so a root the
lint accepts and the flow scope rejects got both lines.
This PR's new pins were first run with the source unchanged: **3 failed,
3 passed (6)**. The card's expression failed on the scope note. The
`trigger.status` pin and the editor pin failed on "Valid CEL" shown
beside the note. The 3 that passed are the controls.
## The change
### `record` is in scope on a record-triggered Start node
In `flow-scope.ts`, the `record` ref is pushed at every node of a
record-triggered flow, the Start node included. What still differs on
the Start node is the per-field prefix only: bare there, `record.`
downstream. A schedule, manual or API Start node gains nothing. The
existing record-trigger gate (`RECORD_TRIGGER_TYPES` plus an
`objectName`) is untouched.
`flow-ref-check.ts` is **not** touched. Adding `record` / `previous` to
its `RUNTIME_GLOBALS` would accept them on schedule and manual flows,
where the engine binds no record.
### `previous` follows the engine's pre-image
The engine binds `previous` on **every** run: to the pre-image the
record-change trigger hands it, or to `null` when there is none. So
Studio scopes it where a pre-image exists.
| Trigger | `record` | `previous` | What the engine binds |
|---|---|---|---|
| `record-after-update`, `record-before-update` | in scope | in scope |
the prior row |
| `record-after-write`, `record-before-write` | in scope | in scope |
`null` on the create leg, the prior row on the update leg (`previous ==
null` picks the create leg) |
| `record-after-delete` | in scope | **in scope (new)** | the deleted
row: the data engine binds the pre-image before the delete runs, and the
trigger reads `record` from it too |
| `record-after-create` | in scope | **out (unchanged)** | always
`null`: there is no prior row |
| `schedule`, `manual`, `api` | out | out | no record is handed to the
run |
**Why create stays out** (the seat's answer A to this PR's open
question): a member read on `null`, such as `previous.status`, is a CEL
evaluation error. The engine's `evaluateCondition` throws on it, so the
run fails. And `previous == null` is constantly true there. Showing the
scope note is the useful verdict. The table is pinned row by row in
`flow-scope.test.ts`, at the Start node and downstream.
### One verdict: the scope note replaces "Valid CEL"
**Zone 2 #3's location is falsified.** The scope line under the entry
condition is not `FlowExprIssue`'s. It is `FlowNodeConfigField`'s own
`describeUnknownRefs` note, rendered under the control it mounts.
`FlowExprIssue` never renders beside a `CelPredicateField`: only the
Start node's `condition` descriptor opts into `conditionBuilder`. So the
rule lands where both verdicts render:
- `FlowNodeConfigField` computes its scope verdict before it builds the
control, and hands `scopeIssue` to the `ConditionBuilder` it mounts;
- `ConditionBuilder` forwards the new optional `scopeIssue` to its raw
editor;
- `CelPredicateField` withholds "Valid CEL" when `scopeIssue` is set.
The lint, its findings and `onLintChange` are unchanged. With no
`scopeIssue`, nothing changes. That covers the permission set's
row-level security clauses, pinned as a control.
The note's wording is unchanged and no catalogue row was added.
`FlowNodeConfigField`'s `FLOW_TRIGGER_CONTEXT_SUBJECTS` doc comment said
`flow-scope.ts` withholds `record` on the Start node, which this change
makes false, so it was reworded. The builder still offers only the bare
spelling, because `record.FIELD` is the same value. Two sibling test
files carried the same false reason in a test name and a comment, and
were reworded the same way. No assertion changed.
### Side effect: an edge leaving the Start node
An edge's guard is judged against the scope at its source
(`resolveEdgeScope` / `useEdgeScope`). So an edge leaving the Start node
now accepts `record` too. That matches the engine: `traverseNext`
evaluates those guards against the same run variables. The Problems
panel's expression scan skips the Start node and its out-edges
(`flowExpressionProblems`, by design: there the trigger fields are not
expanded), so its output does not move.
## Verification
All at `b097f9a` (this branch merged with `main` `455c646`) unless
stated.
- `pnpm --filter @object-ui/app-shell type-check` (echoed `tsc --noEmit
&& tsc -p tsconfig.test.json`): **exit 0**. The dependency closure was
rebuilt first with `pnpm --workspace-concurrency=2 --filter
'@object-ui/app-shell^...' build`, exit 0. `--listFiles` on
`tsconfig.test.json` lists the new pin file.
- The targeted set (the new pin file, `CelPredicateField.test.tsx`,
`flow-scope.test.ts`, both `entryCondition` suites, the
`ConditionBuilder.*` suites): `Test Files 14 passed (14)`, `Tests 227
passed (227)`, exit 0.
- At `1bbfc4b` (before the merge of `main`): `pnpm exec vitest run
packages/app-shell/` gave `Test Files 1078 passed | 1 skipped (1079)`,
`Tests 10616 passed | 9 skipped (10625)`, exit 0. `main`'s app-shell
changes since then (objectui#11783, objectui#11811) touch none of these
files. The package-wide run on the merged head is CI's.
- `pnpm exec eslint` on the 8 touched source and test files: 0 errors, 8
warnings, all on lines outside this diff.
- `pnpm check:control-bytes`, `check:test-path-roots`,
`check:changeset-claims`, `check:pending-changeset-literals`,
`check:new-line-citations`, plus `scripts/check-changeset-presence.mjs`
and `scripts/check-changeset-no-major.mjs`: all exit 0 on `b097f9a`. The
i18n gates are not applicable: no locale pack or catalogue row changed.
**Ablation**, run at `1bbfc4b` through `ablation-replace` (the anchor
must hit, and the restore is proven against HEAD). The tests import the
subjects by relative source path, so there is no `dist` leg.
- **scope**: put `if (!onStart)` back on the `record` push in
`flow-scope.ts`. Anchor 1 → 0, blob `3180f147cab2` → `65c6870b91c0`.
Result **8 failed / 51 passed**: the card's pin, the Start-node pin and
the 6 record-trigger rows of the table. Restored: blob == HEAD
`3180f147cab2`, `git diff HEAD` empty.
- **one verdict**: changed `issues.length === 0 && !scopeIssue;` to
`issues.length === 0;` in `CelPredicateField.tsx`. Blob `51ef56396d7d` →
`e2187dea09eb`. Result **2 failed / 57 passed**: the `trigger.status`
pin and the editor-contract pin. Restored: blob == HEAD `51ef56396d7d`,
diff empty.
Both went red, as predicted.
**Clause-② (no)**, measured on the built package. A transitive walk of
`dist/index.d.ts`'s relative imports reaches 172 declaration files. None
of `CelPredicateField`, `ConditionBuilder`, `FlowNodeConfigField`,
`flow-scope`, `flow-ref-check`, `FlowExprIssue` or `useFlowScope` is
among them. The positive control `DirectoryPage.d.ts` is reached.
`scopeIssue` is in the emitted `CelPredicateField.d.ts` and
`ConditionBuilder.d.ts`, and in none of the reachable files. `exports`
declares only `.` and `./styles.css`.
## Overlap
Per the seat's claim amendment, objectui#11788 may edit
`FlowNodeConfigField.tsx` in another region (the notify Recipients and
field-mapping rows). This PR's edit there is the entry condition's scope
verdict, the `scopeIssue` handed to its `ConditionBuilder`, and the
`FLOW_TRIGGER_CONTEXT_SUBJECTS` comment. Whoever lands second merges
`main`.
## Acceptance notes
Noted, not filed. Neither one gives a wrong verdict at a public door
today.
- **`time_relative` Start nodes get no trigger scope.** The engine's
time-relative sweep hands each matched row to the run as `record`.
`flow-scope.ts` gives `time_relative` no trigger scope: the type is not
in `RECORD_TRIGGER_TYPES`, and its object is at
`config.timeRelative.object`, not `config.objectName`. The effect is
silent today. With no declared variable, the ref check has no roots and
says nothing. The shipped producer (app-showcase's
`showcase_task_due_reminder`, with `{record.title}` templates) declares
none. A flow that also declared a variable would see `record` flagged.
Carrier: none.
- **Four record-trigger tokens are not in Studio's set.** The
record-change trigger accepts
`record-(before|after)-(create|insert|update|delete|write)`, and
`RECORD_TRIGGER_TYPES` lists 6 of those 10 tokens. A grep over
objectstack `main` (`15ec50e5`) examples and package sources finds no
producer of the other four (before-create, before-insert, after-insert,
before-delete). The control, the same grep for `record-after-update`,
hits 15 times in 3 example files. Studio's trigger select does not offer
them either. Carrier: none.
Changeset: `.changeset/11789-cel-scope-verdict.md`, `patch` on
`@object-ui/app-shell`.
Implemented by the os-dev run under session
`https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8`, dispatched by
the `domain:ui` seat 3 claim on the card.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent 4a9fe31 commit 5aa7f55
9 files changed
Lines changed: 354 additions & 56 deletions
File tree
- .changeset
- packages/app-shell/src/views/metadata-admin
- inspectors
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
Lines changed: 213 additions & 0 deletions
| 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 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
Lines changed: 14 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
107 | 107 | | |
108 | 108 | | |
109 | 109 | | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
110 | 120 | | |
111 | 121 | | |
112 | 122 | | |
| |||
146 | 156 | | |
147 | 157 | | |
148 | 158 | | |
| 159 | + | |
149 | 160 | | |
150 | 161 | | |
151 | 162 | | |
| |||
342 | 353 | | |
343 | 354 | | |
344 | 355 | | |
345 | | - | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
346 | 359 | | |
347 | 360 | | |
348 | 361 | | |
| |||
Lines changed: 9 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
521 | 521 | | |
522 | 522 | | |
523 | 523 | | |
524 | | - | |
| 524 | + | |
525 | 525 | | |
526 | 526 | | |
527 | 527 | | |
| |||
598 | 598 | | |
599 | 599 | | |
600 | 600 | | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
601 | 608 | | |
602 | 609 | | |
603 | 610 | | |
| |||
740 | 747 | | |
741 | 748 | | |
742 | 749 | | |
| 750 | + | |
743 | 751 | | |
744 | 752 | | |
745 | 753 | | |
| |||
packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.entryCondition.test.tsx
Lines changed: 4 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
316 | 316 | | |
317 | 317 | | |
318 | 318 | | |
319 | | - | |
| 319 | + | |
320 | 320 | | |
321 | 321 | | |
322 | | - | |
323 | | - | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
324 | 325 | | |
325 | 326 | | |
326 | 327 | | |
| |||
0 commit comments