From bd41a1f8ba559f347724ca390162f40da76b9840 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 09:30:54 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(spec,core):=20recognise=20a=20filter=20?= =?UTF-8?q?placeholder=20by=20intent=20=E2=80=94=20any=20brace-wrapped=20v?= =?UTF-8?q?alue=20refuses=20loudly=20(#5586)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01MwoubC3jL271FYt9rGXwxb --- .changeset/filter-token-brace-intent.md | 57 ++++++++++ packages/core/src/utils/filter-tokens.test.ts | 77 +++++++++++++ packages/core/src/utils/filter-tokens.ts | 12 +++ .../objectql/src/engine-filter-tokens.test.ts | 101 ++++++++++++++++++ packages/spec/src/data/context-tokens.test.ts | 67 ++++++++++++ packages/spec/src/data/context-tokens.zod.ts | 52 ++++++++- 6 files changed, 365 insertions(+), 1 deletion(-) create mode 100644 .changeset/filter-token-brace-intent.md diff --git a/.changeset/filter-token-brace-intent.md b/.changeset/filter-token-brace-intent.md new file mode 100644 index 0000000000..6a5a9ca4ba --- /dev/null +++ b/.changeset/filter-token-brace-intent.md @@ -0,0 +1,57 @@ +--- +"@objectstack/spec": patch +"@objectstack/core": patch +"@objectstack/lint": patch +--- + +fix(spec,core): a filter placeholder is recognised by INTENT — `{TODAY()}` refuses loudly instead of comparing as a literal (#5586) + +`UnknownFilterTokenError` had a hole exactly where authors fall in. Recognition +used the token-NAME grammar `/^\$?\{([a-zA-Z0-9_]+)\}$/`, so any placeholder +carrying a **non-word character** classified as "not a placeholder at all" and +was handed to the driver verbatim, to be compared as a literal string — the +silent-wrong-result failure the diagnostic exists to abolish. + +The failure was inverted against the author. Measured on 17.0.0-rc.2 against a +four-row fixture: + +| filter value | before | | +|---|---|---| +| `due_date < '{today}'` | 2 rows | correct — the two overdue rows | +| `due_date < '{TODAY}'` | throws `UnknownFilterTokenError` | diagnostic working | +| `due_date < '{TODAY()}'` | **4 rows** | diagnostic bypassed — literal string compare, and `'2026-…' < '{'` in lexicographic order swallowed a row due a week later | + +So misspelling `{today}` as `{TODAY}` was reported by name, while misspelling it +as `{TODAY()}` returned the wrong rows in silence — and the parenthesised, +kebab-case, natural-language and dotted spellings (`{TODAY()}`, +`{current-user-id}`, `{30 days ago}`, `{user.id}`) are precisely what an author +migrating from another system's macro syntax writes first. + +**Both directions of the behaviour change:** + +- **Previously silent, now refuses loudly** — a filter value that is entirely + brace-wrapped and outside the vocabulary now throws `UnknownFilterTokenError` + (`code: FILTER_TOKEN_UNKNOWN`, `status: 400`) on the ObjectQL read and write + paths and the analytics dataset executor, and is reported as + `filter-token-unknown` by `objectstack build` / `validate` / `lint`. Before, + it reached the data engine and compared as text. +- **Unchanged** — `{today}` / `{current_user_id}` still resolve; `{TODAY}` still + refuses with the same identity; a value that merely *contains* braces + (`'acme {x} deal'`), or is not ONE pair around the whole value (`{a}{b}`, + `{{x}}`, `{}`), is still an ordinary literal and still reaches the driver + untouched. + +Recognition and vocabulary are now two named grammars rather than one: +`FILTER_TOKEN_WRAPPED_RE` (`/^\$?\{([^{}]+)\}$/`) answers "did the author mean a +placeholder", and `isContextToken` / `isDateMacroToken` answer "is it in the +vocabulary". Wide in, strict out. No escape hatch for a literal `{…}` comparand +ships with this: a repo-wide measurement across structured metadata, examples, +seed data and fixtures found zero legitimate consumers comparing a +brace-wrapped literal, and an escape syntax is a public micro-contract that can +be added the day one shows up. + +Flow templates are unaffected. `interpolateFilter` in +`@objectstack/service-automation` already recognised the same wide shape and +resolves `{record.id}` / `{TODAY() + 30}` from flow variables **before** the +filter reaches ObjectQL; its hand-off to the engine is keyed on the token +vocabulary (`isKnownFilterToken`), which this change does not touch. diff --git a/packages/core/src/utils/filter-tokens.test.ts b/packages/core/src/utils/filter-tokens.test.ts index 623d89cae9..5d6a6b592e 100644 --- a/packages/core/src/utils/filter-tokens.test.ts +++ b/packages/core/src/utils/filter-tokens.test.ts @@ -232,6 +232,83 @@ describe('resolveFilterTokens — tree walk', () => { }); }); +/** + * #5586 — a placeholder carrying a NON-WORD character used to bypass the + * diagnostic entirely. + * + * Recognition was the token-NAME grammar, so `{TODAY()}` classified as "not a + * placeholder", was handed to the driver verbatim and compared as a literal + * string. Measured on 17.0.0-rc.2 against a four-row fixture: `due_date < + * '{today}'` returned the 2 genuinely overdue rows, `due_date < '{TODAY()}'` + * returned all 4 — lexicographic string order puts every `'2026-…'` before + * `'{'`, so the window silently swallowed a row due a week later. + * + * The refusal is asserted on the ADR-0112 envelope (`code` + `status`) plus the + * offending token, never on the bare fact of a throw: the resolver already + * throws for other reasons, so a throw-only assertion cannot tell "refused with + * the right identity" from "blew up somewhere else". + */ +describe('resolveFilterTokens — non-word placeholder shapes refuse loudly (#5586)', () => { + const ctx = { now: NOW, userId: 'usr_1', orgId: 'org_9' }; + + const shapes: Array<[label: string, value: string, token: string]> = [ + ['call syntax (Salesforce/Excel migrants)', '{TODAY()}', 'TODAY()'], + ['kebab-case', '{current-user-id}', 'current-user-id'], + ['natural language', '{30 days ago}', '30 days ago'], + ['dotted path', '{user.id}', 'user.id'], + ['the `${…}` prefix variant', '${TODAY()}', 'TODAY()'], + ]; + + it.each(shapes)('%s — %s refuses with the full error identity', (_label, value, token) => { + let err: unknown; + try { + resolveFilterTokens({ due_date: { $lt: value } }, ctx); + } catch (e) { + err = e; + } + expect(err).toBeInstanceOf(UnknownFilterTokenError); + const e = err as UnknownFilterTokenError; + expect(e.name).toBe('UnknownFilterTokenError'); + // ADR-0112 envelope: the caller's filter is malformed, the server is fine. + expect(e.code).toBe('FILTER_TOKEN_UNKNOWN'); + expect(e.status).toBe(400); + // The author has to see what THEY wrote, not a normalised paraphrase. + expect(e.token).toBe(token); + expect(e.message).toContain(`{${token}}`); + }); + + it('still resolves the canonical spelling — the widening did not eat `{today}`', () => { + expect(resolveFilterTokens({ due_date: { $lt: '{today}' } }, ctx)) + .toEqual({ due_date: { $lt: '2026-07-15' } }); + }); + + it('still refuses the word-character near miss `{TODAY}`', () => { + // The shape that ALREADY worked. It is the control: if this ever goes + // quiet, the widening has replaced the diagnostic instead of extending it. + let err: unknown; + try { + resolveFilterTokens({ due_date: { $lt: '{TODAY}' } }, ctx); + } catch (e) { + err = e; + } + expect(err).toBeInstanceOf(UnknownFilterTokenError); + expect((err as UnknownFilterTokenError).token).toBe('TODAY'); + expect((err as UnknownFilterTokenError).code).toBe('FILTER_TOKEN_UNKNOWN'); + }); + + // Decided, not emergent: recognition is ONE brace pair around the WHOLE + // value. Anything else is ordinary text and reaches the driver untouched — + // that is what keeps `titleFormat`-style strings and human prose out of the + // rule, and it is the property that holds false positives at zero. + it.each(['acme {x} deal', '{a}{b}', '{{x}}', '{}', '{a}b', 'x{a}'])( + '%s is a literal and passes through unchanged', + (value) => { + const filter = { title: value }; + expect(resolveFilterTokens(filter, ctx)).toBe(filter); + }, + ); +}); + describe('filterTokenContextFrom', () => { it('maps ExecutionContext onto the resolver inputs', () => { expect( diff --git a/packages/core/src/utils/filter-tokens.ts b/packages/core/src/utils/filter-tokens.ts index 4a36a6e6f3..f616aa9771 100644 --- a/packages/core/src/utils/filter-tokens.ts +++ b/packages/core/src/utils/filter-tokens.ts @@ -48,6 +48,18 @@ * through is precisely the silent-zero bug this module exists to end, so it is * a hard error carrying the near-miss suggestion (`{current_user}` → * `{current_user_id}`). Values that merely CONTAIN braces are left untouched. + * + * "Entirely `{something}`" means ANY character between the braces (#5586). + * Until then the recognition grammar was the token-NAME grammar + * (`[a-zA-Z0-9_]+`), so a placeholder carrying a non-word character — + * `{TODAY()}`, `{current-user-id}`, `{30 days ago}`, `{user.id}` — was not + * recognised as a token at all and fell straight through to the literal + * comparison this module exists to abolish. The failure was inverted against + * the author: `{TODAY}` threw (diagnostic working), `{TODAY()}` returned rows + * (diagnostic bypassed) — and the parenthesised, kebab-case and + * natural-language spellings are exactly what an author migrating from another + * system's macro syntax reaches for first. See `FILTER_TOKEN_WRAPPED_RE` in + * `@objectstack/spec`. */ import { diff --git a/packages/objectql/src/engine-filter-tokens.test.ts b/packages/objectql/src/engine-filter-tokens.test.ts index 1cd2c08185..e5d59f358d 100644 --- a/packages/objectql/src/engine-filter-tokens.test.ts +++ b/packages/objectql/src/engine-filter-tokens.test.ts @@ -180,6 +180,107 @@ describe('engine filter placeholders (framework#3582)', () => { expect(seen.findAst?.where).toEqual({ title: 'acme {x} deal', owner: 'usr_2' }); }); + /** + * #5586 — the read path is where the bypass was measured, so it is pinned + * here and not only at the resolver. + * + * A placeholder carrying a non-word character was not recognised as a token, + * so it rode the AST all the way to the driver and was compared as a literal + * string. On the issue's four-row fixture `due_date < '{TODAY()}'` returned + * 4 rows where `due_date < '{today}'` returned the 2 genuinely overdue ones + * — wrong rows, no error, indistinguishable from a correct answer. + */ + describe('non-word placeholder shapes refuse before the driver (#5586)', () => { + it.each([ + ['{TODAY()}', 'TODAY()'], + ['{current-user-id}', 'current-user-id'], + ['{30 days ago}', '30 days ago'], + ['{user.id}', 'user.id'], + ['${TODAY()}', 'TODAY()'], + ])('find(): %s throws and the driver is never reached', async (value, token) => { + const { driver } = makeDriver(); + const ql = await makeEngine(driver); + + let err: any; + try { + await ql.find('deal', { where: { close_date: { $lt: value } }, context: CTX }); + } catch (e) { + err = e; + } + + // The specific identity, not the bare fact of a rejection: the point of + // the fix is WHICH error the caller gets, and a `rejects.toThrow()` here + // would stay green on a driver-level blow-up. + expect(err?.name).toBe('UnknownFilterTokenError'); + expect(err?.code).toBe('FILTER_TOKEN_UNKNOWN'); + expect(err?.status).toBe(400); + expect(err?.token).toBe(token); + expect(err?.message).toContain(`{${token}}`); + expect(driver.find).not.toHaveBeenCalled(); + }); + + it('find(): the word-character near miss `{TODAY}` still throws', async () => { + // Control for the widening: the shape that already refused must keep + // refusing with the same identity. + const { driver } = makeDriver(); + const ql = await makeEngine(driver); + + let err: any; + try { + await ql.find('deal', { where: { close_date: { $lt: '{TODAY}' } }, context: CTX }); + } catch (e) { + err = e; + } + expect(err?.name).toBe('UnknownFilterTokenError'); + expect(err?.token).toBe('TODAY'); + expect(driver.find).not.toHaveBeenCalled(); + }); + + it('find(): `{today}` still resolves to a concrete date', async () => { + const { driver, seen } = makeDriver(); + const ql = await makeEngine(driver); + + await ql.find('deal', { where: { close_date: { $lt: '{today}' } }, context: CTX }); + + expect(seen.findAst?.where?.close_date?.$lt).toMatch(/^\d{4}-\d{2}-\d{2}$/); + }); + + it.each(['{a}{b}', '{{x}}', '{}', 'a{b}c'])( + 'find(): %s is not ONE wrapped token and reaches the driver as a literal', + async (value) => { + // Pinned deliberately: recognition is one brace pair around the whole + // value. These shapes are ordinary text and must not start throwing. + const { driver, seen } = makeDriver(); + const ql = await makeEngine(driver); + + await ql.find('deal', { where: { title: value }, context: CTX }); + + expect(seen.findAst?.where).toEqual({ title: value }); + }, + ); + + it('delete(multi): the write path refuses the same shape before deleting', async () => { + // The verb-parity property #3810 established: one filter, one row set. + // A widened `delete` is the most expensive place for a silent literal. + const { driver } = makeDriver(); + const ql = await makeEngine(driver); + + let err: any; + try { + await ql.delete('deal', { + where: { close_date: { $lt: '{TODAY()}' } }, multi: true, context: CTX, + } as any); + } catch (e) { + err = e; + } + expect(err?.name).toBe('UnknownFilterTokenError'); + expect(err?.code).toBe('FILTER_TOKEN_UNKNOWN'); + expect(err?.token).toBe('TODAY()'); + expect(driver.deleteMany).not.toHaveBeenCalled(); + expect(driver.delete).not.toHaveBeenCalled(); + }); + }); + // ── Write path (framework#3810) ──────────────────────────────────────── // The evaluator originally reached only find/findOne/count/aggregate, so the // SAME filter selected different rows depending on the verb: `find` matched diff --git a/packages/spec/src/data/context-tokens.test.ts b/packages/spec/src/data/context-tokens.test.ts index 55987d87c8..e9a1f405ef 100644 --- a/packages/spec/src/data/context-tokens.test.ts +++ b/packages/spec/src/data/context-tokens.test.ts @@ -116,3 +116,70 @@ describe('classifyFilterToken', () => { } }); }); + +/** + * #5586 — recognition is placeholder-by-INTENT, not by well-formed token name. + * + * Recognition used to be the token-NAME grammar (`[a-zA-Z0-9_]+`), so every + * placeholder carrying a non-word character classified as `null` — "not a + * placeholder" — and was handed to the data engine to be compared as a literal + * string. That is the silent-wrong-rows outcome the classification exists to + * abolish, and it hit the author backwards: `{TODAY}` was refused by name while + * `{TODAY()}` quietly returned the wrong rows. + */ +describe('classifyFilterToken — brace-wrapped by intent (#5586)', () => { + // Each of these is a shape an author reaches for when migrating from another + // system's macro syntax: call syntax, kebab-case, natural language, a dotted + // path. All four used to classify as `null`. + it.each([ + ['{TODAY()}', 'TODAY()'], + ['{current-user-id}', 'current-user-id'], + ['{30 days ago}', '30 days ago'], + ['{user.id}', 'user.id'], + ])('%s is an UNKNOWN token, not a literal', (value, token) => { + expect(classifyFilterToken(value)).toEqual({ kind: 'unknown', token, suggestion: undefined }); + }); + + it('recognises the `${…}` prefix variant of a wide shape too', () => { + expect(classifyFilterToken('${TODAY()}')).toEqual({ + kind: 'unknown', + token: 'TODAY()', + suggestion: undefined, + }); + }); + + it('does not tolerate padding — the canonical spelling carries none', () => { + // Refused loudly rather than trimmed: a lenient consumer here would make + // `{ current_user_id }` legal on this surface and illegal on every other + // one that spells the vocabulary out. + expect(classifyFilterToken('{ current_user_id }')).toEqual({ + kind: 'unknown', + token: ' current_user_id ', + suggestion: undefined, + }); + }); + + it('still resolves the canonical spelling and still refuses the near miss', () => { + // Regression guards for the two poles the widening sits between. + expect(classifyFilterToken('{today}')).toEqual({ kind: 'date-macro', token: 'today' }); + expect(classifyFilterToken('{TODAY}')).toEqual({ + kind: 'unknown', + token: 'TODAY', + suggestion: undefined, + }); + }); + + // The widening is "ONE whole pair of braces around the WHOLE value". These + // shapes are not that, and each stays a literal by explicit decision rather + // than by emergent regex behaviour. + it.each([ + ['a{b}c', 'braces mid-value — ordinary text that happens to contain a brace pair'], + ['{a}{b}', 'two pairs — not one wrapped token'], + ['{{x}}', 'nested pairs — not one wrapped token'], + ['{}', 'empty braces — there is no token to name in a diagnostic'], + ['{a}b', 'a wrapped head with a trailing literal'], + ['x{a}', 'a literal head with a wrapped tail'], + ])('%s stays a plain literal (%s)', (value) => { + expect(classifyFilterToken(value)).toBeNull(); + }); +}); diff --git a/packages/spec/src/data/context-tokens.zod.ts b/packages/spec/src/data/context-tokens.zod.ts index bda6624af2..4d028457f2 100644 --- a/packages/spec/src/data/context-tokens.zod.ts +++ b/packages/spec/src/data/context-tokens.zod.ts @@ -100,9 +100,51 @@ export function isContextToken(token: string): boolean { * Match the **wrapped** form a filter author actually writes — * `{current_user_id}` or `${current_user_id}`. Shares the date-macro * grammar so a single walk over a filter tree can classify both. + * + * This is the **well-formed token NAME** grammar: what a spelling has to look + * like before it can be a member of either vocabulary. It is deliberately NOT + * the grammar that decides whether a value is a placeholder *attempt* — see + * {@link FILTER_TOKEN_WRAPPED_RE}. */ export const CONTEXT_TOKEN_WRAPPED_RE = DATE_MACRO_WRAPPED_RE; +/** + * Match a filter value that is a placeholder **by intent**: entirely wrapped + * in one pair of braces, whatever the characters inside (#5586). + * + * # Why this is wider than {@link CONTEXT_TOKEN_WRAPPED_RE} + * + * Recognition and vocabulary are two different questions, and conflating them + * left the diagnostic with a hole exactly where authors fall in. While + * recognition used the token-NAME grammar (`[a-zA-Z0-9_]+`), any placeholder + * carrying a non-word character — `{TODAY()}`, `{current-user-id}`, + * `{30 days ago}`, `{user.id}` — was not classified as a token at all. It was + * therefore handed to the driver verbatim and compared as a **literal string**: + * the silent-wrong-result mode this vocabulary exists to abolish. The failure + * was inverted against the author, too — misspelling `{today}` as `{TODAY}` + * produced a loud `UnknownFilterTokenError`, while misspelling it as + * `{TODAY()}` produced *rows*, and on a string comparison the wrong ones + * (`'2026-…' < '{'` in lexicographic order, so a `<` window silently gained + * every future-dated row). + * + * The shapes that leaked are precisely the ones an author migrating from + * another system's macro syntax writes first: `TODAY()` (Salesforce/Excel-style + * call syntax), kebab-case, natural language, dotted paths. + * + * So: wide in, strict out. Anything fully brace-wrapped is read as "the author + * meant a placeholder", and the *vocabulary* check then either resolves it or + * refuses it by name. No author writes a filter comparand whose intended + * literal value is the six characters `{foo}`; a value that merely CONTAINS + * braces (`'acme {x} deal'`) is untouched, as are `{a}{b}` and `{{x}}`, which + * are not one wrapped token. + * + * The flow template engine's filter position already used exactly this shape + * (`interpolateFilter` in `@objectstack/service-automation`, #3810) to decide + * "is this string one whole token?" before consulting the vocabulary — this + * aligns the platform diagnostic with the recognition rule that surface had. + */ +export const FILTER_TOKEN_WRAPPED_RE = /^\$?\{([^{}]+)\}$/; + /** Strict zod schema for the **token name** (the bit inside `{}`). */ export const ContextTokenSchema = z .string() @@ -185,6 +227,14 @@ export function isKnownFilterToken(token: string): boolean { * * Distinguishing `unknown` from "not a placeholder" is what lets lint stay * quiet about ordinary values while still catching `{current_user}`. + * + * Recognition is {@link FILTER_TOKEN_WRAPPED_RE} — placeholder by INTENT, not + * by well-formedness (#5586). A fully brace-wrapped value whose inside is not + * a legal token name (`{TODAY()}`, `{user.id}`, `{30 days ago}`) is `unknown`, + * NOT `null`: `null` would send it to the data engine to be compared as a + * literal string, which is the silent-wrong-rows outcome this classification + * exists to prevent. The token reported is the raw text between the braces, so + * the caller's error names exactly what the author wrote. */ export function classifyFilterToken( value: unknown, @@ -194,7 +244,7 @@ export function classifyFilterToken( | { kind: 'unknown'; token: string; suggestion?: ContextToken } | null { if (typeof value !== 'string') return null; - const m = value.match(CONTEXT_TOKEN_WRAPPED_RE); + const m = value.match(FILTER_TOKEN_WRAPPED_RE); if (!m) return null; const token = m[1]; if (isContextToken(token)) return { kind: 'context', token: token as ContextToken }; From 403ca6dfdf4b83da3e8d27c7ae2ef26e61b75900 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 10:09:37 +0000 Subject: [PATCH 2/2] chore(spec): regenerate api-surface snapshot for FILTER_TOKEN_WRAPPED_RE (#5586) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MwoubC3jL271FYt9rGXwxb --- packages/spec/api-surface/data.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/spec/api-surface/data.json b/packages/spec/api-surface/data.json index c3afbd77aa..7735939642 100644 --- a/packages/spec/api-surface/data.json +++ b/packages/spec/api-surface/data.json @@ -229,6 +229,7 @@ "FILTER_OPERATORS (const)", "FILTER_TEXT_CASES (const)", "FILTER_TEXT_ROWS (const)", + "FILTER_TOKEN_WRAPPED_RE (const)", "FeedFilterMode (type)", "FeedItemType (type)", "Field (type)",