Skip to content

fix(ui-core): evaluatePlainCondition reads a bare-string predicate as CEL, as evaluateCondition does - #27773

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-4vb0fy
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-4vb0fy

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Bill · project thread

Fixes #27605

Clause-②: no (narrowing)
Metadata-protocol: narrowed
Skills-docs: none — no skill or doc names evaluatePlainCondition (repo grep over skills/, content/, docs/: 0 hits); the CEL teaching for bare predicates is #27587's

维护者速读

  • 改了什么:@object-ui/core 对外导出的 evaluatePlainCondition(condition, record) 把「裸字符串」条件按 CEL 求值,与 evaluateCondition 走同一个 CEL 引擎;只有 ${…} 模板仍走旧 JS 求值器。
  • 为什么改:spec 把裸字符串定义为 CEL 简写,服务端和控制台其余谓词路径(fix(ui-core): read a bare-string predicate as CEL shorthand, as the spec and server do #27584 之后)都按 CEL 读;这个导出却还按旧 JS 语法读,调用它的宿主拿到的是另一种方言。
  • 风险与回滚:只依赖 JS 语法的裸字符串(===、.includes()、.length 等)现在返回 false,原来能算出 true;仓内除自身测试外零调用方(实测见下)。回滚即 revert 本 PR。
  • 先看:ui/packages/core/src/evaluator/ExpressionEvaluator.ts(evaluatePlainCondition 函数体)、ui/packages/core/src/evaluator/__tests__/ExpressionEvaluator.test.ts(describe('evaluatePlainCondition'))、.changeset/27605-plain-condition-cel.md。
  • 两分钟验证:pnpm --dir ui exec vitest run packages/core/src/evaluator/__tests__/ExpressionEvaluator.test.ts,预期 88 个测试全绿。
  • 需要你做的:无,席位落地。

What changes

  • evaluatePlainCondition (@object-ui/core, published through evaluator/index.ts): a bare string now goes to ExpressionEvaluator.evaluateCondition(condition, { throwOnError: true }), which since fix(ui-core): read a bare-string predicate as CEL shorthand, as the spec and server do #27584 evaluates a bare string on the canonical @objectstack/formula CEL engine (evaluateCelCondition). The evaluation context is unchanged ({ ...record, data: record }), so both status and data.status resolve in CEL.
  • The function's fault policy is kept: throwOnError: true turns a CEL fault into a throw, which the existing catch answers with false; a non-boolean result answers false (the engine seam already compares === true). A blank condition answers false as it did before, by an explicit guard, because evaluateCondition answers a blank gate true ("no gate").
  • A ${…} template is unchanged: evaluate(condition, { throwOnError: true }) === true on the legacy engine.
  • Changeset .changeset/27605-plain-condition-cel.md: '@object-ui/core': minor, both declaration lines, a BREAKING note that teaches the CEL spelling with no FROM-TO list and no console-only escape hatch, ADR-0087 disposition not-required (no-migration-prescription) as in the precedent's changeset.

Why

@objectstack/spec defines a bare string in a predicate slot as { dialect: 'cel', source } (the string arm of ExpressionInputSchema in packages/spec/src/shared/expression.zod.ts), the server reads it on CEL, and #27584 moved every console predicate path to CEL. This export was the one public reader left on the legacy JavaScript grammar (contract review of #27584, should-fix 3; triage graded it align, not retire).

Tests

All in ExpressionEvaluator.test.ts, describe('evaluatePlainCondition'):

  • Rewritten pin: the compound case now uses the CEL spelling (amount > 1000 && status == 'urgent'), true and false legs.
  • New: CEL-only constructs evaluate (status in [...], size(tags) > 0), each with a true and a false leg.
  • New: a JS-only spelling is refused with false where the JS grammar would have answered true (===, .includes(), each beside its CEL control (==, .contains() answering true, and no console line.
  • New: data.amount > 1000 as a bare CEL predicate.
  • New: parity, six source and record pairs, evaluatePlainCondition equals new ExpressionEvaluator({ ...record, data: record }).evaluateCondition(source).
  • New: a ${…} template still reads === on the legacy engine.
  • New: a blank and a whitespace-only condition answer false.
  • Kept: field reference, numeric comparison, template data. reference, invalid syntax, non-boolean result.

Acceptance notes

  • Narrowing, declared Clause-②: no (narrowing) / Metadata-protocol: narrowed as the claim and the precedent (fix(ui-core): read a bare-string predicate as CEL shorthand, as the spec and server do #27584) do: a bare string that only the JS grammar reads now answers false from this export.
  • Caller sweep at d93d9be5 (git grep evaluatePlainCondition over the whole tree, ui/ and packages/ included): 0 callers outside its own file and its test file. Sibling repositories (HotCRM, cloud) were not measured: NOT MEASURED, out of this card's scope.
  • Blank condition: kept at false (unchanged), so it deliberately differs from evaluateCondition's diagnosed "no gate" true; this export answers "does the condition hold", and a blank one never did. Pinned.
  • No console line on a fault, as before: the throwOnError route passes warn: false to the engine seam, so the answer false is the whole report. Pinned.

Verification

Ablation (scripts/ablation-replace.mjs under the verify lock, restored to HEAD after)
  • Anchor return evaluator.evaluateCondition(condition, { throwOnError: true }); replaced with return evaluator.evaluateExpression(condition) === true; (the legacy route): 3 failed of 88, the CEL-only, JS-only-refused and parity tests. Restore: blob equal to HEAD, git diff HEAD empty.
Local runs
  • pnpm --dir ui exec vitest run packages/core/: 218 files passed, 4141 tests passed, 27 skipped.
  • pnpm --filter @object-ui/core type-check (tsc --noEmit && tsc -p tsconfig.test.json): exit 0.
  • eslint on both changed source files: 0 errors (warnings are the file's existing anys).
  • node ui/scripts/check-new-cross-file-line-citations.mjs: 0 new citations.
Gates

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 34 commands on this diff; each was run with its exit code recorded, and all 34 exit 0 on 873fd637. --ran reconciliation: 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN.

  • pnpm --dir ui exec vitest run scripts/ first exited 1 with 2 failures, both from this container's build state and not from the diff: unit-registry-absence-collision could not import 31 modules because @objectstack/client had no dist/ here, and check-vi-mock-inherit timed out at 15 s on the shared box. After pnpm --filter '@objectstack/client...' build, the full command re-ran: 169 files passed, 2 skipped, 4902 tests passed, exit 0. That re-run is the recorded line.
Dogfood battery

Run once on the final head 873fd637, after building the closure (pnpm --filter '@objectstack/dogfood^...' build, exit 0), under the verify lock: pnpm --filter @objectstack/dogfood test, VERDICT command-exit 0; Test Files 354 passed, 3 skipped (357); Tests 2859 passed, 20 skipped (2879); 0 failures.


Generated by Claude Code

… CEL

A bare string handed to the published evaluatePlainCondition export now
takes the CEL route evaluateCondition gives it, as the spec defines the
shorthand; a ${...} template stays on the legacy engine. Its JS-grammar
pins are rewritten to the CEL spelling, with a JS-only spelling pinned as
refused (false) and a parity pin against evaluateCondition.

Claude-Session: https://claude.ai/code/session_01Utp6CYKdX5M2WLtcLDH7qF
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:ui#4 · os-bill · session_01Utp6CbDAucNyQ4UtzCJKyh
Contract-review-claim: session_01Utp6CbDAucNyQ4UtzCJKyh · head 873fd63 · 2026-10-29T22:52Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:ui#4 · os-bill · session_01Utp6CbDAucNyQ4UtzCJKyh

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 873fd6374add3de6b474155ee09a0d96238c0aba
Local-runs: none
Scope: ui/packages/core/src/evaluator/ExpressionEvaluator.ts, ui/packages/core/src/evaluator/tests/ExpressionEvaluator.test.ts + packages/spec/src/shared/expression.zod.ts, ui/packages/core/src/evaluator/fieldRules.ts, ui/packages/core/src/evaluator/declaredPredicate.ts, ui/packages/core/src/evaluator/ExpressionContext.ts, ui/packages/core/src/evaluator/index.ts, ui/packages/core/src/index.ts, ui/packages/core/package.json, .changeset/27457-bare-string-predicate-cel.md (precedent, on main), .changeset/config.json, scripts/check-adr-0087-registration.mjs, .github/workflows/pr-automation.yml (the job that runs it)

Inputs: card #27605 body and its 3 comments (triage 6500555416, dispatch 6508518080, dev report 6510355078), PR #27773 body, its 3-file list and the net diff against origin/main at the head (merge base d93d9be5, 2 commits, +92 / -8), the 54 check-runs on the head, and as precedent PR #27584 (#27457) with its record 6499697983 whose should-fix 3 filed this card. Everything below is read off the tree at the head, not off the PR body.

① Derived judgments

Accept-set of the published export evaluatePlainCondition(condition, record) (@object-ui/core 18.2.0, reached through evaluator/index.ts L10 and src/index.ts L86; @object-ui/react does not wildcard re-export core, so this is the one carrier). On main (L716-731) a bare string went to evaluator.evaluateExpression(condition) on the legacy JS grammar and the answer was result === true; on the head (L725-741) it goes to evaluator.evaluateCondition(condition, { throwOnError: true }), which since #27584 hands a bare string to evaluateCelCondition (L434-436, the legacy branch at L441-443 is reachable only for an unwrapped non-cel envelope, which a string argument can never be).

  • Newly ACCEPTED (evaluated where they faulted to false): every CEL-only construct, in over a list, size(), has(), .contains(), today(). Each was always the spec's reading of a bare string: ExpressionInputSchema L190-191 and EvaluatedExpressionInputSchema L269-272 transform the string arm to { dialect: 'cel', source }. RIGHT, and pinned with a true and a false leg each (status in [...], size(tags) > 0).
  • Newly REFUSED (answered false where the JS grammar evaluated them): ===, !==, ?., ??, .includes(, .length, ternaries, JS methods, and a read of a key the record does not hold (legacy read undefined and compared clean; CEL faults). None was ever in the spec's string arm, and the server's compile step refuses them. RIGHT, and declared as a narrowing of what THIS EXPORT tolerated, not of the protocol. Pinned with the paired CEL control (=== beside ==, .includes( beside .contains(), the pair the precedent record asked for. Measured on the head: git grep evaluatePlainCondition over the whole tree hits only the function, its test and the changeset. 0 callers, as the report says.
  • Fault policy, read through the seam rather than the PR body: throwOnError: true makes evaluateCelCondition run evalFieldPredicate twice with warn: false (L506-507); a parse fault sets reason, the warn: false branch skips warnPredicateFailure (the one console.warn at fieldRules L164), both runs return their opposite fallbacks, the divergence throws CEL predicate failed to evaluate (L508-510) and the function's existing catch answers false. A non-boolean result is NOT a fault at the seam: value = res.value === true (fieldRules L297) is false in both runs, they agree, and false is returned without a throw. So the three legs the docblock names (fault, non-boolean, blank) each answer false with no console line. RIGHT; the "no console line" pin spies the channel the seam actually uses.
  • The blank guard (head L729-731): typeof condition !== 'string' || isBlankPredicateText(condition) returns false before an evaluator is built. Without it a blank bare string would reach answerBlankGate (L535-547) and be answered true as "no gate", which inverts this function's question ("does the condition hold"). RIGHT to branch before the call, and the deliberate difference from evaluateCondition is declared in the PR and pinned ('' and ' '). No diagnostic is emitted for it, as before; ADR-0137 D4 forbids a silent true, not a closed false.
  • Evaluation context unchanged: { ...record, data: record } flows through ExpressionContext.toObject() (L99-105) into the CEL extra bag, and bag.record is absent so the row itself is the record namespace (L478-481). status, data.status and, as a by-product, record.status all resolve. Pinned for data.amount > 1000. RIGHT.
  • ${…} template route unchanged: evaluator.evaluate(condition, { throwOnError: true }) === true (L735), the single-template arm at evaluate L237-239 on the legacy engine, with === still read inside it. Pinned. Matches what evaluateCondition L422-424 and the precedent keep for templates.
  • Signature and return type unchanged; no other public symbol moves. The rewritten compound pin (=== to ==) is the fix the card asks of authored predicates, and the flipped assertion gained a counter-case rather than losing one.
  • Parity pin: six evaluating pairs compared against new ExpressionEvaluator({ ...record, data: record }).evaluateCondition(source) in fail-soft mode. Correctly scoped to predicates that evaluate; on a fault the two would differ (true fail-soft vs this export's false), which is the function's declared policy, not a parity gap.

② Semver level

  • Changeset .changeset/27605-plain-condition-cel.md: '@object-ui/core': minor, Clause-②: no (narrowing), Metadata-protocol: narrowed, one BREAKING banner, exactly one adr-0087 marker. The PR body carries the same two declaration lines plus Skills-docs: none — no skill or doc names evaluatePlainCondition. Lines match on both carriers; Check Changeset and PR-body guards are green on the head.
  • Level: narrowed on a package the diff moves takes at least minor (AGENTS.md L1022-1026). minor is right. @object-ui/core is in the fixed group of .changeset/config.json, so the whole group moves together as it did for the precedent; nothing else needs its own entry.
  • ADR-0087 disposition not-required (no-migration-prescription): the gate's own check for this category is statement-against-statement, the body must carry no migration prescription (findMigrationPrescription: arrow branches, framed sections, rewrite tables, framed removals). The body has no arrow, no migration heading, no table. Check Changeset (the job that runs check-adr-0087-registration.mjs --base MERGE_BASE, pr-automation.yml L1001-1002) is green. Substantively: no spec key, export or config field a host can write under the protocol moves; the refused spellings were never in the spec's string arm; objectstack migrate meta has nothing to rewrite. HOLDS. The BREAKING paragraph teaches CEL by concept ("equality is ==, membership is in, a substring test is .contains(…), a length is size(…)") without a FROM-TO list, which is the shape the precedent record's should-fix 1 prescribed, and the ${…} template is stated as unchanged rather than offered as a way to keep the legacy reading. Honest.
  • Check-runs on the head at the final read: 54 total, 26 success, 11 skipped, 17 in progress. Green: Check Changeset, PR-body guards, Governed Surface Queue Guard, Check PR Size, Lint UI, Build & E2E UI, Lint · ESLint, every root Lint · lane (PM dispatch-gates, docs & code guards, repo & tooling gates included), Type Check · source gates / workspace / consumer gates / debt ledger, Test Core, Dogfood Regression Gate (its shards path-skipped), Live E2E UI, Build UI Deps. In progress: all 16 Test UI shards and Type Check UI. Those are the gate verdicts for the rewritten pins and the ui type-check; this record does not pre-empt them, the queue waits for green.

③ Boundary flags

Dev report 6510355078 is on this head (873fd637), the dogfood battery was run once on it after the closure build (the precedent's should-fix 2 is answered here by construction), and open_questions is empty.

  • Deviation 1 (PR body section order per os-dev.md): procedural, the guards are green. ANSWERED.
  • Deviation 2 (Skills-docs: none — REASON in the claim template's form): the grep claim is re-measured, no skill or doc names the export; the CEL teaching for bare predicates belongs to skills/objectui teaches JavaScript-syntax bare predicates (===, !, raw JS operators) that the console will read as CEL after #27457 #27587 and is not this card's. ANSWERED.
  • Deviation 3 (blank condition kept false, differs from evaluateCondition's "no gate" true): judged in ① and ④, the right branch for this function's question, declared and pinned. ANSWERED.
  • Deviations 4 and 5 (no assignee / reviewer; footer form): per the dispatch order and AGENTS.md. ANSWERED.
  • out_of_scope_findings: sibling repositories (HotCRM, cloud) NOT MEASURED, carrier none. Acceptable: the host-facing carrier for a consumer outside this repo is the changeset's BREAKING paragraph itself, which names the refused spellings and the CEL ones; there is no contract violation left unfiled.
  • Seat premise re-measured on the head: evaluateCondition sends a bare string to CEL at L434-436; the dispatch's responsibility (route, rewrite the JS pins, changeset declares the dialect change) is met in the three files, and the responsibility's "any other pin the repo grep finds" clause resolves to none on the tree.
  • The dispatching seat's file surface is honoured: no path outside it is touched, no skills/** or Tier H path moves.

④ Design judgment

The narrowing, held against the spec on main and the cited code; tried to break it on fault policy, the blank leg, the context bag, double evaluation and the template leg.

  • Does it take a declared capability away at the consumer? No. The spec declares a bare predicate string as CEL and nothing else (expression.zod.ts L184-193, L228-237); the JS grammar was a capability this one export kept after every other console reader gave it up in fix(ui-core): read a bare-string predicate as CEL shorthand, as the spec and server do #27584. The triage ruling graded align, not retire, and the diff aligns: the export survives, its signature survives, its answer shape survives, only the dialect of the bare string moves to the one the spec declares. This makes the spec's refusal real at the last public reader, which is exactly the ④ test on this face. HOLDS.
  • Fault policy: throwOnError: true is the right mode for a function whose only report channel is its return value, and it is the mode the function already used for templates. The cost is the two-call probe every throwOnError caller already pays (ActionEngine, DeclaredActionsBar); a bare string here pays what its envelope paid elsewhere. No new double evaluation outside that mode. HOLDS.
  • Blank leg: branching before evaluateCondition is the only way to keep false, since answerBlankGate answers true in every mode including throwOnError (L521-523). The guard uses the repo's one definition of blank (isBlankPredicateText, declaredPredicate L42-49), so a whitespace-only string and an empty one are one answer. HOLDS.
  • Context bag: the spread-plus-data bag reaches CEL as extra, and the absent record key makes the row the record namespace, so the three spellings resolve. A host record carrying its own record object would pin that object as the namespace instead; that is evaluateCondition's existing behaviour on the same bag, parity holds, and no console caller exists. Not a finding.
  • Template leg: unchanged on purpose, and the same choice the precedent made for evaluateCondition. The docblock and the changeset state it as "unchanged", not as an escape hatch. HOLDS.
  • Pin sweep: the one rewritten pin asserts the CEL verdict on both polarities; the new pins cover the accepted set (CEL-only), the refused set with controls, the data. namespace, parity, the template leg and the blank leg; the kept pins (field reference, numeric comparison, invalid syntax, non-boolean) still hold on the new route by the seam reading above. No pin was deleted or weakened to "old assertion gone". The dev's ablation (legacy route restored, 3 of 88 red: CEL-only, JS-only-refused, parity) is consistent with that reading; the record does not rely on it.

Findings (none must-fix):

  • should-fix — the adr-0087 marker's reason text is the precedent's sentence verbatim ("no stored metadata the server accepted changes meaning"), which argues about a metadata slot; this card's subject is a runtime function argument that never was metadata. The disposition still holds (no protocol-legal spelling moves, nothing for migrate meta to rewrite), but the reason should say this export's own fact: a host-side function argument with no spec slot or stored representation, whose only reader was this function. Marker text only, no behaviour.

Implemented-by: claude/project-thread-4vb0fy
Reviewed-by: session_01Utp6CbDAucNyQ4UtzCJKyh

VERDICT: PASS

Rendered 2026-10-29T22:59Z.


Generated by Claude Code

…n on its own facts

Claude-Session: https://claude.ai/code/session_01Utp6CYKdX5M2WLtcLDH7qF
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:ui#4 · os-bill · session_01Utp6CbDAucNyQ4UtzCJKyh

Regen-provenance: 6510497934 · 873fd6374add3de6b474155ee09a0d96238c0aba → 97d27dba05ced3767f20abc2e2742502329e0112 · git diff --stat → one file, .changeset/27605-plain-condition-cel.md (+1/−1)

The one commit after the reviewed head answers the record's only should-fix. It rewrites the reason text of the adr-0087 marker so it states this export's own fact: the narrowed operand is a runtime function argument, with no spec schema, no stored representation, and this function as its only reader. The disposition stays not-required (no-migration-prescription). The bump, both declaration lines and the BREAKING paragraph are unchanged, and no source or test moves. check-adr-0087-registration --base origin/main exits 0 locally on the new head. Record 6510497934 (CONTRACT_REVIEW_TIER PASS) keeps governing this head.


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:ui#4 · os-bill · session_01Utp6CbDAucNyQ4UtzCJKyh

席位核对

ACCEPT · head 97d27dba05ced3767f20abc2e2742502329e0112 · 2026-10-29T23:20Z

  • Shape: draft → main, Fixes #27605.
    • Clause-②: no (narrowing) and Metadata-protocol: narrowed appear in the PR body, the changeset and the claim. The changeset is a minor on @object-ui/core, with one BREAKING paragraph and an adr-0087: not-required (no-migration-prescription) marker.
    • Contract review: CONTRACT_REVIEW_TIER PASS 6510497934 at 873fd637, with no must-fix. It governs this head through Regen-provenance 6510551796, because the one later commit changes only the marker's reason text, which was the record's single should-fix.
  • Change, read: evaluatePlainCondition now hands a bare string to evaluateCondition(condition, { throwOnError: true }), so it is read on CEL, the one dialect the spec's string arm declares, instead of the legacy JS grammar. A blank string still answers false before evaluation. A fault or a non-boolean result answers false with no console line. The ${…} template leg is unchanged. git grep finds no caller outside the function's own test.
  • Evidence (report 6510355078): the pins cover the accepted CEL-only set (in, size()), the refused JS-only set beside paired CEL controls (=== beside ==, .includes( beside .contains(), the data. namespace, parity with evaluateCondition, the template leg and the blank leg. The one rewritten pin asserts the CEL verdict on both polarities, and no pin was deleted. Ablation (legacy route restored) turns 3 of 88 red. The dogfood battery ran once on the reviewed head.
  • Gates (check-runs on the head): 44 success, 11 skipped, 0 failure. Check Changeset, which runs check-adr-0087-registration, is green. check-governed-merges --pr 27773: NOT governed, 0 of 3 paths, 100 changed lines.
  • Landing: non-governed, all green, mergeable_state clean, so the seat lands it with pr_ready and automerge_enable.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 29, 2026 23:21
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 29, 2026 23:21
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 5c083e8 Oct 29, 2026
56 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-4vb0fy branch October 29, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@object-ui/core evaluatePlainCondition still reads a bare-string predicate on the legacy JS grammar after #27457

1 participant