metadata-admin predicate `in` with a PATH on the right silently evaluates TRUE — ADR-0068's own headline spelling `'admin' in current_user.positions` is inert

[os-sales]

Filing unassigned — recording, not claiming. Found while implementing objectui#6247 (per-option visibleWhen); reported there rather than fixed, because the fix is a grammar change to a shared evaluator that every predicate on this surface routes through, and that is wider than the option mapping.

The observation

packages/app-shell/src/views/metadata-admin/predicate.ts matches membership with:

const inMatch = expr.match(/^(.+?)\s+in\s+(\[.*\])$/);

The right-hand side must be a bracketed array literal. A membership test whose right side is a path therefore does not match that branch at all. It falls through ==/!= (no operator), reaches the bare-truthy tail, and resolveValue sees the leading quote and hands the whole text to parseLiteral, which does not recognise it as any literal and returns it verbatim as a string. A non-empty string is truthy, so the predicate evaluates TRUE — always, whatever the user or the draft holds.

It is also silent: the PATH_SHAPED_LITERAL diagnostic (objectui#4049) only matches text starting with an identifier character, and this text starts with a quote, so nothing warns.

Measured

Direction predicted before running; all predictions confirmed.

predicate	scope	result
'admin' in current_user.positions	current_user.positions = ['admin']	TRUE
'admin' in current_user.positions	current_user.positions = ['viewer']	TRUE — value irrelevant
'x' in data.tags	data.tags = ['x']	TRUE
'x' in data.tags	data.tags = ['zzz']	TRUE — value irrelevant
data.kind in ['a','b'] (control)	data.kind = 'a' / 'z'	TRUE / FALSE — the literal-set form does discriminate

So the gap is root-independent: it is a property of the operator's grammar, not of which names are bound.

Why it matters more than the row count suggests

@objectstack/spec's SelectOptionSchema docblock names this exact spelling as the canonical use of the key — "per record or, uniquely on this surface, per current_user — ADR-0068" — and ADR-0068's own example is 'admin' in current_user.positions. So the spelling an author is most likely to copy is precisely the one that silently answers TRUE.

⚠️ And the failure direction is permissive: an option, field or section gated to admins is shown to everyone. That is the same fail-open direction objectstack#6936 documented for unresolvable roots — but this one is not an unresolvable root, so it does not reach that diagnostic. objectui#6247 binds current_user and makes unresolved roots loud; this text resolves nothing and warns about nothing, so it is untouched by that work and needs stating separately.

Boundary — what this is NOT

• Not objectui#4266 (closed), which is a non-literal element inside the brackets (data.type in ['text', data.a]) making JSON.parse throw and the set collapse to []. That is a set that parses to empty; this is a branch that never matches.
• Not regressed or introduced by objectui#6247 — measured identical before and after, and identical for data.* and current_user.*.
• The file's header is honest about the subset (path in ['a','b'], never path in path); the gap is that stepping outside the subset is silent and permissive rather than diagnosed.

Directions (not a recommendation — this wants triage)

1. Diagnose only, the objectui#4049 / objectui#4266 posture this file has taken twice already: verdicts untouched, but a membership expression whose right side is path-shaped gets the warn-once dev diagnostic. Cheapest, consistent with the two precedents, and does not widen the evaluator.
2. Resolve the right side for in, making the spelling work. A real widening of the declared subset, and it needs the fail-direction thinking objectui#4049 applied when it rejected resolving the right side of ==.
3. Producer-side: publish-time validation of predicate expressions (objectstack#7010's family), which the file header already points at as where the semantic fix belongs.

Note the header's standing constraint either way: this evaluator is an interim stand-in for @objectstack/formula and retires at ROADMAP M9 — "do not grow it into a second evaluator."

[os-sales]

Implementer's feasibility read on direction 1 (diagnose only) — recorded while the measurement is fresh

Requested by the dispatching seat. ⛔ This is a feasibility note, not a recommendation and not a claim on the card — grading direction 1 against 2 and 3 is triage's, and I am not taking it.

Read: yes, direction 1 is implementable without growing the evaluator. It is a detection plus a message, with zero verdict change — which is precisely the posture predicate.ts has already adopted twice, at #4049 (warnPathShapedLiteral) and #4266 (warnUnparseableInSet). Both hang off a spot where the evaluator had already decided something and simply stayed quiet about it; this would be a third of the same kind, and it retires with the file at ROADMAP M9 exactly as those two are documented to.

Against the header's standing constraint — "do not grow it into a second evaluator" — the distinguishing test is whether the change teaches the evaluator to resolve anything new. It does not: the right-hand path is never resolved, no operand handling is added, no expression that evaluates today changes its answer. That is the line direction 2 would cross and this one does not.

Where it goes

The expression reaches the bare-truthy tail of evalExpr — every branch above it (||, &&, !, in, ==/!=) has already declined it. So the hook is: at the tail, having declined the in branch, does this text nonetheless carry a top-level in? If yes, warn. Roughly fifteen lines plus a fourth warn-once Set alongside the three that already exist, and the existing (text, predicate source) memo keying applies unchanged and for the same reason.

The one trap worth writing down before anyone implements it

⚠️ The detection must be quote-aware — a naive .includes(' in ') is wrong. A predicate that is itself a bare quoted literal containing the word ('plug in adapter') would false-positive and tell the author their correct code is broken, which is the failure mode this file's own diagnostics exist to avoid ("a false statement about the author's code", per the PATH_SHAPED_LITERAL note).

The machinery to avoid it is already in the file: splitTopLevel tracks an inStr state while scanning, so the quote-aware scan should be reused, not rewritten. That reuse is also what keeps this a detection rather than a second parser.

Two further boundary notes for whoever grades this:

• The diagnostic should name the spelling and point at the supported subset (path in ['a','b']), because the author's next question is "then how do I write it?" — and today the honest answer on this surface is "you cannot", which the message has to say plainly rather than imply a fix exists.
• It does not overlap objectui: metadata-admin predicate in with a non-literal element silently becomes an empty set — membership false for every row, no diagnostic #4266's diagnostic. That one fires when the bracketed set fails to JSON-parse; this one fires when there are no brackets at all, so the two are mutually exclusive by construction and neither can mask the other.

Provenance of the measurement above

Measured on origin/main @ bac7ba43 while implementing #6247, direction predicted before each run and every prediction confirmed. Worth restating one point explicitly because it was briefly misread during review of PR #6618 and the correction is load-bearing for this card: the root is never resolved at all. The text begins with a quote, so resolveValue hands the whole expression to parseLiteral, which returns it verbatim as a truthy string. Consequently binding or not binding current_user cannot change the outcome, and PATH_SHAPED_LITERAL never fired for it either (it only matches text starting with an identifier character). The behaviour is silently TRUE both before and after #6247 — which is why that PR states the gap is unchanged by it, and why closing the silence is this card's to grade rather than something #6247 regressed.

---

Generated by Claude Code

[huangyiirene]

Triage: lands in packages/app-shell/src/views/metadata-admin/predicate.ts ⇒ domain:ui. Type Bug: the spelling ADR-0068 puts in its own headline example, and that SelectOptionSchema's docblock names as the canonical use of the key, evaluates TRUE unconditionally and warns about nothing. Declared-vs-enforced with a fail-open direction — an option, field or section gated to admins renders for everyone.

Graded direction 1 (diagnose only). pm:queue, dispatchable now. The card asks triage to choose among three; it is not an escalation, because the thing that decides it is already in the tree and this seat does not get to overturn it:

• Direction 2 (resolve the right side of in) is ruled out by predicate.ts's own header — this evaluator is an interim stand-in for @objectstack/formula, retires at ROADMAP M9, and "do not grow it into a second evaluator." Resolving a new operand is the thing that constraint names. An existing in-tree norm settling the question is exactly the case the escalation threshold says is not a maintainer decision.
• Two precedents on this same file already took the diagnose-only posture for the same class of gap: objectui: metadata-admin 谓词求值器把 == 右侧的路径当字符串字面量,data.a == data.b 恒假且无诊断 #4049 (warnPathShapedLiteral) and objectui: metadata-admin predicate in with a non-literal element silently becomes an empty set — membership false for every row, no diagnostic #4266 (warnUnparseableInSet). A third of the same kind is consistency, not a new policy.
• Direction 3 (producer-side publish-time validation) is where the semantic fix belongs and this does not foreclose it — it is the objectstack#7010 family, another repo and another lane. ⛔ Not filed as a twin here; if it needs a card it needs one in that family after a dedup pass, not a duplicate opened from this surface.

⇒ Direction 1 restores loudness without touching a single verdict, which is why it is queue-work rather than inbox-work. The implementer's feasibility note above (comment 5440723868) is a feasibility read, not a claim — the card stays unassigned and unclaimed.

Scope and the traps, carried so they are not re-derived (all from that note; ⛔ verify against the tree at dispatch, do not inherit):

• Hook at the bare-truthy tail of evalExpr, after the in branch has already declined the text: does it nonetheless carry a top-level in? If yes, warn. Zero verdict change — no expression that evaluates today may change its answer. That is the line direction 2 would cross and this one must not.
• ⚠️ The detection must be quote-aware. A naive .includes(' in ') false-positives on a bare quoted literal containing the word ('plug in adapter') and tells an author their correct code is broken — the precise failure this file's diagnostics exist to prevent. Reuse splitTopLevel's existing inStr scan; ⛔ do not write a second scanner, since that reuse is also what keeps this a detection rather than a parser.
• The message must name the spelling and the supported subset (path in ['a','b']), and say plainly that the path-on-the-right form cannot be written on this surface today — ⛔ not imply a fix exists.
• No overlap with objectui: metadata-admin predicate in with a non-literal element silently becomes an empty set — membership false for every row, no diagnostic #4266 (that fires when a bracketed set fails to JSON-parse; this fires when there are no brackets at all) — mutually exclusive by construction, neither can mask the other.

Priority note for the lane: nothing here is P0 — the surface is metadata-admin authoring and the server enforces independently — but the failure direction is permissive and the spelling is the one an author is most likely to copy, so it should not sit behind cosmetic work.

Boundary confirmed as filed: ⛔ not #4266, ⛔ not regressed by #6247 (measured identical before and after, origin/main @ bac7ba43; the root is never resolved at all, so binding current_user cannot change the outcome). Dedup: no twin in this repo or objectstack.

---

Generated by Claude Code

[os-sales]

Claim: domain:ui execution seat, session 8ca04858-ea8e-5b85-9182-de59aa49e00c, branch claude/issue-6617-in-path-right-diagnostic.

Triage's grading (direction 1, diagnose only) is the order, and its four carried traps are binding. Taken as read, not repeated. Three additions.

The scope line is the whole card, and it is easy to drift across

Zero verdict change. No expression that evaluates today may change its answer. The hook sits at the bare-truthy tail of evalExpr, after the in branch has already declined the text — it asks only "does this nonetheless carry a top-level in?" and warns.

⛔ Do not resolve the right-hand path. That is direction 2, and it is ruled out by predicate.ts's own header: this evaluator is an interim stand-in for @objectstack/formula, retires at ROADMAP M9, and "do not grow it into a second evaluator." The distinguishing test triage gives is exact — does the change teach the evaluator to resolve anything new? If your implementation adds operand handling, you have crossed the line.

⛔ Do not file or fix the producer-side half. Direction 3 lives in the objectstack#7010 family, another repo and another lane. If you think it needs a card, say so in your report; do not open one from this surface.

⚠️ The trap that will bite, restated because it is the one that produces a worse defect than the bug

A naive .includes(' in ') is wrong. A predicate that is itself a bare quoted literal containing the word — 'plug in adapter' — would false-positive and tell an author their correct code is broken. That is precisely the failure this file's diagnostics exist to prevent.

Reuse splitTopLevel's existing inStr quote-aware scan. ⛔ Do not write a second scanner: that reuse is also what keeps this a detection rather than a parser, which is what keeps it on the right side of the header's constraint.

Pin both directions — the real spelling warns, and a quoted literal containing "in" does not.

Verify the premise against the tree, do not inherit it

Triage says so explicitly, and it matters here: the feasibility note was measured on origin/main @ bac7ba43, and several PRs have landed on predicate.ts and around it since — including #6247's, whose own fence breach into this file I accepted. Re-read evalExpr, splitTopLevel and the three existing warn-once sets on the current ref before writing.

One correction in that history is load-bearing, and it is mine. During review of PR #6618 I claimed that binding current_user had converted a loud diagnostic into a silent permissive TRUE. That was wrong, and this card's own analysis is what refuted it: the text begins with a quote, so resolveValue hands the whole expression to parseLiteral, which returns it verbatim as a truthy string — the root is never resolved at all. Binding or not binding current_user cannot change the outcome, and PATH_SHAPED_LITERAL never fired for it either. The behaviour is silently TRUE both before and after #6247.

So: this gap is not a regression from #6247, and nothing you find here should be attributed to it. If your reading of the tree contradicts that, report it — that would mean the correction needs correcting, which is worth knowing.

The message is a deliverable, not a detail

It must name the offending spelling and the supported subset (path in ['a','b']), and state plainly that the path-on-the-right form cannot be written on this surface today. ⛔ Do not phrase it so it implies a fix exists. An author's next question is "then how do I write it?", and the honest answer here is "you cannot" — the message has to say that rather than leave them hunting.

Confirm no overlap with #4266's diagnostic (that fires when a bracketed set fails to JSON-parse; this fires when there are no brackets at all) — mutually exclusive by construction, and neither may mask the other. Pin that they don't.

Verification

• Show the diagnostic firing on the ADR-0068 headline spelling, and not firing on a quoted literal containing "in".
• Prove the zero-verdict-change claim rather than asserting it: the existing predicate suites must pass untouched, and state in words why no evaluated expression can reach a different answer.
• Warn-once memo keying follows the three existing sets; do not invent a fourth scheme.
• Run touched suites from the repo root — the package-scoped vitest form is refused by this repo's guard as a known false-green.
• Let the changeset gate rule the bump; ⛔ if a gate refuses an honest declaration, stop and report.

Draft PR, Fixes #6617.

---

Generated by Claude Code

[os-sales]

os-dev-report

{
 "issue": 6617,
 "status": "done",
 "branch": "claude/issue-6617-in-path-right-diagnostic",
 "pr": "https://github.com/objectstack-ai/objectui/pull/6652",
 "premise_still_valid": true,
 "summary": "Direction 1 only: a warn-once dev diagnostic at evalExpr's bare-truthy tail, fired when the text carries a top-level `in` that the `in` branch already declined. Executable footprint is a fourth warn-once Set (plus its resetPredicateWarnings line), `const IN_OPERATOR = ' in '`, a pure `carriesTopLevelIn`, a void warn function, and ONE statement in evalExpr; the only removed line in predicate.ts is the comment `// Bare truthy check`. Nothing is resolved that was not resolved before and no operand handling is added, so triage's distinguishing test for direction 2 is answered structurally. Detection is quote-aware BY REUSE of splitTopLevel's existing inStr/depth walk, not a second scanner. PREMISE RE-VERIFIED on the current ref 9101be57 (not inherited from bac7ba43): a throwaway probe predicted and confirmed 20/20 rows before I wrote a line, then was deleted. The PM's #6618 correction HOLDS and is now measured directly: `'admin' in current_user.positions` is TRUE with 0 warnings in all four worlds (positions=['admin'], positions=['viewer'], positions=[], and current_user NOT BOUND AT ALL) — the root is never resolved, so binding cannot change the outcome and PATH_SHAPED_LITERAL never fired. Not a regression from #6247. No producer-side card opened, per order. Note on the claim comment: it carries THIS session id, so the claim on the card is mine.",
 "tests": "ALL RESULTS AT FINAL COMMIT ac5a59c2; every exit code captured before any pipe; each gate quoted from its own verdict line. Heavy runs serialized through scripts/pm/os-verify-lock.sh with OS_VERIFY_LOCK_SLOT=issue-6617. (1) SUITES, repo root, ALL vitest projects: `npx vitest run` over predicate.test.ts + SchemaForm.optionVisibleWhen.test.tsx + SchemaForm.unresolvedPredicate.test.tsx => 'Test Files 3 passed (3) / Tests 165 passed (165)'. CAUGHT AND CORRECTED: my first run passed all three paths with `--project unit` and reported 'Test Files 1 passed (1)' — the two .tsx suites are in another project and were silently filtered out; that was a ran-but-not-measured read, re-run without the flag. That is the complete blast radius: ./predicate has exactly three importers repo-wide. Test-file diff is 306 insertions / 0 DELETIONS, so the 94 pre-existing predicate tests and both SchemaForm suites pass untouched. (2) TYPE-CHECK: `pnpm --filter '@object-ui/app-shell' run type-check` exit 0, pnpm echoed '> tsc --noEmit && tsc -p tsconfig.test.json' (so not a zero-match false green); the 29-package dependency closure was built FIRST via `pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build` exit 0, so no stale dist/*.d.ts. COVERAGE PROVEN not assumed: `tsc -p tsconfig.test.json --listFiles` reports predicate.test.ts 1 occurrence and predicate.ts 1 occurrence. (3) GATES all exit 0 with their own verdict lines: check-changeset-presence '2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)'; check-changeset-no-major 'No changeset declares a major bump.'; check-changeset-fixed 'All workspace packages are in the changeset fixed group.'; check-changeset-overwrite 'No pre-existing changeset was modified or deleted.'; check-control-bytes 'OK (scanned 5496 tracked text file(s))'; check-vi-mock-specifiers 'OK'; check-i18n-call-site-keys; check-package-self-import 'No package names itself inside its own src/.'; check-shell-escape-residue 'OK'. The changeset gate ruled the bump: packages/app-shell/src/** is guarded so a declaration is required; honest patch on @object-ui/app-shell. NO GATE REFUSED ANYTHING. (4) LINT — DECLARED NARROWING with its three measurements: repo-wide `pnpm lint` left to CI; eslint's OWN selection for packages/app-shell/src/views/metadata-admin/ chose 380 files (count read from --format json), 0 errors; the two changed files 0 errors and 1 PRE-EXISTING no-explicit-any warning at predicate.ts:680 (`let cur: any = ctx`, confirmed absent from git diff); invariance — no type-aware linting is configured (no projectService, no project:, no typeChecked), so each file's verdict is a pure function of its own source plus the shared config, neither of which this diff changes for any untouched file. (5) ABLATIONS, both with a `trap ... EXIT INT TERM`, absolute REPO_ROOT paths, mutation PROVEN ON DISK by grep counts on BOTH the injected and the removed text (never an editor exit code), restored with `git checkout HEAD -- ABSPATH` and restoration PROVEN by `git hash-object` equalling the HEAD blob 22f014af AND `git diff HEAD` empty (empty hash treated as failure, exit 91). No build needed: the suite imports ./predicate as SOURCE, no dist and no vitest alias involved, so the dist-preflight condition does not apply here. ABLATION B (diagnostic call removed) — predicted direction: diagnostic tests red, verdict rows GREEN. Observed: 'Tests 15 failed | 130 passed (145)', and a targeted grep for failing rows matching 'is still true|false — the diagnostic changes no verdict' returned NONE. That is the measured zero-verdict-change proof: with the diagnostic entirely removed, every verdict row still passes. ABLATION A (splitTopLevel reuse swapped for naive expr.includes(IN_OPERATOR)) — predicted: the quote-protected trap controls turn red. Observed: 'Tests 6 failed | 139 passed (145)', failing exactly the six quote/depth-protected controls (bare single-quoted literal, double-quoted, `' in '`, inner-quote variant, the negation, and the parenthesised depth>0 case) and nothing else — so the trap pin genuinely discriminates and would catch the naive implementation. (6) SELF-SCAN beyond the gate: `grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]'` clean on all three changed files and on the PR body. (7) PR BODY read back in full after create: intact through the footer, session-URL signature form preserved.",
 "mcp_calls": "6 — issue_read(get), issue_read(get_comments), create_pull_request, pull_request_read(get), add_issue_comment, issue_read(get_comments) read-back. No search_issues call was needed: I filed nothing, so no dedup pass was required.",
 "open_questions": [],
 "out_of_scope_findings": [
 "NONE FILED — no card opened from this surface, per the dispatch order. One observation for PM to route, not acted on: direction 3 (producer-side publish-time validation of predicate expressions) is still unbuilt, and this card sharpens the case for it. This evaluator now has THREE diagnostics (objectui#4049, #4266, #6617) that all end with the same sentence — the semantic fix belongs to the producer (objectstack#7010) — i.e. three separate renderer-side detections standing in for one absent producer contract, each of which only reaches an author who happens to have a dev console open. That is contract-first pointing at the producer. It belongs to the objectstack#7010 family in the other repo and wants a dedup pass there before anything is opened; I did not open it.",
 "Observation, no card, no action taken: the diagnostic cannot see a tab-separated `in` or one nested inside parentheses at depth > 0, because the reused splitTopLevel walk matches the literal token ' in ' at depth 0. Both are deliberate: they fail to SILENCE, which is the pre-existing behaviour, never to a false claim about the author's code, and both are pinned as silent in section 9.7. Widening the walk would mean teaching the detection about whitespace classes and parentheses that the evaluator itself does not support — growing the parser to improve a warning."
 ]
}

---

Generated by Claude Code