Skip to content

Commit 6a9124e

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-5330-empty-combinator-lint
2 parents a58e440 + b127c8b commit 6a9124e

10 files changed

Lines changed: 673 additions & 45 deletions

File tree

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
'@objectstack/formula': patch
3+
---
4+
5+
fix(formula): `classifyError` grades a CEL fault by error class + code, never by the message (#6223)
6+
7+
`EvalResult.error.kind` is author-facing — `@objectstack/objectql`'s `cel-fault`
8+
puts it in front of the author as `` `${kind}: ${first line}` `` and
9+
`packages/rest` re-emits it as the HTTP body's `reason`. cel-js embeds the
10+
author's own **source line** in `message` (`formatErrorWithHighlight`), so a
11+
classifier that regex-matches that text is matching text the author writes.
12+
PR #6202 closed the `ParseError` arm this way and left `type` / `runtime` on the
13+
keyword table pending a per-code audit. This is that audit, and its verdict is
14+
that the table goes entirely.
15+
16+
Measured on cel-js 8.0.0 — one `no such overload` **evaluation** fault, four
17+
field names, three wrong answers:
18+
19+
```text
20+
record.status > 1 -> runtime (right)
21+
record.parse_status > 1 -> parse (wrong)
22+
record.syntax_mode > 1 -> parse (wrong)
23+
record.type_code > 1 -> type (wrong)
24+
```
25+
26+
`parse` is the inverse of the #6133 misdirection: the expression is
27+
syntactically perfect and failed on the data, and the author was told to go fix
28+
an expression that has nothing wrong with it.
29+
30+
`classifyError` now reads only structured contract:
31+
32+
- `ParseError` -> `bounds` when `code === 'limit_exceeded'`, else `parse`
33+
(unchanged, from #6202);
34+
- `EvaluationError` -> `type` for the one declaration-class code
35+
(`unknown_variable`, the root identifier is not bound in this scope at all),
36+
else `runtime`;
37+
- anything that is not a cel-js error -> `runtime`.
38+
39+
Two findings from the audit worth recording. First, the residual keyword arm was
40+
**not** dormant: `matches()` is an ObjectStack stdlib binding over `new
41+
RegExp(...)`, so an uncompilable pattern escapes as a native `SyntaxError` whose
42+
message echoes the pattern — and the pattern can come off the row, not just out
43+
of the source. `matches(record.name, record.re)` with `re = "type("` was
44+
graded `type`; with `"Exceeded maxAstNodes("` it was graded `bounds`. A data
45+
value was picking the error kind. Second, there is deliberately no `TypeError`
46+
arm: cel-js raises that class only from its non-evaluating `TypeChecker`, which
47+
runs only inside `Environment#check`, and that method catches it and *returns*
48+
`{ valid: false, error }`. The check-time `TypeError -> type` mapping already
49+
lives in `celEngine.compile`, which reads that object.
50+
51+
Six evaluate-time codes change verdict from `type` to `runtime`
52+
(`int_conversion_error`, `uint_conversion_error`, `double_conversion_error`,
53+
`invalid_index_type`, `heterogeneous_list_element`,
54+
`invalid_comprehension_range`). Each is a fault decided against the row; every
55+
one of them was graded `type` only because cel-js happens to use the word "type"
56+
in its prose (`int() type error: cannot convert to int`). Every evaluate-time
57+
code the engine can reach now has a fixture pinning its `kind`.
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/core": patch
4+
"@objectstack/lint": patch
5+
---
6+
7+
fix(spec,core): a filter placeholder is recognised by INTENT — `{TODAY()}` refuses loudly instead of comparing as a literal (#5586)
8+
9+
`UnknownFilterTokenError` had a hole exactly where authors fall in. Recognition
10+
used the token-NAME grammar `/^\$?\{([a-zA-Z0-9_]+)\}$/`, so any placeholder
11+
carrying a **non-word character** classified as "not a placeholder at all" and
12+
was handed to the driver verbatim, to be compared as a literal string — the
13+
silent-wrong-result failure the diagnostic exists to abolish.
14+
15+
The failure was inverted against the author. Measured on 17.0.0-rc.2 against a
16+
four-row fixture:
17+
18+
| filter value | before | |
19+
|---|---|---|
20+
| `due_date < '{today}'` | 2 rows | correct — the two overdue rows |
21+
| `due_date < '{TODAY}'` | throws `UnknownFilterTokenError` | diagnostic working |
22+
| `due_date < '{TODAY()}'` | **4 rows** | diagnostic bypassed — literal string compare, and `'2026-…' < '{'` in lexicographic order swallowed a row due a week later |
23+
24+
So misspelling `{today}` as `{TODAY}` was reported by name, while misspelling it
25+
as `{TODAY()}` returned the wrong rows in silence — and the parenthesised,
26+
kebab-case, natural-language and dotted spellings (`{TODAY()}`,
27+
`{current-user-id}`, `{30 days ago}`, `{user.id}`) are precisely what an author
28+
migrating from another system's macro syntax writes first.
29+
30+
**Both directions of the behaviour change:**
31+
32+
- **Previously silent, now refuses loudly** — a filter value that is entirely
33+
brace-wrapped and outside the vocabulary now throws `UnknownFilterTokenError`
34+
(`code: FILTER_TOKEN_UNKNOWN`, `status: 400`) on the ObjectQL read and write
35+
paths and the analytics dataset executor, and is reported as
36+
`filter-token-unknown` by `objectstack build` / `validate` / `lint`. Before,
37+
it reached the data engine and compared as text.
38+
- **Unchanged**`{today}` / `{current_user_id}` still resolve; `{TODAY}` still
39+
refuses with the same identity; a value that merely *contains* braces
40+
(`'acme {x} deal'`), or is not ONE pair around the whole value (`{a}{b}`,
41+
`{{x}}`, `{}`), is still an ordinary literal and still reaches the driver
42+
untouched.
43+
44+
Recognition and vocabulary are now two named grammars rather than one:
45+
`FILTER_TOKEN_WRAPPED_RE` (`/^\$?\{([^{}]+)\}$/`) answers "did the author mean a
46+
placeholder", and `isContextToken` / `isDateMacroToken` answer "is it in the
47+
vocabulary". Wide in, strict out. No escape hatch for a literal `{…}` comparand
48+
ships with this: a repo-wide measurement across structured metadata, examples,
49+
seed data and fixtures found zero legitimate consumers comparing a
50+
brace-wrapped literal, and an escape syntax is a public micro-contract that can
51+
be added the day one shows up.
52+
53+
Flow templates are unaffected. `interpolateFilter` in
54+
`@objectstack/service-automation` already recognised the same wide shape and
55+
resolves `{record.id}` / `{TODAY() + 30}` from flow variables **before** the
56+
filter reaches ObjectQL; its hand-off to the engine is keyed on the token
57+
vocabulary (`isKnownFilterToken`), which this change does not touch.

packages/core/src/utils/filter-tokens.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,83 @@ describe('resolveFilterTokens — tree walk', () => {
232232
});
233233
});
234234

235+
/**
236+
* #5586 — a placeholder carrying a NON-WORD character used to bypass the
237+
* diagnostic entirely.
238+
*
239+
* Recognition was the token-NAME grammar, so `{TODAY()}` classified as "not a
240+
* placeholder", was handed to the driver verbatim and compared as a literal
241+
* string. Measured on 17.0.0-rc.2 against a four-row fixture: `due_date <
242+
* '{today}'` returned the 2 genuinely overdue rows, `due_date < '{TODAY()}'`
243+
* returned all 4 — lexicographic string order puts every `'2026-…'` before
244+
* `'{'`, so the window silently swallowed a row due a week later.
245+
*
246+
* The refusal is asserted on the ADR-0112 envelope (`code` + `status`) plus the
247+
* offending token, never on the bare fact of a throw: the resolver already
248+
* throws for other reasons, so a throw-only assertion cannot tell "refused with
249+
* the right identity" from "blew up somewhere else".
250+
*/
251+
describe('resolveFilterTokens — non-word placeholder shapes refuse loudly (#5586)', () => {
252+
const ctx = { now: NOW, userId: 'usr_1', orgId: 'org_9' };
253+
254+
const shapes: Array<[label: string, value: string, token: string]> = [
255+
['call syntax (Salesforce/Excel migrants)', '{TODAY()}', 'TODAY()'],
256+
['kebab-case', '{current-user-id}', 'current-user-id'],
257+
['natural language', '{30 days ago}', '30 days ago'],
258+
['dotted path', '{user.id}', 'user.id'],
259+
['the `${…}` prefix variant', '${TODAY()}', 'TODAY()'],
260+
];
261+
262+
it.each(shapes)('%s — %s refuses with the full error identity', (_label, value, token) => {
263+
let err: unknown;
264+
try {
265+
resolveFilterTokens({ due_date: { $lt: value } }, ctx);
266+
} catch (e) {
267+
err = e;
268+
}
269+
expect(err).toBeInstanceOf(UnknownFilterTokenError);
270+
const e = err as UnknownFilterTokenError;
271+
expect(e.name).toBe('UnknownFilterTokenError');
272+
// ADR-0112 envelope: the caller's filter is malformed, the server is fine.
273+
expect(e.code).toBe('FILTER_TOKEN_UNKNOWN');
274+
expect(e.status).toBe(400);
275+
// The author has to see what THEY wrote, not a normalised paraphrase.
276+
expect(e.token).toBe(token);
277+
expect(e.message).toContain(`{${token}}`);
278+
});
279+
280+
it('still resolves the canonical spelling — the widening did not eat `{today}`', () => {
281+
expect(resolveFilterTokens({ due_date: { $lt: '{today}' } }, ctx))
282+
.toEqual({ due_date: { $lt: '2026-07-15' } });
283+
});
284+
285+
it('still refuses the word-character near miss `{TODAY}`', () => {
286+
// The shape that ALREADY worked. It is the control: if this ever goes
287+
// quiet, the widening has replaced the diagnostic instead of extending it.
288+
let err: unknown;
289+
try {
290+
resolveFilterTokens({ due_date: { $lt: '{TODAY}' } }, ctx);
291+
} catch (e) {
292+
err = e;
293+
}
294+
expect(err).toBeInstanceOf(UnknownFilterTokenError);
295+
expect((err as UnknownFilterTokenError).token).toBe('TODAY');
296+
expect((err as UnknownFilterTokenError).code).toBe('FILTER_TOKEN_UNKNOWN');
297+
});
298+
299+
// Decided, not emergent: recognition is ONE brace pair around the WHOLE
300+
// value. Anything else is ordinary text and reaches the driver untouched —
301+
// that is what keeps `titleFormat`-style strings and human prose out of the
302+
// rule, and it is the property that holds false positives at zero.
303+
it.each(['acme {x} deal', '{a}{b}', '{{x}}', '{}', '{a}b', 'x{a}'])(
304+
'%s is a literal and passes through unchanged',
305+
(value) => {
306+
const filter = { title: value };
307+
expect(resolveFilterTokens(filter, ctx)).toBe(filter);
308+
},
309+
);
310+
});
311+
235312
describe('filterTokenContextFrom', () => {
236313
it('maps ExecutionContext onto the resolver inputs', () => {
237314
expect(

packages/core/src/utils/filter-tokens.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,18 @@
4848
* through is precisely the silent-zero bug this module exists to end, so it is
4949
* a hard error carrying the near-miss suggestion (`{current_user}` →
5050
* `{current_user_id}`). Values that merely CONTAIN braces are left untouched.
51+
*
52+
* "Entirely `{something}`" means ANY character between the braces (#5586).
53+
* Until then the recognition grammar was the token-NAME grammar
54+
* (`[a-zA-Z0-9_]+`), so a placeholder carrying a non-word character —
55+
* `{TODAY()}`, `{current-user-id}`, `{30 days ago}`, `{user.id}` — was not
56+
* recognised as a token at all and fell straight through to the literal
57+
* comparison this module exists to abolish. The failure was inverted against
58+
* the author: `{TODAY}` threw (diagnostic working), `{TODAY()}` returned rows
59+
* (diagnostic bypassed) — and the parenthesised, kebab-case and
60+
* natural-language spellings are exactly what an author migrating from another
61+
* system's macro syntax reaches for first. See `FILTER_TOKEN_WRAPPED_RE` in
62+
* `@objectstack/spec`.
5163
*/
5264

5365
import {

0 commit comments

Comments
 (0)