Skip to content

fix(predicate): 空谓词与非谓词值不再是门 —— 收敛最后三处自带范围的判定 (#3955, #3957, #3960) #3977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yinlianghui merged 1 commit into from
Aug 9, 2026
Merged

fix(predicate): 空谓词与非谓词值不再是门 —— 收敛最后三处自带范围的判定 (#3955, #3957, #3960) #3977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions .changeset/predicate-scope-converge-3955-3957-3960.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
---
"@object-ui/core": patch
"@object-ui/react": patch
---

Blank predicates and non-predicate values are no longer gates, at the last three entries that still judged them (objectui#3955, objectui#3957, objectui#3960)

objectui#3850 sank "is a predicate gate DECLARED here?" into one definition,
`@object-ui/core`'s `hasDeclaredPredicate`. Three places were left out of that
ruling's placement clause, each with the same shape of defect: the evaluator's
single default for "there is nothing here to evaluate" is `true`, meaning
*visible/enabled*, and wherever a too-wide "declared" test hands it an empty
predicate on an INVERTED key, that `true` turns a control off for a value the
metadata never used to say anything.

**`SchemaRenderer`'s `hidden` / `hiddenOn` legs (objectui#3955)** asked
`!== undefined` and did NOT negate the verdict, so an empty predicate meant HIDE
and the node disappeared — on the generic rendering path, since that block runs
for every schema type. Harder to diagnose than the `disabled` twin objectui#3862
fixed: a greyed-out control is still on screen, while a node that never rendered
is indistinguishable from metadata that meant to hide it. Both legs now read the
shared definition.

**The "blank" criterion now covers the envelope spelling (objectui#3960).** The
definition trimmed a whitespace-only STRING and not an envelope's whitespace-only
`source`, because `toPredicateInput` folds a `source` of `''` and does not trim.
So `{ dialect: 'cel', source: ' ' }` was a declared gate whose verdict came from
core's own CEL entry calling that exact value "no predicate" (`if (!source.trim())
return true`) — `disabled` greyed out forever and `ActionRunner.execute` answered
`{ success: false, error: 'Action is disabled' }` with the handler never invoked.
Blankness is now decided once for both spellings, at the definition. The
NORMALIZER's contract is deliberately unchanged: "what shape does the evaluator
accept" is not the same question as "is there a condition", and moving the trim
there would have flipped verdicts for every
`useCondition(toPredicateInput(…))` call site, including container-level `visible`
reads that never asked this question at all.

**`ActionEngine.getActionsForLocation`'s `visible` filter (objectui#3957)** was the
last consumer answering the question with a range of its own — three empty
spellings folded by hand, everything else coerced with `Boolean(raw)`. It now reads
the shared definition and the coercion branch is gone, so one value no longer gets
two answers depending on whether an action was surfaced by the engine or rendered
standalone (the invariant objectui#3314 established). Its fail-CLOSED posture on a
predicate that THROWS is untouched (`throwOnError: true` + `warnHiddenPredicate`):
"the predicate faulted" and "there is no predicate" are different facts.

Behaviour changes, before → after. Observation-class: each needs an author to write
an empty/blank predicate or a non-predicate value, and there is no known user path
today.

| value | `ActionEngine` `visible` | `SchemaRenderer` `hidden` | `disabled` (action face + generic path) | `ActionRunner.execute` `disabled` |
|---|---|---|---|---|
| `''` / `null` | shown → shown | HIDDEN → rendered | unchanged | unchanged |
| `' '` (blank text) | HIDDEN → shown | HIDDEN → rendered | unchanged | unchanged |
| `0` / `NaN` | HIDDEN → shown | HIDDEN → rendered | unchanged | unchanged |
| `{}` / `[]` | shown → shown | HIDDEN → rendered | unchanged | unchanged |
| `{ dialect: 'cel', source: '' }` | shown → shown | HIDDEN → rendered | unchanged | unchanged |
| `{ dialect: 'cel', source: ' ' }` | shown → shown | HIDDEN → rendered | GREY → on | refused → runs |
| `{ source: ' ' }` (no dialect) | HIDDEN → shown | HIDDEN → rendered | GREY → on | refused → runs |
| `true` / `false` / bare CEL / `${…}` / a non-blank envelope | unchanged | unchanged | unchanged | unchanged |

Every row moves toward "there is no gate here", never away from it, and no value
that HAS a verdict changes it — a declared `false` is still a verdict, not a
missing gate (objectui#3812), and blankness is `trim()`, not "short": `{ dialect:
'cel', source: ' x ' }` is a predicate. One alias precedence changes with the
`hidden` legs and is pinned rather than claimed as an equivalence: an undeclared
`hidden` no longer short-circuits the chain, so a declared `hiddenOn` is finally
consulted.

`SchemaRenderer`'s four `visible*` legs keep `!== undefined` deliberately, as
objectui#3850 ruled: their `true` is negated, so an empty predicate already lands
on "shown", and narrowing them would change alias precedence rather than fix
anything.
51 changes: 36 additions & 15 deletions packages/core/src/actions/ActionEngine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import { ActionRunner, type ActionDef, type ActionContext, type ActionResult } from './ActionRunner';
import { toPredicateInput } from '../evaluator/predicateInput.js';
import { hasDeclaredPredicate } from '../evaluator/declaredPredicate.js';

/**
* Action location types — re-exported from `@objectstack/spec/ui` so the
Expand Down Expand Up @@ -187,13 +188,18 @@ export class ActionEngine {
* Filtering applied (in order):
* 1. `locations.includes(location)` — location/region match
* 2. `action.visible` — evaluated against the runner's current context
* ({ record, recordId, objectName, user, … }). Missing or `true`
* passes; any other value is coerced to boolean. Evaluator errors
* hide the action (fail-closed) rather than throwing — this matches
* the contract used by every individual action renderer
* (`action-button`, `action-menu`, `action-bar`, …) so the same
* action behaves identically whether surfaced via the engine or
* consumed standalone.
* ({ record, recordId, objectName, user, … }) whenever a gate is
* DECLARED, which is asked through core's one definition
* `hasDeclaredPredicate` (objectui#3850's ruling, adopted here by
* objectui#3957). No gate — absent, `''`, blank predicate text in either
* spelling, an envelope with no evaluable `source`, or a value that is
* not a predicate at all — means visible. Evaluator errors hide the
* action (fail-closed) rather than throwing — this matches the contract
* used by every individual action renderer (`action-button`,
* `action-menu`, `action-bar`, …) so the same action behaves identically
* whether surfaced via the engine or consumed standalone, which is now
* true of the gate in front of the verdict as well as the verdict
* (objectui#3314: one value, one answer, whichever entry reads it).
*
* Predicate normalization goes through the shared `toPredicateInput`
* (`@object-ui/core`'s canonical helper — semantically identical to
Expand Down Expand Up @@ -227,20 +233,35 @@ export class ActionEngine {
})
.filter(ra => {
const raw = ra.action.visible;
if (raw == null || raw === '' || raw === true) return true;
if (raw === false) return false;
// Ask "is a `visible` gate DECLARED?" through the repo's ONE definition
// (`evaluator/declaredPredicate.ts`, objectui#3850's ruling), which is
// what the renderers, `SchemaRenderer` and `ActionRunner`'s execution
// gates ask. This filter used to answer it with a range of its own —
// three empty spellings folded by hand (`null` / `''` / an envelope with
// an empty `source`) and everything else coerced with `Boolean(raw)` —
// and it was the LAST consumer to do so, so the same value got two
// answers depending on which entry read it: `visible: 0` / `NaN` hid the
// action here and showed it on the renderer face, and `visible: ' '`
// hid it here because the normalizer wraps a blank string into `'${ }'`
// whose verdict is falsy. That is the shape objectui#3314's invariant
// forbids, and it is fixed by deleting a range, not by adding one
// (objectui#3957). A value that is not a predicate, or a predicate that
// says nothing, must not be the reason an action is hidden from
// everyone — the same fail-open posture `ActionRunner` already committed
// to for `disabled` (`catch { isDisabled = false }`).
//
// Booleans need no branch of their own: they are DECLARED (`visible:
// false` is a verdict, objectui#3812) and `evaluateCondition`
// short-circuits them.
if (!hasDeclaredPredicate(raw)) return true;
// #3314 — one shared normalization, not two. Hand-rolling the envelope
// unwrap here is what dropped `dialect: 'cel'` and demoted the
// predicate to the legacy JS engine; `toPredicateInput` keeps the
// envelope so `evaluateCondition` can route it to `@objectstack/formula`
// (the engine the server enforces with), exactly as the renderers do.
// Declared ⇒ the normalizer left something to evaluate, so there is no
// `undefined` case left to handle here.
const expr = toPredicateInput(raw);
if (expr === undefined) {
// Not an evaluable predicate: an envelope with an empty `source`
// (→ nothing declared → visible), or a stray non-predicate value,
// which keeps the historical `Boolean(raw)` coercion.
return typeof raw === 'object' ? true : Boolean(raw);
}
try {
// `throwOnError: true` is required for fail-closed semantics: the
// evaluator's default behavior swallows expression errors and
Expand Down
15 changes: 8 additions & 7 deletions packages/core/src/actions/ActionRunner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,13 +593,14 @@ function withIdentityAlias(context: ActionContext): ActionContext {
* boolean, bare CEL, `${…}` template, `{ dialect, source }` envelope — keeps the
* verdict it reaches today.
*
* 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
* and in the shared definition.
* The `visible`-filter template in `ActionEngine.getActionsForLocation` used to
* be the one place this scope was NOT shared: its non-predicate branch kept a
* historical `Boolean(raw)` coercion, so `0` there meant "hidden" — fail-CLOSED
* on junk, the opposite of what this module committed to for `disabled`
* (`catch { isDisabled = false }`). That filter now reads
* {@link hasDeclaredPredicate} too (objectui#3957), so a value that is not a
* predicate at all decides nothing on either key, at either entry: `0` / `{}` are
* "no gate" here, in the engine filter, and in the shared definition.
*/

export class ActionRunner {
Expand Down
130 changes: 130 additions & 0 deletions packages/core/src/actions/__tests__/ActionEngine.visibility.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,18 @@
* 4. Predicate errors fail closed (action hidden), not open.
* 5. `null`/`undefined`/`''`/`true` all mean "always visible".
* 6. Literal `false` always hides.
*
* Since objectui#3957 the "is a gate declared?" half of that contract is not this
* filter's own any more: it reads core's one definition `hasDeclaredPredicate`
* (`evaluator/declaredPredicate.ts`, objectui#3850's ruling), the same one the
* action renderers, `SchemaRenderer` and `ActionRunner`'s execution gates ask. The
* suite at the bottom of this file is that convergence; rules 1-6 above are
* unchanged by it.
*/

import { describe, it, expect, beforeEach } from 'vitest';
import { ActionEngine } from '../ActionEngine';
import { hasDeclaredPredicate } from '../../evaluator/declaredPredicate';
import type { ActionDef } from '../ActionRunner';

function makeEngine(context: any) {
Expand Down Expand Up @@ -365,3 +373,125 @@ describe('ActionEngine.getActionsForLocation — visibility filter', () => {
});

});

/**
* objectui#3957 — the engine filter was the LAST consumer answering "is a
* `visible` gate declared?" with a range of its own.
*
* It folded three empty spellings by hand (`raw == null || raw === '' || raw ===
* true`), passed an envelope with an empty `source` through
* `toPredicateInput`'s fold, and coerced everything else with `Boolean(raw)`. Two
* classes of value therefore got a different answer here than at every other
* entry:
*
* value | engine (before) | renderer face | shared definition
* 0 / NaN | HIDDEN | shown | no gate → shown
* ' ' (blank) | HIDDEN | shown | no gate → shown
* {} / '' | shown | shown | no gate → shown
* {cel, source:''}| shown | shown | no gate → shown
*
* `' '` never even reached the `Boolean(raw)` branch: the normalizer wraps a
* blank string into `'${ }'`, which is not `undefined`, so it was EVALUATED and
* came out falsy. One value, two answers, which is the shape objectui#3314's
* invariant forbids — and it is fixed by deleting a range, not adding one: the
* filter now asks `hasDeclaredPredicate` and the `Boolean(raw)` branch is gone.
*
* Cross-face parity for the same values lives in
* `packages/react/src/hooks/__tests__/actionPredicate.parity.test.tsx`, which can
* see both faces; core cannot import the renderer packages.
*
* ## Reverse verification (direction predicted before running)
*
* Restoring the hand-rolled range (the three folded spellings + `return typeof raw
* === 'object' ? true : Boolean(raw)`) must turn RED exactly the rows this suite
* calls "no gate" that the old range answered differently — `0`, `NaN`, `' '`,
* `'\t\n'`, and the blank-`source` envelopes — and leave GREEN every row the two
* ranges agreed on (`''`, `null`, `undefined`, `{}`, `[]`, the empty-`source`
* envelope) plus every anti-mutation row below. Nothing can go red in the other
* direction: this change only ever stops hiding an action.
*/
describe('ActionEngine `visible` reads the ONE declared-gate definition (objectui#3957)', () => {
const CTX = { record: { id: 'r1', status: 'open' }, user: { id: 'u1' } };

/** Does the filter surface an action carrying this `visible` value? */
function shows(visible: unknown): boolean {
const engine = new ActionEngine({ ...CTX });
engine.registerAction(
{ name: 'probe', type: 'api', visible } as unknown as ActionDef,
{ locations: ['record_section'] },
);
return engine.getActionsForLocation('record_section').length === 1;
}

/**
* Every shape the shared definition calls "nothing to evaluate". `changed`
* marks the ones the engine's own range answered differently — the rows
* objectui#3957 measured as divergent from the renderer face.
*/
const NO_GATE: Array<{ label: string; value: unknown; changed: boolean }> = [
{ label: 'undefined (no key)', value: undefined, changed: false },
{ label: 'null', value: null, changed: false },
{ label: "'' (empty predicate)", value: '', changed: false },
{ label: "' ' (blank predicate text)", value: ' ', changed: true },
{ label: "'\\t\\n' (other blanks)", value: '\t\n', changed: true },
{ label: '0 (not a predicate)', value: 0, changed: true },
{ label: 'NaN (not a predicate)', value: NaN, changed: true },
{ label: '{} (no source)', value: {}, changed: false },
{ label: '[] (array)', value: [], changed: false },
{ label: "{ dialect: 'cel' } (no source key)", value: { dialect: 'cel' }, changed: false },
{ label: "{ dialect: 'cel', source: '' } (what `objectstack build` emits)", value: { dialect: 'cel', source: '' }, changed: false },
{ label: "{ dialect: 'cel', source: ' ' } (blank source — objectui#3960)", value: { dialect: 'cel', source: ' ' }, changed: false },
{ label: "{ source: ' ' } (blank source, no dialect)", value: { source: ' ' }, changed: true },
];

it.each(NO_GATE)('visible: $label → no gate, so the action is surfaced', ({ value }) => {
// Both halves of the claim: the shared definition says "not declared", and
// this filter agrees. Asserting the definition here is what ties the row set
// to `hasDeclaredPredicate` instead of to a list that could drift from it.
expect(hasDeclaredPredicate(value)).toBe(false);
expect(shows(value)).toBe(true);
});

it('the rows whose verdict CHANGED are exactly the ones the engine used to answer alone', () => {
// Guards the report's per-value table; a later edit that widens the blast
// radius fails here rather than passing quietly.
expect(NO_GATE.filter(s => s.changed).map(s => s.label)).toEqual([
"' ' (blank predicate text)",
"'\\t\\n' (other blanks)",
'0 (not a predicate)',
'NaN (not a predicate)',
"{ source: ' ' } (blank source, no dialect)",
]);
});

it('a DECLARED gate is still evaluated in both directions (anti-mutation)', () => {
// "Surface everything" satisfies every case above. These refuse it.
expect(shows(true)).toBe(true);
expect(shows(false)).toBe(false);
expect(shows('record.status == "open"')).toBe(true);
expect(shows('record.status == "closed"')).toBe(false);
expect(shows({ dialect: 'cel', source: 'record.status == "open"' })).toBe(true);
expect(shows({ dialect: 'cel', source: 'record.status == "closed"' })).toBe(false);
// A blank source is "no gate", but one significant character is a predicate:
expect(shows({ dialect: 'cel', source: ' record.status == "closed" ' })).toBe(false);
});

it('a THROWING predicate still fails closed and still warns (the posture is untouched)', () => {
// The one place `visible` is deliberately fail-CLOSED, and the reason this
// change is not "the engine went fail-open": a predicate that FAULTED said
// something the evaluator could not answer, which is a different fact from a
// value that declares nothing. `throwOnError` + `warnHiddenPredicate` stay.
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
const engine = new ActionEngine({ record: { id: 'r1' } });
engine.registerAction(
{ name: 'bare_field', type: 'api', visible: '!done' } as unknown as ActionDef,
{ locations: ['record_section'] },
);
expect(engine.getActionsForLocation('record_section')).toHaveLength(0);
expect(warn.mock.calls.filter(c => String(c[0]).includes('bare_field'))).toHaveLength(1);
} finally {
warn.mockRestore();
}
});
});
Loading
Loading