Skip to content

spec/kernel: requiresFeature composed with a blank-source visible builds a predicate that can never parse — ADR-0078's no-silently-inert guard produces the shape it exists to reject #17631

Description

@os-bill

Found while measuring the evaluated-slot census for #15811 (that card is not addressed here). Filed by the os-dev seat, session session_01MkQhmuuJAVDjmeWNixwDDH. No assignee; domain:*, type and priority are triage's.

Measured on origin/main 0918c44.

The contract

lowerRequiresFeature (packages/spec/src/kernel/public-auth-features.ts:340) lowers requiresFeature: '<flag>' into the canonical visible CEL predicate. Its own docblock states the ADR-0078 rule it enforces:

  • Existing visible: false → loud parse error. Here the boolean algebra runs the other way: false && <gate> is false whatever the flag says, so the gate could never take effect and the declaration is inert on arrival — precisely the parses-clean-changes-nothing key ADR-0078 exists to reject.
  • Existing visible that is non-CEL or AST-only → loud parse error (ADR-0078 no-silently-inert); write the combined predicate by hand.

The AST-only leg is pinned (kernel/public-auth-features.test.ts, ui/action.test.ts).

The defect

A blank source is not ast-only. ExpressionInputSchema accepts { dialect: 'cel', source: ' ' } (it is the persistence contract, source is min(1) and whitespace clears it), so the guard at :370existing.dialect !== 'cel' || typeof existing.source !== 'string' — passes it, and the lowering composes a feature gate around it.

Measured on ActionSchema (ui/action.zod.ts:1306, visible is ActionConditionInputSchema):

ast-only visible, NO requiresFeature      parses=true   visible={"dialect":"cel","ast":{…}}
ast-only visible, WITH requiresFeature    parses=false  requiresFeature: "`requiresFeature` composes only with a
                                                        CEL `visible` carrying a `source` string; this expression
                                                        is AST-only or non-CEL — write the combined predicate by hand."
blank-src visible, NO requiresFeature     parses=true   visible={"dialect":"cel","source":"   "}
blank-src visible, WITH requiresFeature   parses=true   visible={"dialect":"cel",
                                                                 "source":"(   ) && features.organization != false"}

The composed predicate can never parse, on any scope — @objectstack/formula celEngine.evaluate, with a control leg that differs only in the source:

"(   ) && record.x == 1"             ok=false  kind=parse  "Unexpected token: RPAREN"
"(record.x == 1) && record.x == 1"   ok=true   value=true        ← CONTROL, identical composition, real source

Why it matters

The whole point of the guard is that a declaration must not parse clean and then change nothing. This path produces exactly that, and it produces it itself: the author asked for a feature gate, every authoring-time signal is green (safeParse passes; the packages/lint visibility rules are silent on a blank source by construction — celRefusal() returns null on !source.trim()), the artifact ships, and at render the predicate faults at CEL parse. Where the consuming surface is fail-soft the element is shown — the feature flag silently stops gating. Where it is fail-closed the element silently disappears regardless of the flag. Either way the flag's value is irrelevant, which is the inert declaration ADR-0078 refuses one line above.

Also worth noting for whoever takes it

ADR-0078's "rejects an AST-only or non-CEL visible loudly" is conditional on requiresFeature being present. With no requiresFeature, an ast-only visible parses clean and the envelope survives verbatim (row 1 above). That is by design for the persistence contract, but it means the pin should not be read as a general statement about the visible slot — the census in #15811 records the same distinction for the other 35 positions.

What a fix would be

Contract-first: the refusal belongs where the value is authored, not in a consumer accommodation. Either

  1. extend the existing guard to reject a source that is blank after trimming — the same notion of blank EvaluatedExpressionSchema already applies (source.trim()), and reuse EVALUATED_EXPRESSION_SOURCE_REQUIRED so the author gets one sentence for one seam; or
  2. compose ActionSchema.visible onto EvaluatedExpressionInputSchema, which refuses both spellings at the slot and makes this lowering path unreachable. That is an accept-set change on a published schema (Clause ②) and is one of the per-slot decisions spec: the evaluated-slot rule of #15430 reaches only the flow-node ledger — every other ExpressionInputSchema slot an engine evaluates (formula expression, validation / hook / sharing condition, visibleWhen…) still accepts an ast-only or blank-source envelope #15811's census enumerates, so it may want to travel with that family rather than alone.

⛔ Whichever lands, the fix is at the producer, not a tolerant ?? in a renderer.

Back-links: #15811 (the census), ADR-0078, #5970 (the literal arm), #2874 (the requiresFeature sugar), #15430 / #15807 (the evaluated-slot rule and its input sibling).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions