diff --git a/.changeset/6617-in-path-right-diagnostic.md b/.changeset/6617-in-path-right-diagnostic.md new file mode 100644 index 0000000000..f3418db7fa --- /dev/null +++ b/.changeset/6617-in-path-right-diagnostic.md @@ -0,0 +1,45 @@ +--- +'@object-ui/app-shell': patch +--- + +**metadata-admin predicates: `in` with a path on the right is now diagnosed in +dev mode instead of failing silently.** No verdict changes — this is a +diagnostic only. + +The Setup/Studio predicate evaluator matches membership as `path in ['a','b']`: +the right-hand side must be a bracketed literal set. A membership test whose +right side is a **path** therefore never matched that branch at all. Carrying no +`==` / `!=` either, it fell through to the bare-truthy tail and the *whole* text +was evaluated as one operand: + +- `'admin' in current_user.positions` — ADR-0068's own headline example, and the + spelling `SelectOptionSchema`'s docblock names as the canonical use of the key + — leads with a quote, so it came back verbatim as a non-empty string. A + non-empty string is truthy, so the predicate read **TRUE for every user**, + whatever `positions` held. The failure direction is **permissive**: an option, + field or section gated to admins rendered for everyone. +- `data.roles in current_user.positions` — path-shaped, so the resolver walked + off the draft mid-path and the predicate read **FALSE for every row** instead. + +Either way the verdict had nothing to do with the membership that was written, +and nothing appeared in the console: objectstack#6936's unresolved-path warning +hangs on the path resolver, which quote-leading text never enters, and +objectui#4049's path-shaped-literal warning only matches text starting with an +identifier character. + +Such a predicate now emits a warn-once dev-mode diagnostic naming the offending +text, the predicate that carried it, and the supported subset — and stating +plainly that **a path on the right of `in` cannot be written on this surface +today**, rather than implying that some other punctuation would work. The +detection reuses the evaluator's existing quote-aware scan, so a predicate that +is itself a quoted literal containing the word (`'plug in adapter'` — correct +code) is *not* accused. + +Nothing is resolved that was not resolved before and no operand handling is +added; every predicate that evaluates today reaches exactly the same answer, +pinned before and after. The semantic fix belongs to publish-time validation of +predicate expressions (objectstack#7010) and to the real CEL runtime — this +evaluator is an interim stand-in for `@objectstack/formula` and this diagnostic +retires with it at ROADMAP M9, the same as objectui#4049's and objectui#4266's. + +objectui#6617. diff --git a/packages/app-shell/src/views/metadata-admin/predicate.test.ts b/packages/app-shell/src/views/metadata-admin/predicate.test.ts index d886518c91..3fb4e99437 100644 --- a/packages/app-shell/src/views/metadata-admin/predicate.test.ts +++ b/packages/app-shell/src/views/metadata-admin/predicate.test.ts @@ -557,3 +557,309 @@ describe('a non-literal element inside `in [...]` is diagnosed, not resolved (ob } }); }); + +/* ── 9. `in` with a PATH on the right is diagnosed, not resolved (objectui#6617) ── */ + +/** + * objectui#6617. The `in` branch matches `/^(.+?)\s+in\s+(\[.*\])$/` — the right + * side must be a BRACKETED literal set — so a membership test whose right side is + * a PATH never reaches it. It carries no `==`/`!=` either, so it falls to the + * bare-truthy tail and the WHOLE text is evaluated as one operand: + * + * `'admin' in current_user.positions` → quote-leading, so `parseLiteral`'s tail + * hands it back verbatim as a non-empty string ⇒ TRUE for every user. + * `data.roles in current_user.positions` → path-shaped, so `resolveValue` walks + * off the draft at `roles in current_user` ⇒ FALSE for every row. + * + * Both were SILENT: objectstack#6936's warning hangs on `resolveValue`'s path + * branch, which quote-leading text never enters, and objectui#4049's + * `PATH_SHAPED_LITERAL` only matches text starting with an identifier character. + * + * Ruling on this card: **diagnose only, zero semantic change** — the same posture + * #4049 and #4266 took. Nothing new is resolved; the verdicts are pinned IDENTICAL + * before and after (§9.3), measured on `origin/main` @ 9101be57 before the change + * and asserted here after it. + * + * ⚠️ §9.2 is the load-bearing half. A naive `expr.includes(' in ')` would report a + * bare quoted literal containing the word (`'plug in adapter'` — correct code, a + * truthy string) as broken, which is a worse defect than the bug: a false + * statement about the author's code. Both directions are pinned. + */ +describe('`in` with a path on the right is diagnosed, not resolved (objectui#6617)', () => { + /** The scope this surface publishes once a host `ExpressionProvider` is present. */ + const userScope = (data: Record, current_user: Record) => ({ + data, + current_user, + }); + + /** + * WHICH diagnostic fired, identified by a phrase unique to each message. + * + * ⚠️ Deliberately not the bare card number: #6617's own text CITES + * objectui#4049 as the rule that paths resolve only on the left of an + * operator, so `toContain('objectui#4049')` is true of #6617's message too and + * a mutual-exclusivity assertion written that way passes for the wrong reason + * in one direction and fails spuriously in the other. + */ + const SIG = { + unresolvedPath6936: "is not a name in this form's evaluation scope", + pathShapedLiteral4049: 'which looks like a path but is being used as the literal string', + unparseableInSet4266: 'the WHOLE set was treated as EMPTY', + inWithoutLiteralSet6617: 'spells a membership test', + } as const; + + /* 9.1 — it fires, and it names both halves plus the supported subset */ + + it("ADR-0068's headline spelling warns, naming the membership text and the predicate", () => { + expect( + evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: ['admin'] })), + ).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + // Both halves or the warning sends nobody anywhere — same bar as #6936/#4049. + expect(warnings()).toContain("'admin' in current_user.positions"); + expect(warnings()).toContain('[metadata-admin] visibility predicate'); + }); + + it('the message names the supported subset', () => { + evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: [] })); + expect(warnings()).toContain("`path in ['a','b']`"); + }); + + it('the message says plainly that this form CANNOT be written here — it does not imply a fix', () => { + evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: [] })); + // The author's next question is "then how do I write it?"; the honest answer + // on this surface is "you cannot", and the message has to say so rather than + // leave them hunting for punctuation that would work. + expect(warnings()).toContain('CANNOT be written here today'); + expect(warnings()).toContain('not with different punctuation, not with a different spelling, not at all'); + }); + + it('the message names the fail-OPEN direction for the quote-leading shape', () => { + evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: [] })); + expect(warnings()).toContain('TRUE for EVERY user'); + expect(warnings()).toContain('open to everyone'); + }); + + it('the path-shaped left side (the FALSE-for-every-row shape) warns too', () => { + expect( + evaluatePredicate( + 'data.roles in current_user.positions', + userScope({ roles: ['admin'] }, { positions: ['admin'] }), + ), + ).toBe(false); + expect(warn).toHaveBeenCalledTimes(1); + expect(warnings()).toContain('data.roles in current_user.positions'); + }); + + it('fires identically whether or not `current_user` is bound — the gap is grammatical', () => { + // The text begins with a quote, so `resolveValue`'s literal shortcut hands it + // to `parseLiteral` and the root is never resolved at all. Binding cannot + // change the verdict, and it does not change the diagnostic either. + expect(evaluatePredicate("'admin' in current_user.positions", scope({}))).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + expect(warnings()).toContain("'admin' in current_user.positions"); + }); + + it('`data.*` on the right is the same gap as `current_user.*`', () => { + expect(evaluatePredicate("'x' in data.tags", scope({ tags: ['zzz'] }))).toBe(true); + expect(warnings()).toContain("'x' in data.tags"); + }); + + it('fires on a sub-expression inside `||`, naming the sub-expression and the whole predicate', () => { + expect( + evaluatePredicate( + "'admin' in current_user.positions || data.kind == 'x'", + userScope({ kind: 'z' }, { positions: [] }), + ), + ).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + expect(warnings()).toContain("'admin' in current_user.positions"); + expect(warnings()).toContain("'admin' in current_user.positions || data.kind == 'x'"); + }); + + it('an extra space around `in` is still detected (the walk matches the token, not the gap)', () => { + expect(evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: [] }))).toBe( + true, + ); + expect(warn).toHaveBeenCalledTimes(1); + }); + + /* 9.2 — THE TRAP, both directions: the quote-aware scan must not accuse + correct code. A naive `.includes(' in ')` fails every case below. */ + + it.each([ + ['a bare single-quoted literal containing the word', "'plug in adapter'", {}], + ['the same, double-quoted', '"plug in adapter"', {}], + ['a literal whose text is only the word', "' in '", {}], + ['a quoted literal on the RIGHT of `==`', "data.label == 'plug in adapter'", { label: 'plug in adapter' }], + ['a quoted literal inside an `in` SET', "data.label in ['plug in adapter','x']", { label: 'x' }], + ['a quoted literal carrying an apostrophe-free inner quote', '"it in that"', {}], + ])('%s does NOT warn — correct code is never accused', (_label, expr, row) => { + evaluatePredicate(expr as string, scope(row as Record)); + expect(warn).not.toHaveBeenCalled(); + }); + + it('the negation of a quoted literal containing the word is silent too', () => { + expect(evaluatePredicate("!'plug in adapter'", scope({}))).toBe(false); + expect(warn).not.toHaveBeenCalled(); + }); + + it('a quoted literal containing the word stays TRUTHY — the verdict is untouched by the scan', () => { + expect(evaluatePredicate("'plug in adapter'", scope({}))).toBe(true); + }); + + it.each([ + ["data.kind in ['a','b']", { kind: 'a' }, true], + ["data.kind in ['a','b']", { kind: 'z' }, false], + ["data.type in ['text','textarea']", { type: 'text' }, true], + ])('control: the supported literal-set form %s stays silent and discriminating', (expr, row, expected) => { + expect(evaluatePredicate(expr as string, scope(row as Record))).toBe(expected); + expect(warn).not.toHaveBeenCalled(); + }); + + /* 9.3 — the zero-semantics proof: verdicts identical to pre-change. + Every row below was MEASURED on origin/main @ 9101be57 before the change and + produced exactly these verdicts; they are re-asserted here after it. */ + + it.each([ + ["'admin' in current_user.positions", { data: {}, current_user: { positions: ['admin'] } }, true], + ["'admin' in current_user.positions", { data: {}, current_user: { positions: ['viewer'] } }, true], + ["'admin' in current_user.positions", { data: {}, current_user: { positions: [] } }, true], + ["'admin' in current_user.positions", { data: {} }, true], + ["'x' in data.tags", { data: { tags: ['x'] } }, true], + ["'x' in data.tags", { data: { tags: ['zzz'] } }, true], + ['data.roles in current_user.positions', { data: { roles: ['admin'] }, current_user: { positions: ['admin'] } }, false], + ['type in current_user.positions', { data: {}, current_user: { positions: ['admin'] } }, true], + ["'plug in adapter'", { data: {} }, true], + ['"plug in adapter"', { data: {} }, true], + ["data.label == 'plug in adapter'", { data: { label: 'plug in adapter' } }, true], + ["'admin' in current_user.positions || data.kind == 'x'", { data: { kind: 'z' }, current_user: { positions: [] } }, true], + ["data.flag && 'admin' in current_user.positions", { data: { flag: true }, current_user: { positions: [] } }, true], + ["data.flag && 'admin' in current_user.positions", { data: { flag: false }, current_user: { positions: [] } }, false], + ["'admin' in current_user.positions", { data: {}, current_user: { positions: [] } }, true], + ["('admin' in current_user.positions)", { data: {}, current_user: { positions: [] } }, true], + ["data.kind in ['a','b']", { data: { kind: 'a' } }, true], + ["data.kind in ['a','b']", { data: { kind: 'z' } }, false], + ["data.type in ['text', data.a]", { data: { type: 'text', a: 'text' } }, false], + ['data.a == data.b', { data: { a: 'x', b: 'x' } }, false], + ])('%s over %j is still %s — the diagnostic changes no verdict', (expr, ctx, expected) => { + expect(evaluatePredicate(expr as string, ctx as never)).toBe(expected); + }); + + /* 9.4 — warn-once discipline, keyed exactly like the three Sets above */ + + it('warns ONCE per (membership sub-expression, predicate) pair, not once per evaluation', () => { + for (let i = 0; i < 5; i++) { + evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: [] })); + } + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('but a different predicate carrying the same membership text gets its own warning', () => { + evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: [] })); + evaluatePredicate( + "'admin' in current_user.positions || data.kind == 'x'", + userScope({ kind: 'z' }, { positions: [] }), + ); + expect(warn).toHaveBeenCalledTimes(2); + }); + + it('`resetPredicateWarnings` clears this memo along with the other three', () => { + evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: [] })); + expect(warn).toHaveBeenCalledTimes(1); + resetPredicateWarnings(); + evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: [] })); + expect(warn).toHaveBeenCalledTimes(2); + }); + + /* 9.5 — dev-mode only */ + + it('the diagnostic is dev-mode only', () => { + const prev = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; + try { + expect( + evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: [] })), + ).toBe(true); + expect(warn).not.toHaveBeenCalled(); + } finally { + process.env.NODE_ENV = prev; + } + }); + + /* 9.6 — no overlap with #4266, and neither can mask the other. + They are mutually exclusive BY CONSTRUCTION: #4266 fires from + `parseLiteral`'s array branch, reachable only for text that already matched + an operator branch (`in`'s right side, or `==`'s); this one fires at the + bare-truthy tail, reachable only when EVERY operator branch declined. */ + + it('a #4266 predicate fires #4266 only — no brackets means no overlap', () => { + expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false); + expect(warn).toHaveBeenCalledTimes(1); + expect(warnings()).toContain(SIG.unparseableInSet4266); + expect(warnings()).not.toContain(SIG.inWithoutLiteralSet6617); + }); + + it('a #4049 predicate fires #4049 only', () => { + expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false); + expect(warn).toHaveBeenCalledTimes(1); + expect(warnings()).toContain(SIG.pathShapedLiteral4049); + expect(warnings()).not.toContain(SIG.inWithoutLiteralSet6617); + }); + + it('a #6617 predicate fires #6617 only', () => { + evaluatePredicate("'admin' in current_user.positions", userScope({}, { positions: [] })); + expect(warn).toHaveBeenCalledTimes(1); + expect(warnings()).toContain(SIG.inWithoutLiteralSet6617); + expect(warnings()).not.toContain(SIG.unparseableInSet4266); + expect(warnings()).not.toContain(SIG.pathShapedLiteral4049); + }); + + it('one predicate carrying BOTH gaps reports BOTH — neither masks the other', () => { + expect( + evaluatePredicate( + "data.type in ['text', data.a] || 'admin' in current_user.positions", + userScope({ type: 'text', a: 'text' }, { positions: [] }), + ), + ).toBe(true); + expect(warn).toHaveBeenCalledTimes(2); + expect(warnings()).toContain(SIG.unparseableInSet4266); + expect(warnings()).toContain(SIG.inWithoutLiteralSet6617); + }); + + it('an unbound root reports BOTH this gap and objectstack#6936 — both statements are true', () => { + // `type in current_user.positions` is path-shaped, so `resolveValue` throws + // on the unbound root `type in current_user`. The membership diagnostic is + // raised BEFORE that throw, so the author learns the shape is unsupported as + // well as that the name is unknown. Verdict unchanged: fail-open true. + expect( + evaluatePredicate('type in current_user.positions', userScope({}, { positions: ['admin'] })), + ).toBe(true); + expect(warn).toHaveBeenCalledTimes(2); + expect(warnings()).toContain(SIG.inWithoutLiteralSet6617); + expect(warnings()).toContain(SIG.unresolvedPath6936); + }); + + /* 9.7 — the spellings the reused walk cannot see fail to SILENCE, which is the + pre-existing behaviour, never to a false claim about the author's code */ + + it('a parenthesised membership sits at depth > 0 and is not detected — silent, as before', () => { + // The evaluator does not support parentheses at all: the whole text is a + // path-shaped operand with an unbound root, so #6936 fires and the verdict + // is fail-open true. Widening the walk to reach it would mean teaching the + // detection about syntax the evaluator itself rejects. + expect( + evaluatePredicate("('admin' in current_user.positions)", userScope({}, { positions: [] })), + ).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + expect(warnings()).toContain(SIG.unresolvedPath6936); + expect(warnings()).not.toContain(SIG.inWithoutLiteralSet6617); + }); + + it('a tab-separated `in` is not detected — a miss is silence, not a false claim', () => { + expect(evaluatePredicate("'admin'\tin\tcurrent_user.positions", userScope({}, { positions: [] }))).toBe( + true, + ); + expect(warnings()).not.toContain(SIG.inWithoutLiteralSet6617); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/predicate.ts b/packages/app-shell/src/views/metadata-admin/predicate.ts index fd74a80834..22f014afb5 100644 --- a/packages/app-shell/src/views/metadata-admin/predicate.ts +++ b/packages/app-shell/src/views/metadata-admin/predicate.ts @@ -142,6 +142,83 @@ * authored predicate that legitimately evaluates false — exactly backwards. * * This diagnostic retires with the file at ROADMAP M9, same as #4049's. + * + * ## `in` with a PATH on the right never reaches the `in` branch (objectui#6617) + * + * The `in` branch matches `/^(.+?)\s+in\s+(\[.*\])$/` — the right side must be a + * BRACKETED literal set. A membership test whose right side is a PATH therefore + * does not match that branch at all. It carries no `==`/`!=` either, so it falls + * all the way to the bare-truthy tail and the WHOLE text is handed to + * `resolveValue` as one operand: + * + * - `'admin' in current_user.positions` — ADR-0068's own headline example, and + * the spelling `SelectOptionSchema`'s docblock names as the canonical use of + * the key — leads with a quote, so `resolveValue`'s literal shortcut hands it + * to `parseLiteral`, whose quoted-string branch declines it (it starts with a + * quote but does not END with one) and whose tail returns it VERBATIM. A + * non-empty string is truthy ⇒ the predicate is TRUE for every user, whatever + * `positions` holds. Fail-OPEN: an option, field or section gated to admins + * renders for everyone. + * - `data.roles in current_user.positions` — path-shaped, so `resolveValue` + * splits it on dots into `data` / `roles in current_user` / `positions`, walks + * off the draft at the second segment and returns `undefined` ⇒ FALSE for + * every row instead. + * + * Either way the verdict has nothing to do with the membership that was written, + * and — this is the part this section closes — it was SILENT. objectstack#6936's + * warning cannot see the first shape (the root is never resolved at all: the text + * begins with a quote, so it never enters `resolveValue`'s path branch) and + * objectui#4049's `PATH_SHAPED_LITERAL` cannot either (it only matches text + * starting with an identifier character, and this text starts with a quote). + * Binding or not binding `current_user` cannot change any of it — the gap is a + * property of the operator's GRAMMAR, not of which names the scope declares, so + * it is not a regression from objectui#6247 and is measured identical before and + * after it. + * + * Ruling on objectui#6617: **diagnose only, zero semantic change** — the third + * time this file takes the posture #4049 and #4266 took, and for the third time + * the reason is the same: the verdict is not corrected here, only the silence is. + * The hook sits at the bare-truthy tail, AFTER the `in` branch has already + * declined the text, and asks one question — does this text nonetheless carry a + * top-level `in`? Nothing is resolved that was not resolved before, no operand + * handling is added, and no expression that evaluates today reaches a different + * answer (pinned in `predicate.test.ts` §9.3, the same shape as §7.3 and §8.3). + * Resolving the right side of `in` is the widening the header's standing + * constraint names — this file is an interim stand-in for `@objectstack/formula` + * and retires at ROADMAP M9; do not grow it into a second evaluator — so it is + * NOT done here, and the diagnostic says so plainly rather than implying that + * some other punctuation would work. + * + * ### Why the detection must be quote-aware, and why it reuses `splitTopLevel` + * + * ⚠️ A naive `expr.includes(' in ')` is WRONG. A predicate that is itself a bare + * quoted literal containing the word — `'plug in adapter'` — is CORRECT code + * (a non-empty string literal, truthy), and announcing it as a broken membership + * test would be a false statement about the author's code: the exact failure the + * `PATH_SHAPED_LITERAL` note refuses to commit. So the scan is `splitTopLevel`'s + * existing `inStr`/`depth` walk, REUSED rather than rewritten — the same walk the + * `&&`/`||` splitters and `findUnparseableSetElement` already trust — with `' in '` + * as the operator. Inside a quoted run it never even tests the operator, so a + * quoted literal cannot false-positive, and that reuse is also what keeps this a + * DETECTION rather than a second parser. + * + * The argument that there is no other false positive is short: an expression that + * reaches the bare-truthy tail and is CORRECT is either a path (no spaces are + * possible in one) or a literal — and the only literal that can contain `' in '` + * is a quoted string, which the `inStr` walk protects. So every remaining hit is + * either a membership test this evaluator cannot run, or text that is not a valid + * predicate at all. + * + * Two spellings are deliberately NOT detected, and both fail to SILENCE (the + * status quo) rather than to a false claim: a tab-separated `in`, and a + * parenthesised `('admin' in current_user.positions)` where the operator sits at + * `depth > 0`. Widening the walk to reach them would mean teaching it about + * whitespace classes and parentheses the evaluator itself does not support — + * growing the parser to improve a warning, which is the trade this file does not + * make. + * + * This diagnostic retires with the file at ROADMAP M9, same as #4049's and + * #4266's. */ /** @@ -307,11 +384,21 @@ const warnedPathShapedLiterals = new Set(); */ const warnedUnparseableInSets = new Set(); +/** + * The same warn-once discipline for the `in`-without-a-literal-set diagnostic + * (objectui#6617), keyed on (the sub-expression that carried the top-level `in`, + * predicate) for the same reason as the three Sets above: keying on the + * sub-expression alone would report the first predicate spelling it and stay + * silent about every sibling gate that spells the same membership test. + */ +const warnedInWithoutLiteralSet = new Set(); + /** Reset the warn-once memos. Exported for tests. */ export function resetPredicateWarnings(): void { warnedUnresolvedPaths.clear(); warnedPathShapedLiterals.clear(); warnedUnparseableInSets.clear(); + warnedInWithoutLiteralSet.clear(); } const isDev = (): boolean => @@ -426,6 +513,61 @@ function warnUnparseableInSet(raw: string, source: string): void { ); } +/** + * The operator spelling this detection looks for. Both boundaries are part of + * the token on purpose: `' in'` alone would also match the leading three + * characters of `' integer'`, and `'in '` the trailing three of `'begin '` — + * either would be the false claim about correct code this diagnostic exists to + * avoid. + */ +const IN_OPERATOR = ' in '; + +/** + * Does this text carry a top-level `in` that the `in` branch already declined? + * + * ⚠️ Quote-aware BY REUSE, never by a second scan: {@link splitTopLevel} is the + * same `inStr`/`depth` walk the `&&` / `||` splitters and + * {@link findUnparseableSetElement} already run, so a bare quoted literal that + * happens to contain the word (`'plug in adapter'` — correct code, a truthy + * string) is inside a quoted run when the operator would be tested and is never + * tested at all. A hand-rolled `expr.includes(' in ')` would report it as broken + * and send the author to un-write a spelling that is in fact fine; see the + * header section on why this stays a detection rather than becoming a parser. + * + * Returns false for the two spellings the reused walk cannot see — a + * tab-separated `in`, and one nested at `depth > 0` inside parentheses. Both are + * misses, i.e. silence, which is the pre-existing behaviour; neither is a false + * statement about the author's code. + */ +function carriesTopLevelIn(expr: string): boolean { + return splitTopLevel(expr, IN_OPERATOR).length > 1; +} + +function warnInWithoutLiteralSet(expr: string, source: string): void { + if (!isDev()) return; + const memo = `${expr}::${source}`; + if (warnedInWithoutLiteralSet.has(memo)) return; + warnedInWithoutLiteralSet.add(memo); + console.warn( + `[metadata-admin] visibility predicate \`${source}\` spells a membership test \`${expr}\` whose ` + + 'right-hand side is not a bracketed literal set. This evaluator matches `in` ONLY against a ' + + "bracketed set of literals (supported subset: `path in ['a','b']`), so this text was never " + + 'treated as a membership test at all — it fell through every operator branch to a BARE TRUTHY ' + + 'check of the whole string, and the verdict has nothing to do with the membership that was ' + + "written. With a quoted left side — ADR-0068's own headline example `'admin' in " + + 'current_user.positions` — the whole text comes back verbatim as a non-empty string, so the ' + + 'predicate reads TRUE for EVERY user and a gate meant for admins is open to everyone; with a ' + + 'path-shaped left side it reads FALSE for every row instead. There is no working spelling of ' + + 'this form on this surface: a path on the RIGHT of `in` CANNOT be written here today. Paths ' + + 'resolve only on the LEFT of an operator (objectui#4049) and the right of `in` must be a ' + + 'literal set spelled out in the predicate itself, so testing membership against a value the ' + + "draft or the signed-in user holds is outside this evaluator's subset and cannot be expressed " + + '— not with different punctuation, not with a different spelling, not at all. This evaluator ' + + 'is an interim stand-in for `@objectstack/formula` until CEL lands (ROADMAP M9), and predicate ' + + 'expressions are validated at publish time (objectstack#7010). objectui#6617.', + ); +} + function evalExpr( expr: string, ctx: PredicateCtx, @@ -464,7 +606,21 @@ function evalExpr( const equal = nullish(left) && nullish(right) ? true : left === right; return eqMatch[2] === '==' ? equal : !equal; } - // Bare truthy check + // Bare truthy check. + // + // objectui#6617: the `in` branch above has already DECLINED this text — its + // right side is not a bracketed literal set — and yet the text may still carry + // a top-level `in`, in which case what follows evaluates the whole membership + // spelling as one bare operand and the verdict is unrelated to it. Diagnose + // that here, where the decline is already a fact. + // + // Placed BEFORE `resolveValue` deliberately: the operand may be unresolvable + // (`type in current_user.positions`), and from inside the throw this line is + // never reached again. An unbound root therefore raises objectstack#6936's + // warning as well — both statements are true, and neither is the other's + // cause. The call cannot alter the verdict: it returns void, touches only its + // own memo Set, and `splitTopLevel` cannot throw on a string. + if (carriesTopLevelIn(expr)) warnInWithoutLiteralSet(expr, source); return Boolean(resolveValue(expr, ctx, source)); }