From 1fa5db15e54d2f020d9a7924bb623c49cddca3b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 02:31:42 +0000 Subject: [PATCH] =?UTF-8?q?fix(core):=20ActionRunner=20=E7=9A=84=20conditi?= =?UTF-8?q?on=20=E9=97=A8=E6=94=B9=E9=97=AE=E3=80=8C=E6=9C=89=E6=B2=A1?= =?UTF-8?q?=E6=9C=89=E5=A3=B0=E6=98=8E=E3=80=8D,condition:=20false=20?= =?UTF-8?q?=E7=9C=9F=E7=9A=84=E6=8B=A6=E4=BD=8F=E5=8A=A8=E4=BD=9C=20(#3872?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ActionRunner.execute` —— 引擎的公共执行入口,所有动作面共用 —— 用 `if (action.condition)` 把门开在**真值**上。真值回答不了门要问的那个问题 (「作者到底声明了条件吗」),而在这个键上它答错的方向是**过度放行**: `condition: false` 落在 `if (false)`,整段被跳过,`evaluateCondition` 根本没被 问过,动作照常执行。基线实测(`origin/main` @ 2937bcf7d,handler 自计数): condition: false | handler ran: true | {"success":true} condition: {cel,'false'} | handler ran: false | {"error":"Action condition not met"} 同一句话的两种拼法,结论相反 —— 布尔字面量执行了,同义 envelope 被拦。`false` 是元数据能写出的最明确的一句「永远不要执行这个」(把动作关掉的模板产出的正是 它),所以方向要紧:动作真的跑了,可能落库。这是 #3492 家族的过度放行那一半, 下面一行的 `disabled` 门(#3848 / PR #3873)病在相反方向。 门现在先问「有没有声明 condition 门」再求值。「没有可求值的条件」取自 core 唯一 的谓词归一器 `toPredicateInput`(把 `''`、`null`、空 `source` envelope、非谓词值 一律映射为 `undefined`),外加它包裹而非折叠的纯空白字符串 —— 这就是 #3850 对 「空谓词」范围的裁决,与 `disabled` 门已经在用的同一份。门问对了问题,verdict 就 不需要自己的布尔分支:`evaluateCondition` 对布尔实参原样返回。 ## 行为变更面:单向,且只有一行 只有一种形状换了结论 —— 已声明的布尔 `false`,从执行改为拒绝(`{ success: false, error: 'Action condition not met' }`,这个键本来就用的文案)。其余逐字节不变: `condition: true`、缺省、真表达式/真 envelope 照样执行;假表达式、假 CEL envelope、假 `${…}` 模板照样被拦;三种空谓词(`''`、纯空白、空 `source` envelope) 照样执行,只是理由从「`if ('')` 恰好为假」换成了「什么都没声明」;非谓词垃圾 (`0`、`{}`)照样执行 —— 不是谓词的值不该决定动作的命运,这正是本模块在 `disabled` 上已经确立的 fail-open 姿态。因此本改动只可能开始拒绝执行,绝不可能 开始放行,是 #3848 修法的镜像。 `ActionDef.condition` 随之放宽为 `string | boolean`,与门现在兑现的面一致(也与 旁边的 `disabled` 一致)。这不是消费端的宽容别名:布尔本来就通过接口的索引签名 在运行时被接受,只是被忽略了。 交给求值器的值刻意保持**原样**而非先归一,理由与 #3848 相同、符号相反: `toPredicateInput` 无条件包裹,已经是模板的 `'${x}'` 会变成 `'${${x}}'`,解析失败 后原样返回并被强制为恒 `true` —— 在 `disabled` 上这拦住一切,在 `condition` 上它 会**放行**一切。该归一器缺陷是 #3871;新钉子旁的 tripwire 会在它被修好那天转红。 ## 反向验证(方向先写后跑) 把门还原成 `if (action.condition)`、求值不动:预判恰好三个测试转红,且都点名 `false` —— `condition: false` 那一行、布尔/envelope 等价性、以及「已声明的布尔确实 到达求值器」(真值门下它永远到不了)。实跑一致:`Tests 3 failed | 38 passed`, `disabled` 门的既有钉子全绿,纯表格断言与两条 tripwire 全绿。 Co-authored-by: Claude --- ...ion-runner-declared-condition-gate-3872.md | 59 ++++ packages/core/src/actions/ActionRunner.ts | 82 ++++- .../ActionRunner.conditionGate.test.ts | 296 ++++++++++++++++++ 3 files changed, 426 insertions(+), 11 deletions(-) create mode 100644 .changeset/action-runner-declared-condition-gate-3872.md create mode 100644 packages/core/src/actions/__tests__/ActionRunner.conditionGate.test.ts diff --git a/.changeset/action-runner-declared-condition-gate-3872.md b/.changeset/action-runner-declared-condition-gate-3872.md new file mode 100644 index 000000000..74393aa77 --- /dev/null +++ b/.changeset/action-runner-declared-condition-gate-3872.md @@ -0,0 +1,59 @@ +--- +"@object-ui/core": patch +--- + +`condition: false` now actually prevents the action from executing (objectui#3872) + +`ActionRunner.execute` — the engine's public execution entry, shared by every +action surface — gated conditional execution on `if (action.condition)`, i.e. on +the raw value's TRUTHINESS. Truthiness cannot answer the question the gate needs +answered ("did the author declare a condition?"), and on this key it answered in +the over-permissive direction: `condition: false` fell on `if (false)`, so the +whole block was skipped, `evaluateCondition` was never consulted, and the action +executed. Measured with a call-counting handler: + +- `condition: false` — handler ran: **true**, result `{ success: true }` +- `condition: { dialect: 'cel', source: 'false' }` — handler ran: false, result `{ success: false, error: 'Action condition not met' }` + +Two spellings of the same statement, opposite outcomes. `false` is the most +explicit "never execute this" metadata can carry — and what a template that +switches an action off emits — so the direction matters: the action really ran, +possibly writing. This is the over-permission half of the objectui#3492 family +(the `disabled` gate one line below is objectui#3848, whose defect pointed the +other way). + +The gate now asks whether a `condition` gate is DECLARED before evaluating. +"Nothing to evaluate" is read from core's single predicate normalizer +(`toPredicateInput`, which maps `''`, `null`, an empty-`source` envelope and +non-predicate values to `undefined`), plus the whitespace-only string, which the +normalizer wraps rather than collapses — objectui#3850's ruling on the scope of +"empty predicate", the same one the `disabled` gate already applies. Once the +door asks the right question the verdict needs no boolean branch of its own: +`evaluateCondition` returns a boolean argument verbatim. + +**Behaviour change surface, deliberately one-directional and one row wide.** +Exactly one shape changes verdict — a declared boolean `false`, from executing to +refused (`{ success: false, error: 'Action condition not met' }`, the message the +key already used). Everything else is byte-identical: `condition: true`, an +absent `condition`, and truthy expressions/envelopes still execute; falsy +expressions, falsy CEL envelopes and falsy `${…}` templates are still refused, +as before; the three empty predicates (`''`, whitespace-only, an empty-`source` +envelope) still execute, now because nothing was declared rather than because +`if ('')` happened to be falsy; and non-predicate junk (`0`, `{}`) still +executes — a value that is not a predicate must not decide an action's fate, +which is the fail-open posture this module already committed to for `disabled`. +So this change can only start refusing execution, never start allowing it — the +mirror image of objectui#3848's fix. + +`ActionDef.condition` is widened to `string | boolean` to match what the gate now +honours (and the `disabled` key beside it). This is not a lenient consumer +alias: the boolean was always accepted at runtime through the interface's index +signature, it was simply ignored. + +The value handed to the evaluator is deliberately left RAW rather than normalized +first, for the same reason as objectui#3848 with the sign flipped: +`toPredicateInput` wraps unconditionally, so an already-templated `'${x}'` +becomes `'${${x}}'`, fails to parse, returns verbatim and coerces to a constant +`true` — on `disabled` that blocks everything, on `condition` it would EXECUTE +everything. That normalizer defect is objectui#3871; a tripwire next to the new +pins goes red the day it is fixed. diff --git a/packages/core/src/actions/ActionRunner.ts b/packages/core/src/actions/ActionRunner.ts index 676206c77..032d52395 100644 --- a/packages/core/src/actions/ActionRunner.ts +++ b/packages/core/src/actions/ActionRunner.ts @@ -104,8 +104,17 @@ export interface ActionDef { confirmText?: string; /** Structured confirmation (from crud.ts) */ confirm?: { title?: string; message?: string; confirmText?: string; cancelText?: string }; - /** Condition expression — if falsy, skip action */ - condition?: string; + /** + * Condition predicate — the action executes only while it holds. + * + * A boolean, a bare CEL string, a `${…}` template, or the normalized + * `{ dialect, source }` envelope `objectstack build` emits. Declared and + * FALSE skips the action; not declared at all (absent / empty predicate) + * executes. `boolean` is spelled out here because the execution gate now + * honours `condition: false` as the verdict it plainly is (objectui#3872) — + * the same widening `disabled` already carries below. + */ + condition?: string | boolean; /** Disabled expression — if truthy, skip action */ disabled?: string | boolean; /** @@ -528,8 +537,35 @@ function withIdentityAlias(context: ActionContext): ActionContext { } /** - * Did this action declare a `disabled` gate at all — i.e. is there a CONDITION - * for the evaluator to reach a verdict on? (objectui#3848) + * Did this action declare a gate at all — i.e. is there a CONDITION for the + * evaluator to reach a verdict on? (objectui#3848 for `disabled`, objectui#3872 + * for `condition`) + * + * ## One definition, two gates — why this is not two helpers + * + * `execute` has two predicate gates, and both need this ONE question answered + * before evaluating. They need it for OPPOSITE reasons, which is exactly why the + * question has to be asked separately from the verdict: + * + * - `disabled` (objectui#3848): an EMPTY predicate must not be handed to + * `evaluateCondition`, whose answer for "no condition" is `true` = BLOCKED. + * - `condition` (objectui#3872): a DECLARED-FALSE predicate must not be + * skipped by a truthiness test. `if (action.condition)` never asked whether + * a gate was declared, so `condition: false` — the most explicit "never + * execute" an author can write — landed on `if (false)`, the evaluator was + * never consulted, and the action RAN. Measured on `origin/main` @ + * `2937bcf7d`: `condition: false` → `{"success":true}`, handler ran, while + * the semantically identical `{ dialect: 'cel', source: 'false' }` was + * refused. Asking "declared?" routes `false` to `evaluateCondition`, which + * short-circuits booleans (`if (typeof condition === 'boolean') return + * condition`) and blocks. + * + * The scope of "empty predicate" below is objectui#3850's ruling (`''` / + * whitespace-only / empty-`source` envelope are NOT declared), so both gates + * read the same one. A second module-private twin of the same question in this + * one file is the drift the closing scope note refuses; the name is therefore + * key-neutral, and the `disabled` gate's semantics and message are unchanged by + * objectui#3872. * * `ExpressionEvaluator.evaluateCondition` documents and implements one default * for "there is no condition here": it returns `true`, meaning @@ -564,6 +600,10 @@ function withIdentityAlias(context: ActionContext): ActionContext { * `Boolean(…)` = `true`. A `${…}`-spelled `disabled` predicate evaluated that * way is therefore ALWAYS "disabled", whatever it says. Measured on this tree: * `disabled: '${user.role === "guest"}'` (false, admin context) → blocked. + * On `condition` the same trap points the other way — a constant `true` would + * make every template-spelled `condition` always EXECUTE — so that gate also + * normalizes only to decide declaredness and evaluates the RAW value, keeping + * the verdict `ActionRunner.test.ts` has pinned for that spelling all along. * * So the value handed to the evaluator is left exactly as it was. Every * non-empty shape — boolean, bare CEL, `${…}` template, `{ dialect, source }` @@ -580,10 +620,17 @@ function withIdentityAlias(context: ActionContext): ActionContext { * moved: it lives in a package that depends on this one, it does not cover the * empty-`source` envelope (objectui#3850) or the whitespace string, and * objectui#3850 is the queued ruling on unifying the scope of "empty - * predicate" across the sites. Kept module-private until that lands — one gate, - * one site, no new exported dialect of the same question. + * predicate" across the sites. Kept module-private until that lands — one + * definition, one module, no new exported dialect of the same question. + * + * Nor is the `visible`-filter template in `ActionEngine.getActionsForLocation` + * copied wholesale: its non-predicate branch keeps a historical + * `Boolean(raw)` coercion, so `0` there means "hidden" — fail-CLOSED on junk, + * the opposite of what this module already committed to for `disabled` + * (`catch { isDisabled = false }`). A value that is not a predicate at all must + * not decide an action's fate, on either key, so `0` / `{}` are "no gate" here. */ -function hasDisabledCondition(value: unknown): boolean { +function hasDeclaredPredicate(value: unknown): boolean { if (typeof value === 'string' && value.trim() === '') return false; return toPredicateInput(value) !== undefined; } @@ -717,8 +764,21 @@ export class ActionRunner { // Resolve the action type const actionType = action.type || action.actionType || action.name || ''; - // Conditional execution - if (action.condition) { + // Conditional execution. Ask "is a `condition` gate DECLARED?" — not "is + // the raw value truthy?" (objectui#3872). Truthiness cannot answer that + // question, and on this key it answered it in the OVER-PERMISSIVE + // direction: `condition: false` (the most explicit "never execute" an + // author can write, and what a template that switches an action off + // emits) fell on `if (false)`, so the evaluator was never consulted and + // the action executed anyway — while the semantically identical + // `{ dialect: 'cel', source: 'false' }` was refused. `evaluateCondition` + // short-circuits booleans, so once the gate asks the right question the + // verdict needs no boolean branch of its own. See `hasDeclaredPredicate` + // for the shared "declared?" scope (objectui#3850) and for why the verdict + // still reads the RAW value (`toPredicateInput` double-wraps an + // already-`${…}` string into a constant true — objectui#3871 — which on + // THIS key would mean "always execute"). + if (hasDeclaredPredicate(action.condition)) { const shouldExecute = this.evaluator.evaluateCondition(action.condition); if (!shouldExecute) { return { success: false, error: 'Action condition not met' }; @@ -730,10 +790,10 @@ export class ActionRunner { // `evaluateCondition`, whose answer for "no condition" is `true`, which on // this key means BLOCKED: `disabled: ''` / `' '` / // `{ dialect: 'cel', source: '' }` all returned `Action is disabled` with - // the handler never invoked. See `hasDisabledCondition` above for why the + // the handler never invoked. See `hasDeclaredPredicate` above for why the // gate normalizes to decide but the verdict below still reads the RAW // value (`toPredicateInput` double-wraps an already-`${…}` string). - if (hasDisabledCondition(action.disabled)) { + if (hasDeclaredPredicate(action.disabled)) { // `disabled` may be a boolean, a CEL string, or the normalized envelope // `{ dialect, source }` (what `objectstack build` emits). The previous // code only evaluated the STRING form and treated any object as truthy, diff --git a/packages/core/src/actions/__tests__/ActionRunner.conditionGate.test.ts b/packages/core/src/actions/__tests__/ActionRunner.conditionGate.test.ts new file mode 100644 index 000000000..29d7f7254 --- /dev/null +++ b/packages/core/src/actions/__tests__/ActionRunner.conditionGate.test.ts @@ -0,0 +1,296 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#3872 — `ActionRunner.execute`'s declared-`condition` gate. The + * neighbour-line sibling of objectui#3848 (PR #3873, the `disabled` gate), and + * the OVER-PERMISSIVE half of the objectui#3492 family. + * + * The gate asked `if (action.condition)`, i.e. "is the raw value truthy?", which + * cannot answer "did the author declare a gate?". `condition: false` — the most + * explicit "never execute" metadata can carry, and what a template that switches + * an action off emits — landed on `if (false)`, so `evaluateCondition` was never + * consulted and the action RAN. Measured on `origin/main` @ `2937bcf7d` before + * this change, with a `probe` handler that counts its own calls: + * + * condition: false | handler ran: true | {"success":true} + * condition: 0 | handler ran: true | {"success":true} + * condition: {cel,'false'} | handler ran: false | {"error":"Action condition not met"} + * condition: bare CEL false | handler ran: false | {"error":"Action condition not met"} + * condition: '' / ' ' / {cel,''} / {} | handler ran: true | {"success":true} + * + * So `false` and the semantically IDENTICAL `{ dialect: 'cel', source: 'false' }` + * reached opposite conclusions: the boolean literal executed, its envelope + * spelling was refused. Direction of the defect is over-permission — the action + * really ran, possibly writing — which is the more dangerous side of the family + * than objectui#3848's over-blocking. + * + * ## What each row detects + * + * • `false` → must now BLOCK. THE defect, and the only row whose behaviour + * this change alters. + * • `true` / a truthy expression / a truthy CEL envelope / absent → still + * runs. Anti-mutation guards: "block whenever the key is present" satisfies + * the `false` row on its own, and these refuse it. + * • a falsy expression / a falsy CEL envelope / a falsy `${…}` template → + * still blocked, unchanged. These are what a correct `condition` already + * did, and the gate must not lose them while learning about `false`. + * • the three EMPTY shapes (`''`, whitespace-only, empty-`source` envelope) → + * still run, now because objectui#3850's ruling says nothing was declared + * rather than because `if ('')` happened to be falsy. Same verdict, sound + * reason; each arrives at "nothing to evaluate" by a different route + * (string identity, `trim()`, envelope `source`), so each is its own + * mutation detector. + * • the non-predicate junk rows (`0`, `{}`) → still run, and NOT because the + * old truthiness test skipped them. See the divergence test below: this is + * a deliberate departure from `ActionEngine.getActionsForLocation`'s + * `Boolean(raw)` junk branch, which would have `0` block here. + * + * ## Scope: what this change does NOT touch + * + * The `disabled` gate below it (objectui#3848 / PR #3873) keeps its semantics + * and its message verbatim; `ActionRunner.disabledGate.test.ts` is its pin and + * is unchanged by this PR. The only shared edit is the module-private + * declaredness helper's key-neutral NAME, now that two gates ask it. + * + * ## Reverse verification (direction predicted before running) + * + * Restoring the truthy gate (`if (action.condition)`) while leaving evaluation + * untouched must turn exactly THREE tests RED, every one of them naming `false`: + * + * 1. the `condition: false` row — the handler runs again, so `blocked` fails; + * 2. the boolean/envelope equivalence test — `false` runs while + * `{ dialect: 'cel', source: 'false' }` is refused, which is precisely the + * divergence objectui#3872 reported; + * 3. "a declared boolean DOES reach the evaluator" — under truthiness it never + * does, so the `evaluateCondition` spy is never called with `false`. + * + * The two purely tabular tests (the changed-row set, the truthiness-vs- + * declaredness table) and both tripwires stay GREEN by construction — they + * assert about `Boolean` / `toPredicateInput` / the engine, not about the gate — + * and every other execution row stays GREEN too, including `0`, `{}` and all + * three empty shapes, because the truthy test and the declaredness test agree on + * every shape except a declared boolean. That is the whole diff, and it is a TIGHTENING: + * this change can only start refusing execution, never start allowing it (the + * mirror image of PR #3873, which could only stop blocking). + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ActionRunner, type ActionContext, type ActionDef } from '../ActionRunner'; +import { ActionEngine } from '../ActionEngine'; +import { ExpressionEvaluator } from '../../evaluator/ExpressionEvaluator'; +import { toPredicateInput } from '../../evaluator/predicateInput'; + +const CONTEXT: ActionContext = { + data: { id: 1 }, + record: { id: 1, status: 'active' }, + user: { id: 'u1', role: 'admin' }, +}; + +interface Shape { + label: string; + /** Omit the key entirely (the "undeclared" row). */ + absent?: boolean; + condition?: unknown; + /** Does the gate refuse to run the handler? */ + blocked: boolean; + /** `true` when this row's verdict CHANGED with objectui#3872. */ + changedBy3872?: boolean; +} + +const SHAPES: Shape[] = [ + // ── the defect: a declared-and-false gate must block ───────────────────── + { + label: 'condition: false (declared "never execute")', + condition: false, + blocked: true, + changedBy3872: true, + }, + // ── unchanged: a declared-and-true gate runs ──────────────────────────── + { label: 'condition: true', condition: true, blocked: false }, + { label: 'condition absent (undeclared)', absent: true, blocked: false }, + // ── unchanged: real predicates keep their verdicts ────────────────────── + { + label: 'condition: bare CEL that is true', + condition: 'user.role == "admin"', + blocked: false, + }, + { + label: 'condition: bare CEL that is false', + condition: 'user.role == "guest"', + blocked: true, + }, + { + label: "condition: { dialect: 'cel', source: 'true' }", + condition: { dialect: 'cel', source: 'true' }, + blocked: false, + }, + { + label: "condition: { dialect: 'cel', source: 'false' }", + condition: { dialect: 'cel', source: 'false' }, + blocked: true, + }, + { + label: 'condition: legacy template that is true', + condition: '${record.status === "active"}', + blocked: false, + }, + { + label: 'condition: legacy template that is false', + condition: '${record.status === "inactive"}', + blocked: true, + }, + // ── unchanged verdict, new reason: nothing was declared ───────────────── + { label: "condition: '' (empty predicate)", condition: '', blocked: false }, + { label: "condition: ' ' (whitespace-only predicate)", condition: ' ', blocked: false }, + { + label: "condition: { dialect: 'cel', source: '' } (empty envelope)", + condition: { dialect: 'cel', source: '' }, + blocked: false, + }, + // ── unchanged: non-predicate junk fails open ──────────────────────────── + { label: 'condition: 0 (not a predicate)', condition: 0, blocked: false }, + { label: 'condition: {} (not a predicate)', condition: {}, blocked: false }, +]; + +/** Execute one shape and report whether the handler ran. */ +async function runShape(shape: Pick) { + const runner = new ActionRunner(CONTEXT); + const onClick = vi.fn(); + const action: Record = { onClick }; + if (!shape.absent) action.condition = shape.condition; + const result = await runner.execute(action as unknown as ActionDef); + return { result, ran: onClick.mock.calls.length > 0 }; +} + +describe('ActionRunner.execute — declared `condition` gate (objectui#3872)', () => { + it.each(SHAPES)('$label', async (shape) => { + const { result, ran } = await runShape(shape); + if (shape.blocked) { + // The handler NOT running is the assertion that matters: objectui#3872 was + // measured by the handler running despite a declared "never execute". + expect(ran).toBe(false); + expect(result).toEqual({ success: false, error: 'Action condition not met' }); + } else { + expect(ran).toBe(true); + expect(result.success).toBe(true); + expect(result.error).toBeUndefined(); + } + }); + + it('changes exactly one shape — the declared boolean `false` (tightening only)', () => { + // Guards the report's per-shape table: if a later edit widens the blast + // radius, this row-set assertion is what says so. + expect(SHAPES.filter(s => s.changedBy3872).map(s => s.label)).toEqual([ + 'condition: false (declared "never execute")', + ]); + // And the direction: the only altered row moved toward REFUSING execution. + expect(SHAPES.find(s => s.changedBy3872)!.blocked).toBe(true); + }); + + it('a declared boolean and its envelope spelling reach the SAME verdict', async () => { + // objectui#3872's headline: these two say the identical thing, and before + // the fix the literal ran while the envelope was refused. + const literal = await runShape({ condition: false }); + const envelope = await runShape({ condition: { dialect: 'cel', source: 'false' } }); + expect(literal.ran).toBe(false); + expect(envelope.ran).toBe(false); + expect(literal.result).toEqual(envelope.result); + + const literalTrue = await runShape({ condition: true }); + const envelopeTrue = await runShape({ condition: { dialect: 'cel', source: 'true' } }); + expect(literalTrue.ran).toBe(true); + expect(envelopeTrue.ran).toBe(true); + }); + + it('an empty predicate does not even reach the evaluator (the gate decides, not the verdict)', async () => { + const runner = new ActionRunner(CONTEXT); + const spy = vi.spyOn( + runner.getEvaluator() as unknown as { evaluateCondition: (c: unknown) => boolean }, + 'evaluateCondition', + ); + const onClick = vi.fn(); + await runner.execute({ condition: '', onClick } as unknown as ActionDef); + expect(onClick).toHaveBeenCalledOnce(); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + + it('a declared boolean DOES reach the evaluator, which short-circuits it', async () => { + // Why the gate needs no boolean branch of its own: asking the right question + // at the door is sufficient, because `evaluateCondition` returns a boolean + // argument verbatim. This is also what makes the fix a two-line change + // rather than a re-implementation of the verdict. + const runner = new ActionRunner(CONTEXT); + const spy = vi.spyOn( + runner.getEvaluator() as unknown as { evaluateCondition: (c: unknown) => boolean }, + 'evaluateCondition', + ); + await runner.execute({ condition: false, onClick: vi.fn() } as unknown as ActionDef); + expect(spy).toHaveBeenCalledWith(false); + spy.mockRestore(); + + const ev = new ExpressionEvaluator(CONTEXT); + expect(ev.evaluateCondition(false)).toBe(false); + expect(ev.evaluateCondition(true)).toBe(true); + }); +}); + +describe('why the `condition` gate cannot ask truthiness (objectui#3872)', () => { + it('truthiness and declaredness disagree on exactly the declared booleans', () => { + // The mechanism in one table. `hasDeclaredPredicate` is module-private, so + // its two ingredients are exercised through their public spellings. + const declared = (v: unknown) => + typeof v === 'string' && v.trim() === '' ? false : toPredicateInput(v) !== undefined; + + // `false` is the divergence: not truthy, yet plainly declared. + expect(Boolean(false)).toBe(false); + expect(declared(false)).toBe(true); + + // Everywhere else the two questions agree, which is why one row changed. + for (const v of ['', ' ', 0, {}, { dialect: 'cel', source: '' }]) { + expect(declared(v), `${JSON.stringify(v)} declares no gate`).toBe(false); + } + for (const v of [true, 'user.role == "admin"', { dialect: 'cel', source: 'false' }]) { + expect(declared(v), `${JSON.stringify(v)} declares a gate`).toBe(true); + expect(Boolean(v)).toBe(true); + } + }); + + it('TRIPWIRE (objectui#3871): normalizing the VERDICT would make every template-spelled condition always execute', () => { + const ev = new ExpressionEvaluator(CONTEXT); + const falsePredicate = '${record.status === "inactive"}'; + // Raw — the correct verdict, and what this gate evaluates. + expect(ev.evaluateCondition(falsePredicate)).toBe(false); + // Normalized — wrapped a second time, unparseable, returned verbatim, truthy. + // On `disabled` this constant `true` blocks everything (PR #3873's tripwire); + // on `condition` it does the opposite and RUNS everything, so this key must + // keep reading the raw value for the same reason. + expect(toPredicateInput(falsePredicate)).toBe('${${record.status === "inactive"}}'); + expect(ev.evaluateCondition(toPredicateInput(falsePredicate) as never)).toBe(true); + // When objectui#3871 is fixed this goes RED on the last two expectations — + // the signal to delete this tripwire and (only then) let the gate evaluate + // the normalized value. + }); + + it('DOCUMENTED DIVERGENCE: the engine `visible` filter coerces junk, this gate does not', () => { + // `ActionEngine.getActionsForLocation` is the in-repo template this fix took + // its shape from, but its non-predicate branch keeps a historical + // `Boolean(raw)` coercion, so `visible: 0` HIDES. This gate deliberately + // does not copy that: `catch { isDisabled = false }` already committed this + // module to fail-OPEN on junk, and a value that is not a predicate must not + // decide an action's fate. Pinned so the next reader sees the difference is + // chosen, not overlooked — objectui#3850's follow-up owns unifying it. + const engine = new ActionEngine(CONTEXT); + engine.registerAction( + { name: 'junk_visible', type: 'script', target: '"ran"', visible: 0 } as unknown as ActionDef, + { locations: ['record_section'] }, + ); + expect(engine.getActionsForLocation('record_section')).toHaveLength(0); + }); +});