diff --git a/.changeset/mixed-wrapper-refusal.md b/.changeset/mixed-wrapper-refusal.md new file mode 100644 index 0000000000..087e913888 --- /dev/null +++ b/.changeset/mixed-wrapper-refusal.md @@ -0,0 +1,43 @@ +--- +'@objectstack/service-analytics': patch +--- + +fix(analytics): a field constraint mixing `$` operators with non-`$` sibling keys is refused (400 `INVALID_FILTER`), not silently narrowed to its operators + +**Observable behaviour change.** A `where` field wrapper that carries `$`-operator +keys and non-`$` keys at once used to compile its operators and silently DROP +every non-`$` sibling. It is now refused with `INVALID_FILTER` / 400, the +envelope every other refusal at this door already carries. Ruled Option A +(refuse) on #6444, 2026-08-08; Option B (flattening the siblings as nested +paths) was rejected because it would compile the likely-real cause — a dropped +`$` — into a predicate on a non-existent member such as `amount.gte`. + +| `where` | used to normalize to | reading | +|---|---|---| +| `{d: {$eq: 1, nested: 'x'}}` | `d equals [1]` | the `nested` conjunct vanished in silence | +| `{amount: {gte: 10, $lte: 20}}` | `amount lte 20` | the missing-`$` typo: the lower bound silently gone | +| `{$not: {d: {$null: true, nested: 'x'}}}` | `NOT(d set AND d notSet)` | a contradiction that negates to TRUE — every row | + +Every row WIDENED the query — a dropped conjunct returns rows the author +excluded, with nothing to read (the #3650 family this module refuses everywhere +else). Unlike #6386's `undefined` comparand, this shape survives JSON, so it can +sit in stored dashboard / report / dataset metadata as well as in-process +callers of `AnalyticsService.query({ where })`. + +**What to change if this refuses your filter.** The message names the offending +key(s) and both repairs, because the shape has two readings this door cannot +tell apart: + +- an operator missing its `$` was meant → spell it with the prefix + (`gte` → `$gte`: `{ "amount": { "$gte": 10, "$lte": 20 } }`); +- a nested-relation member was meant → give it a wrapper of its own with no `$` + siblings (`{ "d": { "nested": "x" } }` compiles to the member `d.nested`) and + AND it with the operator constraint explicitly via `$and`. + +⛔ **The two pure shapes do not move.** A wrapper that is all `$`-operators +compiles exactly as before (`{amount: {$gte: 10, $lte: 20}}` stays the AND of +its bounds), and a wrapper that is all non-`$` keys keeps flattening to the +dotted member (`{d: {nested: 'x'}}` → `d.nested`). `$null` / `$exists` flag +semantics, the `null` comparand rulings (#5332 / #5526) and the sibling door +`read-scope-sql.ts` — which has always failed closed on this shape — are +untouched. diff --git a/packages/services/service-analytics/src/__tests__/filter-normalizer-mixed-wrapper.test.ts b/packages/services/service-analytics/src/__tests__/filter-normalizer-mixed-wrapper.test.ts new file mode 100644 index 0000000000..8b1be75736 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/filter-normalizer-mixed-wrapper.test.ts @@ -0,0 +1,354 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6444, ruled Option A on 2026-08-08] A field wrapper mixing `$`-operator + * keys with non-`$` sibling keys is REFUSED — `INVALID_FILTER` / 400 — and the + * two pure shapes on either side of it do not move. + * + * ## What was wrong + * + * The value-independent sibling of #6386, in the same function. `fieldLeaves`'s + * operator arm iterated `opKeys` only and returned, and the nested-relation + * flatten sits after that early return — so with even one `$` key in the + * wrapper, every non-`$` sibling was silently dropped, whatever its value. + * Measured on `origin/main` (`1a53a0253`) by calling + * `normalizeAnalyticsFilterTree({ where })` directly: + * + * | `where` | normalized to | reading | + * |---|---|---| + * | `{d: {$eq: 1, nested: 'x'}}` | `d equals [1]` | the `nested` conjunct vanished in silence | + * | `{d: {$eq: 1, nested: undefined}}` | `d equals [1]` | same — value-independent, unlike #6386 | + * | `{amount: {gte: 10, $lte: 20}}` | `amount lte 20` | the missing-`$` typo: the LOWER BOUND silently gone | + * | `{$not: {d: {$eq: 1, nested: 'x'}}}` | `NOT(d set AND d = 1)` | the sibling vanished INSIDE the negation | + * | `{$not: {d: {$null: true, nested: 'x'}}}` | `NOT(d set AND d notSet)` | a CONTRADICTION that negates to TRUE — every row | + * + * Every row WIDENS — a dropped conjunct does not narrow the query (#3650 / + * #4128), the failure mode this module's own `MONGO_TO_CUBE_OP` miss-branch + * comment forbids. The last row is the strangest: `nullGuardForFieldSpec` + * judged the wrapper while the sibling still existed (`requireValue`), the + * sibling then vanished in `fieldLeaves`, and the surviving guard was + * contradictory — so the negation returned the WHOLE dataset. + * + * ## The ruling, and what the message owes (#6444, 2026-08-08) + * + * Option A — refuse, through the module's one envelope (#5352). Option B + * (flatten the siblings as nested paths) was rejected: it would compile the + * likely-real cause — a dropped `$` — into a predicate on a non-existent + * member `amount.gte`. Because the refused shape has TWO legitimate repairs + * answering two intents the module cannot tell apart, the message must name + * the offending non-`$` key(s) and show BOTH rewrites: the operator spelling + * (`gte` → `$gte`) and the nested-relation form — asserted below as the + * message contract, not prose. + * + * ## The blocks, and which one is the change + * + * `the mixed wrapper is ONE refusal` is the change: run it against pre-#6444 + * code and every row fails, because every row COMPILES (dropping siblings). + * + * `the two pure shapes do not move` is the risk. The gate must move the + * refusal set by EXACTLY the mixed shape: all-`$` wrappers keep compiling, + * all-non-`$` wrappers keep flattening to dotted members. An over-reaching + * gate shows up there as a throw. + * + * `the #5146 rewrite cannot swallow the wrapper` is the gate-side question, + * same as #6386's: the gate sits in `fieldLeaves`, downstream of + * `nullSafeNegationOperand`. For a MIXED wrapper the carry-through is + * structural: a non-`$` key never satisfies `operatorIsNullTotal`, so + * `nullGuardForFieldSpec` never answers `none` for one — the disposition is + * always `requireValue`/`allowNull`, both of which push the spec by + * reference, so the gate always sees the author's wrapper. + * + * ## Reverse verification — direction predicted BEFORE running + * + * Ordinary direction, one knob (the `assertUnmixedFieldWrapper` call in + * `fieldLeaves`). Predicted with the call removed: every refusal row in this + * file goes red (the mixed shapes compile again, siblings dropped), the + * flipped pin in `filter-normalizer-undefined-comparand.test.ts` goes red, + * and the #6444 ledger row in `filter-refusal-envelope.test.ts` goes red — + * while both pure-shape control blocks and the #6386/`null` groups stay + * green (they never depended on this gate). One deliberate exception stays + * green in THIS file too: `{d: {$eq: undefined, nested: 'x'}}` is refused by + * the #6386 gate, not this one. Measured counts are in the PR body. + * + * ## Scope, so a later reader does not "finish the job" + * + * ⛔ `read-scope-sql.ts` is the sibling door; it has ALWAYS failed closed on + * this shape (`compileField`'s non-`$`-key check) and is not touched — this + * change makes the two doors give one answer, in this door's own envelope + * (400: the `where` is caller input; that door compiles a platform artifact + * and answers 500). + * ⛔ `$null` / `$exists` flag semantics (#5347 / #5369 / #6387), `comparand()` + * (#5526) and the null-predicate identity (#5332) are RULED elsewhere and + * untouched; the `null` control group lives in + * `filter-normalizer-undefined-comparand.test.ts` and did not move. + * ⛔ The other compiler faces (#5930's five) are out of scope per the ruling; + * the per-face statement is in PR #6444's body per the #6410 checklist. + */ + +import { describe, it, expect } from 'vitest'; +import { normalizeAnalyticsFilterTree } from '../strategies/filter-normalizer.js'; + +/** The ADR-0112 fields a refusal must carry. */ +interface FilterRefusal extends Error { + code?: unknown; + status?: unknown; +} + +function refusalFor(where: unknown): FilterRefusal | undefined { + try { + normalizeAnalyticsFilterTree({ where }); + return undefined; + } catch (e) { + return e as FilterRefusal; + } +} + +function treeFor(where: unknown): unknown { + return normalizeAnalyticsFilterTree({ where }); +} + +/** + * The mixed shapes, one per position the wrapper can sit in. `field` is the + * member the refusal must name (for a nested mix, the DOTTED member — + * `fieldLeaves` recurses before the gate sees it). `opKeys`/`nonOpKeys` are + * the two lists the message quotes, in wrapper key order. `wasReadAs` is the + * pre-fix normalisation — recorded so each row says what it protects against. + */ +const MIXED: Array<{ + name: string; + where: unknown; + field: string; + opKeys: string[]; + nonOpKeys: string[]; + wasReadAs: string; +}> = [ + { + name: '① operator + nested member in one wrapper', + where: { d: { $eq: 1, nested: 'x' } }, + field: 'd', + opKeys: ['$eq'], + nonOpKeys: ['nested'], + wasReadAs: "d equals [1] — `nested` dropped in silence", + }, + { + name: '② the same mix with an undefined sibling VALUE (value-independent)', + where: { d: { $eq: 1, nested: undefined } }, + field: 'd', + opKeys: ['$eq'], + nonOpKeys: ['nested'], + wasReadAs: 'd equals [1] — the drop never read the value (#6386 pinned this)', + }, + { + name: '③ the canonical agent typo — an operator missing its $', + where: { amount: { gte: 10, $lte: 20 } }, + field: 'amount', + opKeys: ['$lte'], + nonOpKeys: ['gte'], + wasReadAs: 'amount lte 20 — the LOWER BOUND silently gone', + }, + { + name: '④ a $between beside a stray member', + where: { d: { $between: [1, 5], nested: 'x' } }, + field: 'd', + opKeys: ['$between'], + nonOpKeys: ['nested'], + wasReadAs: 'd gte [1] AND d lte [5] — the range survived, the member did not', + }, + { + name: '⑤ a $null flag beside a stray member', + where: { d: { $null: true, nested: 'x' } }, + field: 'd', + opKeys: ['$null'], + nonOpKeys: ['nested'], + wasReadAs: 'd notSet — the flag survived, the member did not', + }, + { + name: '⑥ the mix one relation DOWN, refused on the DOTTED member', + where: { profile: { verified: { $eq: 1, extra: 'x' } } }, + field: 'profile.verified', + opKeys: ['$eq'], + nonOpKeys: ['extra'], + wasReadAs: 'profile.verified equals [1]', + }, + { + name: '⑦ inside a $and branch', + where: { $and: [{ d: { $eq: 1, nested: 'x' } }] }, + field: 'd', + opKeys: ['$eq'], + nonOpKeys: ['nested'], + wasReadAs: 'd equals [1]', + }, + { + name: '⑧ inside a $or branch — the branch quietly LOST a conjunct', + where: { $or: [{ d: { gte: 1, $lte: 2 } }, { stage: 'won' }] }, + field: 'd', + opKeys: ['$lte'], + nonOpKeys: ['gte'], + wasReadAs: "(d lte 2) OR (stage = 'won') — the branch widened, so the whole $or did", + }, +]; + +// ───────────────────────────────────────────────────────────────────────────── + +describe('[#6444] a mixed $/non-$ field wrapper is ONE refusal', () => { + for (const c of MIXED) { + it(`refuses ${c.name} (was: ${c.wasReadAs})`, () => { + const err = refusalFor(c.where); + expect(err, 'compiled instead of refusing — the #3650 sibling-drop widening is back').toBeInstanceOf(Error); + const message = String(err?.message); + // Names the field, the operator side, and — the ruling requirement — + // every offending non-$ key. + expect(message).toContain(`"${c.field}" mixes $-operator keys (${c.opKeys.join(', ')})`); + for (const k of c.nonOpKeys) expect(message).toContain(`"${k}"`); + // The envelope every refusal in this module carries since #5352: the + // `where` is caller input, so 400. A bare toThrow() would carry one bit + // where this defect has two (the ADR-0112 refusal-test rule). + expect(err?.code).toBe('INVALID_FILTER'); + expect(err?.status).toBe(400); + }); + } + + it('names EVERY offending sibling when there are several, not just the first', () => { + const err = refusalFor({ d: { $eq: 1, a: 1, b: 2 } }); + expect(err).toBeInstanceOf(Error); + const message = String(err?.message); + expect(message).toContain('non-$ sibling key(s) "a", "b"'); + // Both get the operator rewrite, so the author repairs the wrapper once. + expect(message).toContain('"a" → "$a"'); + expect(message).toContain('"b" → "$b"'); + expect(err?.code).toBe('INVALID_FILTER'); + expect(err?.status).toBe(400); + }); + + it('says ONE thing, differing only in the field and the two key lists (#5240)', () => { + // Erase the parts that legitimately vary and every message must be the + // same string. Restricted to the single-op/single-sibling rows so the + // erased skeletons are comparable; the multi-sibling wording is asserted + // in its own case above. Longest tokens first, so `"$gte"` is consumed + // before a bare `"gte"` replacement could split it. + const generic = MIXED.map((c) => { + const err = refusalFor(c.where); + // Load-bearing (measured on #6386's twin of this case): without it the + // wording check is VACUOUSLY green when nothing throws — every row maps + // to the same "undefined" string. + expect(err, `${c.name} did not refuse — the wording check would pass on nothing`).toBeInstanceOf(Error); + const k = c.nonOpKeys[0]; + return String(err?.message) + .split(`"${c.field}.${k}"`).join('"."') + .split(`(${c.opKeys.join(', ')})`).join('()') + .split(`"$${k}"`).join('"$"') + .split(`"${k}" → `).join('"" → ') + .split(`"${k}"`).join('""') + .split(`"${c.field}"`).join('""'); + }); + expect(new Set(generic).size, `expected one wording, got:\n${[...new Set(generic)].join('\n\n')}`).toBe(1); + }); + + it('shows BOTH legal rewrites — the two intents it cannot disambiguate (ruling req.)', () => { + const message = String(refusalFor({ amount: { gte: 10, $lte: 20 } })?.message); + // Intent 1 — an operator missing its $: the prefixed spelling, named + // key-by-key and shown in place. + expect(message).toContain('"gte" → "$gte"'); + expect(message).toContain('{ "amount": { "$gte": ... } }'); + // Intent 2 — a nested-relation member: a wrapper of its own, the dotted + // member it compiles to, and the explicit $and (one JSON object cannot + // spell the same field key twice). + expect(message).toContain('{ "amount": { "gte": ... } }'); + expect(message).toContain('"amount.gte"'); + expect(message).toContain('"$and"'); + // …and why it refuses rather than picking: the drop it replaces WIDENED. + expect(message).toContain('WIDENS'); + expect(message).toContain('read-scope-sql.ts'); + }); +}); + +describe('[#6444] the #5146 rewrite cannot swallow the wrapper', () => { + // The gate lives in `fieldLeaves`, DOWNSTREAM of `nullSafeNegationOperand`. + // A mixed wrapper reaches it because a non-$ key never satisfies + // `operatorIsNullTotal`, so `nullGuardForFieldSpec` never answers `none` for + // one — `requireValue` and `allowNull` both push the author's spec by + // REFERENCE. One case per rewrite path that can carry a mixed wrapper. + const REWRITE_PATHS: Array<{ name: string; where: unknown; field: string }> = [ + { + name: '`requireValue` — pushes {k: {$null: false}}, {k: spec}; spec kept by reference', + where: { $not: { d: { $eq: 1, nested: 'x' } } }, + field: 'd', + }, + { + name: 'a null-total operator whose SIBLING forces the guard (was the every-row contradiction)', + where: { $not: { d: { $null: true, nested: 'x' } } }, + field: 'd', + }, + { + name: '`$eq: null` beside a sibling — null-total op, still guarded because of the mix', + where: { $not: { d: { $eq: null, nested: 'x' } } }, + field: 'd', + }, + { + name: 'the nested-relation recursion in `guardFieldEntry`, which guards the DOTTED member', + where: { $not: { profile: { verified: { $eq: 1, extra: 2 } } } }, + field: 'profile.verified', + }, + ]; + + for (const c of REWRITE_PATHS) { + it(`throws rather than changing shape: ${c.name}`, () => { + const err = refusalFor(c.where); + expect(err, 'the rewrite swallowed the wrapper and the gate blessed the new shape').toBeInstanceOf(Error); + expect(String(err?.message)).toContain(`"${c.field}" mixes $-operator keys`); + expect(err?.code).toBe('INVALID_FILTER'); + expect(err?.status).toBe(400); + }); + } +}); + +describe('[#6444] the two pure shapes do not move', () => { + it('an ALL-non-$ wrapper still flattens to the dotted member (the nested-relation path)', () => { + // The pin the issue's own control row named: the ONLY reason the siblings + // were droppable is that this legitimate path sat after the early return. + expect(treeFor({ d: { nested: 'x' } })).toEqual({ + kind: 'leaf', member: 'd.nested', operator: 'equals', values: ['x'], + }); + expect(treeFor({ a: { b: { c: 1 } } })).toEqual({ + kind: 'leaf', member: 'a.b.c', operator: 'equals', values: [1], + }); + // A nested member carrying an OPERATOR wrapper (all-$ one level down) is + // legal on both levels and keeps compiling. + expect(treeFor({ profile: { verified: { $eq: true } } })).toEqual({ + kind: 'leaf', member: 'profile.verified', operator: 'equals', values: [true], + }); + }); + + it('an ALL-$ wrapper still compiles exactly as before, multi-operator included', () => { + expect(treeFor({ amount: { $gte: 10, $lte: 20 } })).toEqual({ + kind: 'and', + children: [ + { kind: 'leaf', member: 'amount', operator: 'gte', values: [10] }, + { kind: 'leaf', member: 'amount', operator: 'lte', values: [20] }, + ], + }); + expect(treeFor({ d: { $null: true } })).toEqual({ kind: 'leaf', member: 'd', operator: 'notSet', values: [] }); + expect(treeFor({ d: { $exists: false } })).toEqual({ kind: 'leaf', member: 'd', operator: 'notSet', values: [] }); + expect(treeFor({ stage: { $in: [] } })).toEqual({ kind: 'const', value: false }); + }); + + it('the neighbouring refusals keep their OWN wordings — the set moved by exactly one shape', () => { + // #5240's zero-operator refusal is disjoint by construction ({} has no + // keys of either kind) and must not borrow the mixed wording. + expect(String(refusalFor({ stage: {} })?.message)).toContain('zero operators'); + // #3948's vocabulary refusal still owns the all-$-but-unknown wrapper. + expect(String(refusalFor({ stage: { $sortOf: 'won' } })?.message)).toContain('Unsupported filter operator'); + }); + + it('a mixed wrapper whose $-comparand is undefined is refused by the #6386 gate first', () => { + // Measured ordering, pinned as a fact rather than a contract: + // `assertDefinedComparands` runs at `fieldLeaves`'s entry, this gate in the + // wrapper arm below it. Both refusals share the envelope, so the REST face + // answers 400 either way — which is why the ordering is allowed to be an + // implementation fact. + const err = refusalFor({ d: { $eq: undefined, nested: 'x' } }); + expect(String(err?.message)).toContain('comparand at "d".$eq is undefined'); + expect(err?.code).toBe('INVALID_FILTER'); + expect(err?.status).toBe(400); + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts b/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts index ef3ce6d5e0..d4ad611d0c 100644 --- a/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts @@ -469,17 +469,24 @@ describe('[#6386] what the sweep deliberately leaves alone', () => { } }); - it('a non-$ SIBLING of an operator is still dropped — a different defect, not this one', () => { - // `{d: {$eq: 1, nested: }}` ignores `nested` whatever its value, so - // this is not an `undefined` reading and is out of scope here. Pinned so the - // measurement is on the record rather than mistaken for coverage: filed - // separately per Prime Directive #10. - expect(treeFor({ d: { $eq: 1, nested: undefined } })).toEqual({ - kind: 'leaf', member: 'd', operator: 'equals', values: [1], - }); - expect(treeFor({ d: { $eq: 1, nested: 'x' } })).toEqual({ - kind: 'leaf', member: 'd', operator: 'equals', values: [1], - }); + it('a non-$ SIBLING of an operator is now REFUSED — the pin this case held flipped (#6444)', () => { + // ⚠️ FLIPPED, not deleted. Until #6444 this case PINNED the measurement + // that `{d: {$eq: 1, nested: }}` silently dropped `nested` — a + // different defect from #6386's, filed separately per Prime Directive #10 + // and ruled Option A (refuse) on 2026-08-08. The full position list, the + // two-rewrite message contract and the pure-shape control groups live in + // `filter-normalizer-mixed-wrapper.test.ts`; this case keeps the SAME two + // inputs the pin measured, so the record of what changed stays readable: + // both rows compiled to `d equals [1]` then, both refuse now, and the + // `undefined` row is why the refusal is value-INDEPENDENT — the drop never + // read the sibling's value, and neither does the gate that replaced it. + for (const where of [{ d: { $eq: 1, nested: undefined } }, { d: { $eq: 1, nested: 'x' } }]) { + const err = refusalFor(where); + expect(err, 'the non-$ sibling was silently dropped again — #6444 regressed').toBeInstanceOf(Error); + expect(String(err?.message)).toContain('"d" mixes $-operator keys ($eq) with non-$ sibling key(s) "nested"'); + expect(err?.code).toBe('INVALID_FILTER'); + expect(err?.status).toBe(400); + } }); it('every ACCEPTED shape the module already had still compiles identically', () => { diff --git a/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts b/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts index b4e6675409..9983999bc7 100644 --- a/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts @@ -154,6 +154,21 @@ const REFUSALS: Array<{ issueBullet: false, addedAfter5352: '#6386', }, + { + // [#6444] A field wrapper mixing $-operator keys with non-$ siblings. + // Before it, the non-$ siblings were silently DROPPED — `fieldLeaves`'s + // operator arm iterated `opKeys` only and returned, so + // `{amount: {gte: 10, $lte: 20}}` (the missing-$ typo) lost its lower + // bound with nothing to read. The full position list, the two-rewrite + // message contract and the pure-shape control groups live in + // `filter-normalizer-mixed-wrapper.test.ts`; this row exists so the + // envelope block below covers the eleventh site the way it covers the ten. + name: 'a mixed $/non-$ field wrapper (#6444)', + where: { amount: { gte: 10, $lte: 20 } }, + message: /"amount" mixes \$-operator keys \(\$lte\) with non-\$ sibling key\(s\) "gte"/, + issueBullet: false, + addedAfter5352: '#6444', + }, ]; /** @@ -290,7 +305,8 @@ describe('[#5352] every refusal carries the ADR-0112 envelope (INVALID_FILTER / ]); expect(REFUSALS.filter((c) => c.addedAfter5352).map((c) => `${c.name} · ${c.addedAfter5352}`)).toEqual([ 'an undefined comparand (#6386) · #6386', + 'a mixed $/non-$ field wrapper (#6444) · #6444', ]); - expect(REFUSALS).toHaveLength(10); + expect(REFUSALS).toHaveLength(11); }); }); diff --git a/packages/services/service-analytics/src/strategies/filter-normalizer.ts b/packages/services/service-analytics/src/strategies/filter-normalizer.ts index ce267e0d7c..dd65bf602c 100644 --- a/packages/services/service-analytics/src/strategies/filter-normalizer.ts +++ b/packages/services/service-analytics/src/strategies/filter-normalizer.ts @@ -286,6 +286,52 @@ * (#5526) keep their exact lowering, pinned as a control group in * `filter-normalizer-undefined-comparand.test.ts`. * + * # A field wrapper cannot MIX $-operators with non-$ members (#6444) + * + * The value-independent sibling of the #6386 defect, in the same function, ruled + * Option A (refuse) by the maintainer on 2026-08-08. A field constraint object + * that carries `$`-operator keys AND non-`$` keys at once used to compile its + * operators and silently DROP every non-`$` sibling — `fieldLeaves`'s + * `if (opKeys.length > 0) { …; return out; }` arm never looked at them, and the + * nested-relation flatten sits after that early return. Measured on + * `origin/main` (`1a53a0253`): + * + * | `where` | normalized to | reading | + * |---|---|---| + * | `{d: {$eq: 1, nested: 'x'}}` | `d equals [1]` | the `nested` conjunct vanished in silence | + * | `{amount: {gte: 10, $lte: 20}}` | `amount lte 20` | the missing-`$` typo: the LOWER BOUND silently gone | + * | `{$not: {d: {$null: true, nested: 'x'}}}` | `NOT(d set AND d notSet)` | a contradiction that negates to TRUE — EVERY row | + * + * Row two is the likeliest producer — a dropped `$` is a canonical agent typo — + * and row three is the strangest: the #5146 guard was computed while the sibling + * still existed (`requireValue`), the sibling then vanished inside + * `fieldLeaves`, and the surviving conjunction was contradictory, so the + * negation widened to the whole dataset. All three WIDEN — the #3650 failure + * mode the note at {@link MONGO_TO_CUBE_OP}'s miss branch forbids in this very + * function. + * + * The refusal is {@link mixedFieldWrapperError} via + * {@link assertUnmixedFieldWrapper}, in this module's one envelope + * (`INVALID_FILTER` / 400, #5352). The message must do one thing more than the + * module's other refusals: the shape has TWO legitimate repairs answering two + * intents this module cannot tell apart — an operator missing its `$` + * (`gte` → `$gte`) and a nested-relation member that needs a wrapper of its own + * — so the message names the offending key(s) and shows BOTH rewrites (ruling + * requirement, #6444). + * + * Option B — flattening the non-`$` siblings as nested paths next to the + * operators — was REJECTED by the same ruling: it would compile the likely-real + * cause (a dropped `$`) into a predicate on a non-existent member `amount.gte`, + * turning a diagnosable mistake into a harder one. + * + * ⛔ What does not move: a wrapper that is ALL non-`$` keys keeps flattening to + * the dotted member (`{d: {nested: 'x'}}` → `d.nested`); a wrapper that is ALL + * `$`-operators compiles exactly as before; `$null` / `$exists` flag semantics + * (#5526 / #5332 / #5347) and {@link comparand} are untouched. The sibling door + * `read-scope-sql.ts` already fails closed on this exact shape + * (`compileField`'s non-`$`-key check) and is not touched — this change makes + * the two doors give one answer. + * * Row-result cover: `filter-operator-coverage.test.ts` for the operator * vocabulary, `native-sql-filter-logic-conformance.test.ts`, which runs the * SHARED combinator table (`FILTER_LOGIC_CASES`, #3774) that the SQL compiler, @@ -294,9 +340,11 @@ * deliberately does not carry (NULL handling, boolean identities), * `filter-array-lowering.test.ts` for the array door (#5334), * `filter-value-type-fidelity.test.ts` for what each comparand TYPE binds on both - * consumers (#5526, carrying #5528's cases forward as end-to-end assertions), and + * consumers (#5526, carrying #5528's cases forward as end-to-end assertions), * `filter-normalizer-undefined-comparand.test.ts` for the `undefined` refusal and - * its `null` control group (#6386). + * its `null` control group (#6386), and + * `filter-normalizer-mixed-wrapper.test.ts` for the mixed `$`/non-`$` wrapper + * refusal and its pure-shape control groups (#6444). */ import { isFilterAST, parseFilterAST, VALID_AST_OPERATORS } from '@objectstack/spec/data'; @@ -659,6 +707,98 @@ function assertDefinedComparands(field: string, spec: unknown): void { } } +/** + * [#6444, ruled Option A on 2026-08-08] A field wrapper mixing `$`-operator + * keys with non-`$` sibling keys. + * + * ONE wording whatever the mix (#5240 — one condition, one wording); only the + * field and the two key lists vary, because only those do. Where this message + * has to do MORE than the module's other refusals: the shape has two + * legitimate repairs answering two different intents, and the module cannot + * tell which one the author held — that inability is exactly why the shape is + * refused rather than read. So the message must present BOTH (ruling + * requirement): + * + * - an OPERATOR missing its `$` — the canonical agent typo + * `{amount: {gte: 10, $lte: 20}}` — repaired by the prefixed spelling + * (`"gte" → "$gte"`); + * - a NESTED-RELATION member that strayed into an operator wrapper — + * repaired by giving it a wrapper of its own (`{ "d": { "nested": … } }` + * compiles to the member `d.nested`), ANDed with the operator constraint + * explicitly, since one JSON object cannot spell the same field key twice. + * + * Option B — flattening the sibling as a nested path beside the operators — + * was rejected by the same ruling: it would compile the missing-`$` typo into + * a predicate on a non-existent member `amount.gte`, converting a diagnosable + * mistake into a harder one. + */ +function mixedFieldWrapperError(field: string, opKeys: string[], nonOpKeys: string[]): Error { + const offending = nonOpKeys.map((k) => `"${k}"`).join(', '); + const rewrites = nonOpKeys.map((k) => `"${k}" → "$${k}"`).join(', '); + const example = nonOpKeys[0]; + return invalidFilterError( + `[analytics] "${field}" mixes $-operator keys (${opKeys.join(', ')}) with non-$ sibling key(s) ` + + `${offending} in ONE field constraint — refusing to compile this filter. A $-prefixed key is an ` + + `OPERATOR and a bare key is a NESTED-RELATION member; the two readings of ${offending} lead to ` + + `different predicates and this module cannot tell which was meant, so any silent choice is a ` + + `guess. If an operator missing its "$" was meant — the usual authoring slip — spell it with the ` + + `prefix: ${rewrites}, as in { "${field}": { "$${example}": ... } }. If a nested-relation member ` + + `was meant, give it a wrapper of its OWN with no $ siblings — { "${field}": { "${example}": ... } } ` + + `compiles to the member "${field}.${example}" — and AND it with the operator constraint ` + + `explicitly: { "$and": [{ "${field}": { "$op": ... } }, { "${field}": { "${example}": ... } }] }. ` + + `This shape used to compile by silently DROPPING every non-$ sibling, and a dropped conjunct ` + + `does not narrow the query, it WIDENS it: the chart included rows the author excluded, with ` + + `nothing to read (#3650's failure mode, which this module refuses everywhere else). The sibling ` + + `door in this package (read-scope-sql.ts) already fails closed on this exact shape — one shape, ` + + `one answer (#6444).`, + ); +} + +/** + * [#6444] Refuse ONE field wrapper that mixes `$`-operator keys with non-`$` + * sibling keys — the value-independent sibling of {@link assertDefinedComparands}. + * + * What made the mix silent: {@link fieldLeaves}'s operator arm iterates + * `opKeys` only and returns, and the nested-relation flatten sits after that + * early return — so with even one `$` key present, every non-`$` sibling was + * simply never visited. Dropping a conjunct WIDENS (#3650), and inside a `$not` + * it did worse than widen by one conjunct: {@link nullGuardForFieldSpec} judged + * the wrapper while the sibling still existed (a non-`$` key never satisfies + * {@link operatorIsNullTotal}, so the disposition was `requireValue` or + * `allowNull`, never `none`), the sibling then vanished here, and for a + * null-predicate operator the surviving guard was CONTRADICTORY — + * `{$not: {d: {$null: true, nested: 'x'}}}` compiled to `NOT(d set AND d + * notSet)`, which is TRUE for every row. That same never-`none` fact is what + * guarantees the #5146 rewrite carries a mixed wrapper to this gate by + * reference instead of swallowing it — pinned in + * `filter-normalizer-mixed-wrapper.test.ts`'s rewrite block. + * + * ## Ordering against the neighbouring gates + * + * Runs in {@link fieldLeaves}'s wrapper arm, after the #5240 zero-operator + * refusal (disjoint by construction: `{}` has no keys of either kind) and + * after {@link assertDefinedComparands} at the function's entry — so + * `{d: {$eq: undefined, nested: 'x'}}` is refused as an undefined comparand, + * not as a mix. Both are refusals in the same envelope, so the REST face + * answers 400 either way; the ordering is pinned as a measured fact, not a + * contract. + * + * ## What is deliberately not judged here + * + * A wrapper that is ALL `$`-operators or ALL non-`$` members passes untouched — + * this gate moves the refusal set by exactly the mixed shape. Whether a non-`$` + * KEY is a real member of the modeled object is the schema's question at a + * different layer, not this compiler's. + */ +function assertUnmixedFieldWrapper(field: string, wrapper: Record): void { + const keys = Object.keys(wrapper); + const opKeys = keys.filter((k) => k.startsWith('$')); + if (opKeys.length === 0) return; + const nonOpKeys = keys.filter((k) => !k.startsWith('$')); + if (nonOpKeys.length === 0) return; + throw mixedFieldWrapperError(field, opKeys, nonOpKeys); +} + /** * Compile one `field: value | { $op: … }` entry into its leaves. * @@ -700,6 +840,12 @@ function fieldLeaves(key: string, raw: unknown): NormalizedFilterNode[] { `shape refused on every backend.`, ); } + // [#6444] A wrapper mixing $-operator keys with non-$ siblings is refused + // BEFORE the operator arm below gets to iterate `opKeys` only and return — + // that early return is exactly how the non-$ siblings used to vanish. See + // {@link assertUnmixedFieldWrapper} for the two-intent message contract and + // the `$not` interaction. + assertUnmixedFieldWrapper(key, wrapper); const opKeys = Object.keys(wrapper).filter((k) => k.startsWith('$')); if (opKeys.length > 0) { for (const opKey of opKeys) {