Repository navigation
Commit 40a6ee5
fix(spec): a value-slot remedy reads a CEL-claimed head through vars (list.0 → vars["list"][0]) (#22524)
Fixes #22290
Clause-②: no (the remedy a refusal prints, and anything else that shares
`celPath`, now evaluates; nothing is newly accepted or refused)
## What was wrong
A value-slot `{…}` refusal prints the CEL spelling of a path token
through `celPath`. That function passed every head through bare except a
`$`-named one. When the head variable is named like an identifier CEL
binds for itself, the bare name reads CEL's binding and not the
variable.
Measured at `4e9fe9ff6` through the built `@objectstack/spec` and
`@objectstack/formula`, with a flow variable `list = ['first',
'second']` in the flow CEL scope:
| template | remedy printed before | `objectstack validate` on that
remedy | evaluation |
|:--|:--|:--|:--|
| `{list.0}` | `list[0]` | refused, `expression-invalid`: `invalid CEL
value: Cannot index type 'type' with type 'int'` | `Cannot index type
'type' with type 'int'` |
| `{list}` | `list` | accepted | the CEL type `list` (a cel-js `Type`
object), not the variable. Nothing reports it. |
| `{list.tags}` | `list.tags` | not probed | fails on the type |
**The card's open question.** The `objectstack validate` reading is from
the built CLI, on a scratch stack with three flows: the old remedy as an
envelope, the new remedy as an envelope, and the `{list.0}` template.
`validate` refuses `source: 'list[0]'` before run time, because the
envelope's own CEL check catches it. So the door that refuses `{list.0}`
was telling the author to write a spelling the same door then refused.
The bare `{list}` → `list` remedy is worse: it passes `validate` and
writes the wrong value. After the fix, the same run prints `Write
{list.0} as { dialect: 'cel', source: 'vars["list"][0]' }`, and the flow
carrying `vars["list"][0]` draws no issue.
## The set, read off the engine this repo builds
`@objectstack/formula` resolves `@marcbachmann/cel-js` 8.0.0. One set,
`CEL_CLAIMED_IDENTIFIERS`, now sits beside `celPath` in
`flow-template-token.ts`, in four groups:
- **Type identifiers**: `bool`, `bytes`, `double`, `int`, `list`, `map`,
`null_type`, `string`, `type`, `uint`. These are `lib/registry.js`
`TYPES`, bound as constants when an environment is built (`for (const n
in TYPES) this.registerConstant(n, 'type', TYPES[n])`).
- **Namespace constants**: `google` (`lib/functions.js`,
`registerConstant('google', …)`), `cel` (`lib/macros.js`) and `optional`
(`lib/optional.js`, bound because the engine sets `enableOptionalTypes:
true`).
- **Reserved words**: `lib/globals.js` `RESERVED`, refused by the parser
as `Reserved identifier: …`. They are `as`, `break`, `const`,
`continue`, `else`, `for`, `function`, `if`, `import`, `let`, `loop`,
`namespace`, `package`, `return`, `var`, `void`, `while`, `__proto__`
and `prototype`.
- **Keywords**: `true`, `false`, `null` (literals) and `in` (operator),
as the lexer reads them.
The first two groups equal `getDefinitions().variables` of an
environment built with the engine's options plus the exported
`registerStdLib` (13 names, measured). `timestamp`, `duration` and `dyn`
are **not** in the set. The card expected `timestamp` and `duration` to
collide, but here they are functions, not bindings, and a variable of
either name already reads correctly (control rows). The card's list also
missed `cel`, `google` and `optional`.
## The fix
- `celPath` reads a claimed head through `vars`, the route it already
took for a `$`-named head: `{list.0}` → `vars["list"][0]`, `{list}` →
`vars["list"]`.
- **In-place, same defect class, same file surface:**
1. A later keyword segment is indexed by name: `{record.in}` printed
`record.in`, which CEL refuses (`Expected IDENTIFIER, got IN`). It now
prints `record["in"]`.
2. `guardOf` prints the `has()` guard only where `has()` accepts it. CEL
refuses `has()` over an index when it runs (`has() invalid argument`,
measured). Before, `{items.1.key}` printed `has(items[1].key) ? …`,
which `evaluateValueEnvelope`'s author-time check refused on the
stale-dist run of the new pins. A path with an index anywhere, a `$`
head or a keyword segment now gets no guard. A claimed head is guarded
as `has(vars.list.tags) ? vars.list.tags : null`.
- Ordinary heads print what they printed before.
- **The same example swap elsewhere** (`list[0]` → `items[0]`, the claim
amended in `6087190357`):
- `content/docs/automation/flows.mdx`, the template table row;
- the semantic migration prose
`18.flow-value-slot-template-dialect-refused.ts`;
- that prose's generated copies (`registry.ts`, `spec-changes.json`,
`docs/protocol-upgrade-guide.md`), regenerated through `check:generated
--fix`.
**Callers of `celPath`** (`git grep`): only
`flow-value-slot-template.ts`, in `guardOf`, in `remedyFor`'s whole-path
branch and in its text-with-holes branch. `flow-text-slot-template.ts`
imports `celExpression` and `templateTokensOf`, not `celPath`. No
ADR-0087 conversion or migration entry calls it, so `Clause-②: no`
stands. The semantic entry `18.flow-value-slot-template-dialect-refused`
carries `list[0]` as prose (see Acceptance notes), not as `celPath`
output.
## Pins
- `packages/spec/src/automation/flow-value-slot-template.test.ts`: the
set equals the enumeration; one row per claimed name (36 rows), covering
`celPath` for a bare head, an indexed head and an index-then-key head,
plus the printed remedy for `{NAME.0}`; guard rows; keyword segments; an
index in the middle; text with holes; controls. The controls are the
ordinary heads `items`, `record`, `timestamp`, `duration`, `dyn`,
`lists`, `map_of` and `vars`, and the `$error` head.
-
`packages/services/service-automation/src/builtin/value-slot-template-grammar.test.ts`
(declared on #6021, test only). The spec cannot import the engine, so
this file holds the evaluation pins. It already pins the judge's reading
against the interpolator's. The new describe adds a third reading: every
CEL spelling the refusal prints (the envelope source, and the guard when
one is printed) goes through `AutomationEngine.evaluateValueEnvelope`,
meaning the author-time envelope check and then the built formula engine
over the real flow CEL scope. It must give the value `interpolateString`
read from the template. The rows:
- 35 name rows, each over `{NAME}`, `{NAME.0}`, `{NAME.1.key}` and
`{NAME.tags}`. `__proto__` is claimed but left out: the flow CEL scope
is a plain object, where `__proto__` names the prototype rather than a
key, so no CEL spelling reads a variable of that name.
- One row that pins the guard being read off the message.
- One row for keyword segments and an index in the middle of a path.
- Four ordinary-head controls (`items`, `timestamp`, `duration`, `dyn`).
- One `$`-head control.
**Ablation.** Run at `347d12026` (fix committed) through
`scripts/ablation-replace.mjs`: the `celPath` head branch was reverted
to the `$`-only test. Anchor ×1 → ×0, marker ×0 → ×1, blob
`9b18b790d96f` → `26df4bf13885`. The spec was rebuilt, and
`ablation-dist-preflight` found the marker in 20 built files. Spec pin
file: **37 failed** | 46 passed (83). Grammar pin file: **35 failed** |
26 passed (61), for example `list: expected {} to deeply equal [
'first', { key: 'second' } ]`. Restore: blob after restore
`9b18b790d96f` equals HEAD, `git diff HEAD` is empty and the whole-tree
porcelain is clean. The spec was rebuilt, and `--absent` found the
marker in none of 232 built files. Spec 83 passed, grammar 61 passed.
After the ablation, `07a6a4dc0` added one line to each name row
(`{NAME.tags}`) and the guard-read row. Those were not ablated.
## Changesets
- `.changeset/22290-value-slot-remedy-cel-claimed-head.md`:
`@objectstack/spec` `patch`.
- `.changeset/19939-flow-value-slot-template-dialect-refused.md`: one
row corrected, `'{list.0}'` / `list[0]` → `'{items.0}'` / `items[0]`.
Nothing else in the file changed. **This edits another card's pending
release note on purpose**: the DELIBERATE CORRECTION class. `Check
Changeset`'s step `Reject an empty-frontmatter changeset added by this
PR` (`scripts/check-empty-changeset.mjs`, the foreign-changeset rule) is
red by design on this edit, and stays red. Locally, `node
scripts/check-empty-changeset.mjs --base origin/main` exits 1 and names
`.changeset/19939-flow-value-slot-template-dialect-refused.md`.
`pr-automation.yml` runs on `pull_request` only, not `merge_group`, and
`Check Changeset` is not one of the seven required contexts. The
correction needs confirming on this PR. Confirmed as a DELIBERATE
CORRECTION in `6087200290`.
## Local verification
At head `b828131c6`. This is the patch round on `07a6a4dc0`, with no
merge of `main`; the PR stays mergeable.
- **Spec `local` tier**, `vitest run --project local`, under the shared
lock: Test Files 630 passed (630); Tests 18840 passed, 1 todo. Lock
verdict `command-exit 0`.
- **Spec `repo` tier**, `vitest run --project repo`: 54 files / 915
tests. `src/api/error-catalog-docs.test.ts` first failed to load on a
transient file that a concurrent `check:skill-examples` run writes and
deletes. Re-run alone under the lock, it passed 5/5.
- **Generated artifacts:** `check:generated` (15 up to date),
`check:migration-registry`, `check:spec-changes` and
`check:upgrade-guide` all exit 0.
- **The pins are unchanged this round.** At `07a6a4dc0`:
`value-slot-template-grammar.test.ts` 62 passed;
`flow-value-slot-template.test.ts` and `flow-text-slot-template.test.ts`
97 passed. The ablation turned the spec pin file red (37 failed) and the
grammar pin file red (35 failed), and the restore was blob == HEAD.
- **Gates.** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` at `b828131c6` derived 115 commands: round
1's 87 plus the docs and migration-registry families.
- 114 exit 0, after a full workspace build that cleared seven
prerequisite refusals on the fresh worktree.
- 1 exits 1 by design: `node scripts/check-empty-changeset.mjs --base
origin/main`, the confirmed deliberate correction.
- `--ran`: `115 derived, 115 run, 0 NOT-MEASURED, 0 UNRUN`.
- `check:api-surface`: `public API surface + factory signatures
unchanged`.
- **CI on `b828131c6`:** all seven required contexts are green. `Check
Changeset` is red by design (confirmed in `6087200290`). There is no
other red.
## Acceptance notes
- The three other texts that taught `list[0]` (the docs table row, the
semantic migration prose and its generated copies) are corrected in this
PR, in commits `650e87277` and `b828131c6`.
- **Expression tokens are not `celPath`.** `celExpression` rewrites
divisors only. `{int * 2}` prints `int * 2`, refused (`no such
overload`), and `{items.0 * 2}` prints `items.0 * 2`, refused (`Expected
IDENTIFIER`); both measured through `evaluateValueEnvelope`. Fixing them
means tokenising the expression, which is not a mechanical change. They
are reported to the seat.
- A flow variable named `vars` is shadowed by the scope's own `vars`
namespace (`vars[0]` → `No such key: 0`; `vars["vars"][0]` reads it).
That binding belongs to the flow runtime, not CEL, so it is not in this
set.
- An envelope an author writes directly as `source: 'list'` still reads
the CEL type. That is CEL's meaning of the identifier, not a remedy this
module prints.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 5910b5e commit 40a6ee5
11 files changed
Lines changed: 240 additions & 18 deletions
File tree
- .changeset
- content/docs/automation
- docs
- packages
- services/service-automation/src/builtin
- spec
- src
- automation
- migrations
- entries/semantic
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
33 | | - | |
| 33 | + | |
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
266 | 266 | | |
267 | 267 | | |
268 | 268 | | |
269 | | - | |
| 269 | + | |
270 | 270 | | |
271 | 271 | | |
272 | 272 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
976 | 976 | | |
977 | 977 | | |
978 | 978 | | |
979 | | - | |
| 979 | + | |
980 | 980 | | |
981 | 981 | | |
982 | 982 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
21 | | - | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
22 | 28 | | |
23 | 29 | | |
24 | 30 | | |
25 | 31 | | |
| 32 | + | |
26 | 33 | | |
27 | 34 | | |
28 | 35 | | |
| |||
100 | 107 | | |
101 | 108 | | |
102 | 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 | + | |
0 commit comments