Repository navigation
Commit 9d96b2b
fix(formula): has() over an indexed or call receiver evaluates as the CEL spec defines it (#25275)
<!-- claude-projects-attribution: {"github_login":"os-bill"} -->
_Requested by **Bill** · [project
thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wg8b7MP77ty5yyBo5R7ks3hC)_
Fixes #25267
Clause-②: yes (widening)
Metadata-protocol: added
## What was wrong
The CEL spec defines `has(e.f)` for any expression `e`: a presence test
on field `f` of whatever `e` evaluates to. cel-js 8.0.0 (the latest
release) implements `has()` as a macro whose type-check walks the
selection chain back to its root and refuses the argument with `has()
invalid argument` unless that root is an identifier. So
`has(data.environments[0].storage.usedMb)`, `has(record.items[0].name)`
or `has(dyn(x).y)` faulted at every type-check and every evaluation.
Measured on `origin/main` 17f3c65 before the change, through
`@objectstack/formula`'s public entries:
| door | `has(data.environments[0].storage.usedMb)` | `has(record.a.b)`
(control) |
|---|---|---|
| `celEngine.evaluate` | `ok: false`, `has() invalid argument` | `ok:
true` |
| `celEngine.compile` | refused | accepted |
| `validateExpression('predicate', …)` (object / flow / action /
sharing-rule / hook slots) | refused | accepted |
| `validateVisibilityPredicates` (page and view `visibleWhen`) | **0
findings** | 0 findings |
The page and view gate checks syntax, unknown functions and bare
identifiers, never types (the #6253 / #13594 rulings), so on that
surface the predicate published clean and then failed open in the
console. The fault is cel-js's own (no ObjectStack wrapping is
involved), and `Environment.registerFunction('has(ast): bool', …)`
cannot replace the built-in macro: cel-js refuses the overlapping
signature.
## The fix: the engine evaluates it, as the spec says
The route is the "evaluate" branch of the card. `has()` over an indexed
or call receiver is valid CEL, and the engine can honour it without
forking cel-js.
`rewriteIndexedHas` (in `cel-engine.ts`, next to the #3306
`rewriteNullableTernary`, built the same way: a parse, an AST walk, a
reserialize only when something changed, a bounded memo) lowers such a
call to `e.?f.hasValue()`. That is the spec's definition of optional
field selection (`e.?f` is `optional.of(e.f)` when `has(e.f)`, else
`optional.none()`), so the two are the same test. cel-js evaluates the
receiver exactly as its `has()` does: an absent key before the last step
still faults `No such key`, and only the last step answers `false`.
- It fires only on a `has()` whose argument is a `.` selection rooted in
something other than an identifier. `has(record.a.b)` is left to cel-js.
So is every argument the spec itself refuses: a bare name
(`has(status)`) and an index as the last step (`has(record.items[0])`)
still fault `has() invalid argument` everywhere.
- It is applied at every place cel-js type-checks: `evaluate`,
`compile`, `firstUndeclaredReference`, `inferCelType` and the
overload-fault probe. The parse-only front end (`parseCelToAst`) keeps
the authored `has(…)` call. It already parsed, and AST consumers such as
the lint's `has(…)` span mask keep finding the call they key on.
- No lint change is needed. Every rule that judges a CEL predicate
reaches the type verdict through `@objectstack/formula`
(`validateExpression`, `celEngine.compile`), or does not judge types at
all (the page and view rules). They now agree with the runtime.
## Tests
New `packages/formula/src/cel-has-indexed-receiver.test.ts`, 9 tests:
- the filed reading (present answers `true`, absent answers `false`),
and agreement with the `'usedMb' in …` form the filing worked around
with;
- parity with cel-js's own identifier-rooted `has()` over ten value
kinds (map with or without the key, key holding `null` / `false`, empty
map, `null`, list, int, string, bool) and two path depths, with faults
compared by kind and first line;
- every door gives one verdict: `compile`, `validateExpression`,
`evaluate`, `inferCelType` (`bool`), and `firstUndeclaredReference`,
which now reports a bare `status` behind an indexed `has()` instead of
being masked by the macro's fault;
- controls: `has(status)` and `has(record.items[0])` are still refused
by `compile`, `validateExpression` and `evaluate`;
- the rewrite is narrow (five untouched spellings), idempotent, and
lowers a `has()` nested inside a lowered receiver.
Ablation at baf8e22, with an early `return source;` injected into
`rewriteIndexedHas` through `scripts/ablation-replace.mjs`: the suite
went 8 failed / 1 passed (the controls test is the one that stays green,
as it should). The restore was verified as blob == HEAD (`f4f10ad31d5c`)
and `git diff HEAD` empty. No `dist/` is involved: the suite imports
`./cel-engine` from source.
Runs at head baf8e22 (under `os-verify-lock.sh`):
- `pnpm --filter @objectstack/formula build && … test && … typecheck`:
exit 0. Test Files 52 passed (52), Tests 1376 passed (1376).
- `pnpm --filter @objectstack/lint test`: exit 0. Test Files 153 passed
(153), Tests 7076 passed (7076).
## Gates
`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 67 commands at baf8e22. All 67
were run, with exit codes captured before any pipe, and `--ran`
reconciles them: 66 run, 1 NOT-MEASURED, 0 unrun. 66 exited 0. Two of
those first exited 3 on missing prerequisites and were 0 on a rerun:
`check-plugin-teardown-shape --self-test` after fetching its
shallow-clone fixture commit, and `check:lean-entry-closure` after
building `@objectstack/objectql`. The NOT-MEASURED one is
`check:dual-build-cjs-loads` (exit 3, needs every package's `dist`),
which is left to CI.
Dogfood: n/a. The title carries no `!` and the `Clause-②` arm is
`(widening)`.
## Acceptance notes
- **Why `Clause-②: yes (widening)` / `added`.** The claim expected `no`.
Measured from the diff, `validateExpression` used to refuse this
argument on object, flow, action, sharing-rule and hook predicates, and
now accepts it. That is a wider publish accept set, and a newly accepted
value for an author. The changeset is `minor` on `@objectstack/formula`
accordingly.
- **A docblock this makes stale, outside the claim's file surface and
left alone.** `packages/spec/src/automation/flow-value-slot-template.ts`
(`guardOf`) and the test title at `flow-value-slot-template.test.ts:201`
say `has()` refuses an index anywhere in its argument at run time. That
reason is now false. The behaviour it explains (no guard is printed for
a path with a numeric segment) is still right, because a path ending in
an index cannot be guarded by `has()`, and an out-of-range index faults
instead of answering `false`.
- **The sibling class stays open.** `has(status)` and
`has(record.items[0])` on a page or view `visibleWhen` still publish
with 0 findings (measured: `validateVisibilityPredicates` against a page
component, with `record.x === 1` as the firing control), and still fault
at runtime. Closing that means adopting one more `check()` verdict on
the page and view rule, which the #13594 ruling ("只拒未知函数裁定,⛔ 不搬运
`check()` 的其他抱怨") reserves for the maintainer. It is not done here.
- **Error excerpts.** A runtime fault inside a lowered call (an absent
intermediate key) quotes the lowered source in cel-js's excerpt, the
same property the #3306 rewrite already has.
- **Removal.** When cel-js lifts the identifier-root restriction from
its `has()` macro, `rewriteIndexedHas` and its five call sites can go,
together with its own describe block; the evaluation and every-door pins
keep holding.
---
_Generated by [Claude
Code](https://claude.ai/code/session_019RfszUM1D9pJhyWiBkucZP)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 1eb1afa commit 9d96b2b
3 files changed
Lines changed: 254 additions & 6 deletions
File tree
- .changeset
- packages/formula/src
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
404 | 404 | | |
405 | 405 | | |
406 | 406 | | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
407 | 410 | | |
408 | 411 | | |
409 | 412 | | |
| |||
423 | 426 | | |
424 | 427 | | |
425 | 428 | | |
426 | | - | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
427 | 432 | | |
428 | 433 | | |
429 | 434 | | |
| |||
862 | 867 | | |
863 | 868 | | |
864 | 869 | | |
865 | | - | |
| 870 | + | |
| 871 | + | |
866 | 872 | | |
867 | 873 | | |
868 | 874 | | |
| |||
1066 | 1072 | | |
1067 | 1073 | | |
1068 | 1074 | | |
1069 | | - | |
| 1075 | + | |
1070 | 1076 | | |
1071 | 1077 | | |
1072 | 1078 | | |
| |||
1463 | 1469 | | |
1464 | 1470 | | |
1465 | 1471 | | |
| 1472 | + | |
| 1473 | + | |
| 1474 | + | |
| 1475 | + | |
| 1476 | + | |
| 1477 | + | |
| 1478 | + | |
| 1479 | + | |
| 1480 | + | |
| 1481 | + | |
| 1482 | + | |
| 1483 | + | |
| 1484 | + | |
| 1485 | + | |
| 1486 | + | |
| 1487 | + | |
| 1488 | + | |
| 1489 | + | |
| 1490 | + | |
| 1491 | + | |
| 1492 | + | |
| 1493 | + | |
| 1494 | + | |
| 1495 | + | |
| 1496 | + | |
| 1497 | + | |
| 1498 | + | |
| 1499 | + | |
| 1500 | + | |
| 1501 | + | |
| 1502 | + | |
| 1503 | + | |
| 1504 | + | |
| 1505 | + | |
| 1506 | + | |
| 1507 | + | |
| 1508 | + | |
| 1509 | + | |
| 1510 | + | |
| 1511 | + | |
| 1512 | + | |
| 1513 | + | |
| 1514 | + | |
| 1515 | + | |
| 1516 | + | |
| 1517 | + | |
| 1518 | + | |
| 1519 | + | |
| 1520 | + | |
| 1521 | + | |
| 1522 | + | |
| 1523 | + | |
| 1524 | + | |
| 1525 | + | |
| 1526 | + | |
| 1527 | + | |
| 1528 | + | |
| 1529 | + | |
| 1530 | + | |
| 1531 | + | |
| 1532 | + | |
| 1533 | + | |
| 1534 | + | |
| 1535 | + | |
| 1536 | + | |
| 1537 | + | |
| 1538 | + | |
| 1539 | + | |
| 1540 | + | |
| 1541 | + | |
| 1542 | + | |
| 1543 | + | |
| 1544 | + | |
| 1545 | + | |
| 1546 | + | |
| 1547 | + | |
| 1548 | + | |
| 1549 | + | |
| 1550 | + | |
| 1551 | + | |
| 1552 | + | |
| 1553 | + | |
| 1554 | + | |
| 1555 | + | |
| 1556 | + | |
| 1557 | + | |
| 1558 | + | |
| 1559 | + | |
| 1560 | + | |
| 1561 | + | |
| 1562 | + | |
| 1563 | + | |
| 1564 | + | |
| 1565 | + | |
| 1566 | + | |
| 1567 | + | |
| 1568 | + | |
| 1569 | + | |
| 1570 | + | |
| 1571 | + | |
| 1572 | + | |
| 1573 | + | |
| 1574 | + | |
| 1575 | + | |
| 1576 | + | |
1466 | 1577 | | |
1467 | 1578 | | |
1468 | 1579 | | |
| |||
1870 | 1981 | | |
1871 | 1982 | | |
1872 | 1983 | | |
1873 | | - | |
| 1984 | + | |
| 1985 | + | |
1874 | 1986 | | |
1875 | 1987 | | |
1876 | 1988 | | |
| |||
1942 | 2054 | | |
1943 | 2055 | | |
1944 | 2056 | | |
1945 | | - | |
1946 | | - | |
| 2057 | + | |
| 2058 | + | |
| 2059 | + | |
| 2060 | + | |
1947 | 2061 | | |
1948 | 2062 | | |
1949 | 2063 | | |
| |||
| 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 | + | |
0 commit comments