Skip to content

Commit bd41a1f

Browse files
committed
fix(spec,core): recognise a filter placeholder by intent — any brace-wrapped value refuses loudly (#5586)
Recognition used the token-NAME grammar, so a placeholder carrying a non-word character ({TODAY()}, {current-user-id}, {30 days ago}, {user.id}) classified as 'not a placeholder' and reached the driver to be compared as a literal string — the silent-wrong-rows mode the diagnostic exists to abolish. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MwoubC3jL271FYt9rGXwxb
1 parent 0e043d8 commit bd41a1f

6 files changed

Lines changed: 365 additions & 1 deletion

File tree

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 {

packages/objectql/src/engine-filter-tokens.test.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,107 @@ describe('engine filter placeholders (framework#3582)', () => {
180180
expect(seen.findAst?.where).toEqual({ title: 'acme {x} deal', owner: 'usr_2' });
181181
});
182182

183+
/**
184+
* #5586 — the read path is where the bypass was measured, so it is pinned
185+
* here and not only at the resolver.
186+
*
187+
* A placeholder carrying a non-word character was not recognised as a token,
188+
* so it rode the AST all the way to the driver and was compared as a literal
189+
* string. On the issue's four-row fixture `due_date < '{TODAY()}'` returned
190+
* 4 rows where `due_date < '{today}'` returned the 2 genuinely overdue ones
191+
* — wrong rows, no error, indistinguishable from a correct answer.
192+
*/
193+
describe('non-word placeholder shapes refuse before the driver (#5586)', () => {
194+
it.each([
195+
['{TODAY()}', 'TODAY()'],
196+
['{current-user-id}', 'current-user-id'],
197+
['{30 days ago}', '30 days ago'],
198+
['{user.id}', 'user.id'],
199+
['${TODAY()}', 'TODAY()'],
200+
])('find(): %s throws and the driver is never reached', async (value, token) => {
201+
const { driver } = makeDriver();
202+
const ql = await makeEngine(driver);
203+
204+
let err: any;
205+
try {
206+
await ql.find('deal', { where: { close_date: { $lt: value } }, context: CTX });
207+
} catch (e) {
208+
err = e;
209+
}
210+
211+
// The specific identity, not the bare fact of a rejection: the point of
212+
// the fix is WHICH error the caller gets, and a `rejects.toThrow()` here
213+
// would stay green on a driver-level blow-up.
214+
expect(err?.name).toBe('UnknownFilterTokenError');
215+
expect(err?.code).toBe('FILTER_TOKEN_UNKNOWN');
216+
expect(err?.status).toBe(400);
217+
expect(err?.token).toBe(token);
218+
expect(err?.message).toContain(`{${token}}`);
219+
expect(driver.find).not.toHaveBeenCalled();
220+
});
221+
222+
it('find(): the word-character near miss `{TODAY}` still throws', async () => {
223+
// Control for the widening: the shape that already refused must keep
224+
// refusing with the same identity.
225+
const { driver } = makeDriver();
226+
const ql = await makeEngine(driver);
227+
228+
let err: any;
229+
try {
230+
await ql.find('deal', { where: { close_date: { $lt: '{TODAY}' } }, context: CTX });
231+
} catch (e) {
232+
err = e;
233+
}
234+
expect(err?.name).toBe('UnknownFilterTokenError');
235+
expect(err?.token).toBe('TODAY');
236+
expect(driver.find).not.toHaveBeenCalled();
237+
});
238+
239+
it('find(): `{today}` still resolves to a concrete date', async () => {
240+
const { driver, seen } = makeDriver();
241+
const ql = await makeEngine(driver);
242+
243+
await ql.find('deal', { where: { close_date: { $lt: '{today}' } }, context: CTX });
244+
245+
expect(seen.findAst?.where?.close_date?.$lt).toMatch(/^\d{4}-\d{2}-\d{2}$/);
246+
});
247+
248+
it.each(['{a}{b}', '{{x}}', '{}', 'a{b}c'])(
249+
'find(): %s is not ONE wrapped token and reaches the driver as a literal',
250+
async (value) => {
251+
// Pinned deliberately: recognition is one brace pair around the whole
252+
// value. These shapes are ordinary text and must not start throwing.
253+
const { driver, seen } = makeDriver();
254+
const ql = await makeEngine(driver);
255+
256+
await ql.find('deal', { where: { title: value }, context: CTX });
257+
258+
expect(seen.findAst?.where).toEqual({ title: value });
259+
},
260+
);
261+
262+
it('delete(multi): the write path refuses the same shape before deleting', async () => {
263+
// The verb-parity property #3810 established: one filter, one row set.
264+
// A widened `delete` is the most expensive place for a silent literal.
265+
const { driver } = makeDriver();
266+
const ql = await makeEngine(driver);
267+
268+
let err: any;
269+
try {
270+
await ql.delete('deal', {
271+
where: { close_date: { $lt: '{TODAY()}' } }, multi: true, context: CTX,
272+
} as any);
273+
} catch (e) {
274+
err = e;
275+
}
276+
expect(err?.name).toBe('UnknownFilterTokenError');
277+
expect(err?.code).toBe('FILTER_TOKEN_UNKNOWN');
278+
expect(err?.token).toBe('TODAY()');
279+
expect(driver.deleteMany).not.toHaveBeenCalled();
280+
expect(driver.delete).not.toHaveBeenCalled();
281+
});
282+
});
283+
183284
// ── Write path (framework#3810) ────────────────────────────────────────
184285
// The evaluator originally reached only find/findOne/count/aggregate, so the
185286
// SAME filter selected different rows depending on the verb: `find` matched

packages/spec/src/data/context-tokens.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,70 @@ describe('classifyFilterToken', () => {
116116
}
117117
});
118118
});
119+
120+
/**
121+
* #5586 — recognition is placeholder-by-INTENT, not by well-formed token name.
122+
*
123+
* Recognition used to be the token-NAME grammar (`[a-zA-Z0-9_]+`), so every
124+
* placeholder carrying a non-word character classified as `null` — "not a
125+
* placeholder" — and was handed to the data engine to be compared as a literal
126+
* string. That is the silent-wrong-rows outcome the classification exists to
127+
* abolish, and it hit the author backwards: `{TODAY}` was refused by name while
128+
* `{TODAY()}` quietly returned the wrong rows.
129+
*/
130+
describe('classifyFilterToken — brace-wrapped by intent (#5586)', () => {
131+
// Each of these is a shape an author reaches for when migrating from another
132+
// system's macro syntax: call syntax, kebab-case, natural language, a dotted
133+
// path. All four used to classify as `null`.
134+
it.each([
135+
['{TODAY()}', 'TODAY()'],
136+
['{current-user-id}', 'current-user-id'],
137+
['{30 days ago}', '30 days ago'],
138+
['{user.id}', 'user.id'],
139+
])('%s is an UNKNOWN token, not a literal', (value, token) => {
140+
expect(classifyFilterToken(value)).toEqual({ kind: 'unknown', token, suggestion: undefined });
141+
});
142+
143+
it('recognises the `${…}` prefix variant of a wide shape too', () => {
144+
expect(classifyFilterToken('${TODAY()}')).toEqual({
145+
kind: 'unknown',
146+
token: 'TODAY()',
147+
suggestion: undefined,
148+
});
149+
});
150+
151+
it('does not tolerate padding — the canonical spelling carries none', () => {
152+
// Refused loudly rather than trimmed: a lenient consumer here would make
153+
// `{ current_user_id }` legal on this surface and illegal on every other
154+
// one that spells the vocabulary out.
155+
expect(classifyFilterToken('{ current_user_id }')).toEqual({
156+
kind: 'unknown',
157+
token: ' current_user_id ',
158+
suggestion: undefined,
159+
});
160+
});
161+
162+
it('still resolves the canonical spelling and still refuses the near miss', () => {
163+
// Regression guards for the two poles the widening sits between.
164+
expect(classifyFilterToken('{today}')).toEqual({ kind: 'date-macro', token: 'today' });
165+
expect(classifyFilterToken('{TODAY}')).toEqual({
166+
kind: 'unknown',
167+
token: 'TODAY',
168+
suggestion: undefined,
169+
});
170+
});
171+
172+
// The widening is "ONE whole pair of braces around the WHOLE value". These
173+
// shapes are not that, and each stays a literal by explicit decision rather
174+
// than by emergent regex behaviour.
175+
it.each([
176+
['a{b}c', 'braces mid-value — ordinary text that happens to contain a brace pair'],
177+
['{a}{b}', 'two pairs — not one wrapped token'],
178+
['{{x}}', 'nested pairs — not one wrapped token'],
179+
['{}', 'empty braces — there is no token to name in a diagnostic'],
180+
['{a}b', 'a wrapped head with a trailing literal'],
181+
['x{a}', 'a literal head with a wrapped tail'],
182+
])('%s stays a plain literal (%s)', (value) => {
183+
expect(classifyFilterToken(value)).toBeNull();
184+
});
185+
});

0 commit comments

Comments
 (0)