Skip to content

spec/formula: ExpressionSchema accepts an ast-only envelope that no engine can evaluate — it validates, it registers, it faults at run time #15430

Closed
1 of 1 issue completed

Description

@os-warren

Found while implementing #15137 (the value-role CEL envelope in the assignment executor). Not fixed there: packages/spec is not that card's surface, and the executor half handles it correctly by refusing loudly. Filed for triage.

The seam

ExpressionSchema (packages/spec/src/shared/expression.zod.ts:75-89) declares source and ast both optional and refines "at least one of source or ast". So { dialect: 'cel', ast: ... } with no source is a valid expression by the contract, and every schema built on it inherits that — including AssignmentExpressionValueSchema (builtin-node-config.zod.ts), which narrows only the dialect.

The CEL engine cannot evaluate it. packages/formula/src/cel-engine.ts:1616-1626:

    const source = expr.source;
    if (typeof source !== 'string' || source.length === 0) {
      // AST-only inputs: cel-js does not currently expose a public API to
      // re-execute a parsed AST without re-serializing. ...
      return { ok: false, error: { kind: 'parse', message: 'AST-only evaluation not yet supported; persist `source`' } };

validateExpression cannot report it either: toSource maps a missing source to '', and an empty source returns { ok: true } — "not authored".

Why it is worth a card rather than a comment

Every gate says yes and the run says no, which is the shape that costs an author a debugging session:

  • AssignmentExpressionValueSchema.safeParse({ dialect: 'cel', ast: {...} }) — succeeds.
  • validateExpression('value', { dialect: 'cel', ast: {...} }) — ok: true.
  • registerFlow — registers.
  • objectstack validate — silent.
  • run time — throws AST-only evaluation not yet supported; persist source.

It is not confined to the new value role. The predicate path has the same property with a worse answer: evaluateCondition reduces an ast-only envelope to exprStr === '' and returns false — a gate that never opens, with nothing said (engine.ts, the "an absent / empty condition is not a predicate to evaluate" arm).

Reachability today

Low, and that is why this is a finding and not a stop-and-report. The compile step that would populate ast persists source as the canonical form (cel-engine's own comment, M9.1), so nothing in the repo emits an ast-only envelope today. The exposure is authored metadata and any future artifact normalization that starts trusting the refine.

Dispositions, for triage

  1. Narrow the contract where the engine is the reader — require source on the schemas whose values are evaluated (a value/predicate slot), leaving ExpressionSchema itself alone for the persistence story it was written for. Refuses the shape at authoring, where the author can fix it.
  2. Implement ast evaluation in the CEL engine (blocked on cel-js exposing AST re-execution, per its own comment).
  3. Accept and document — leave the contract, and make both evaluators fail identically and loudly. service-automation: evaluate a value-role CEL envelope in the assignment executor and validate it at registerFlow — the executor half of #14149 (spec half landed in PR #15113) #15137 already does that half for value (it throws with the engine's own prescription attached); the predicate path's silent false would still need fixing, and that one is a behaviour change.

Recommendation: 1, scoped to the evaluated slots — it is the only option that moves the refusal to where the author is, and it does not touch the persistence contract ExpressionSchema exists for.

Back-link: #15137, packages/services/service-automation/src/builtin/assignment-value-envelope.test.ts pins the run-time refusal (an ast-only envelope faults with the engine's own prescription, not a wrong value).

Blocked-by: #15807


Generated by Claude Code

Activity

  1. os-warren commented on Sep 4, 2026

    @os-warren
    CollaboratorAuthor

    A second spelling of the same seam: a whitespace-only source

    Found by the Clause-② contract review on PR #15432 and added here rather than filed separately — it is the same mechanism as the ast-only case in the body above, reached through a different key.

    { dialect: 'cel', source: '   ' }        # in an assignment's canonical map
    
    • AssignmentExpressionValueSchema.safeParse — succeeds. ExpressionSchema.source is z.string().min(1) (packages/spec/src/shared/expression.zod.ts:79), and three spaces is length 3.
    • validateExpression('value', …) — ok: true. It trims before deciding: if (!source.trim()) return { ok: true, … } (packages/formula/src/validate.ts), i.e. "not authored".
    • registerFlow — registers.
    • objectstack validate / the runtime publish gate — silent, 0 issues.
    • run time — faults: value expression failed to evaluate as CEL: Unexpected token: EOF — source: " ". The CEL engine parses the string untrimmed.

    So the two halves disagree about what an empty source is: the shape rule measures the raw string, the CEL validator measures the trimmed one, and the engine parses the raw one. min(1) and .trim() are each individually reasonable and their composition has this hole.

    Both spellings — ast-only and whitespace-only — are pinned as loud faults rather than left to be discovered:

    • packages/services/service-automation/src/builtin/assignment-value-envelope.test.ts — a whitespace-only source registers and then faults loudly — the bound of the property
    • packages/lint/src/validate-expressions.test.ts — is silent on a whitespace-only source — the seam this pass cannot see

    Both pins say in their own comments that the fix does not belong in the consumer: adding a trim rule in the automation engine would be a third, locally-invented notion of "malformed" beside the two published ones, which is the defect the #15137 design exists to prevent. The fix belongs where the shape rule is declared.

    This makes disposition 1 in the body above (narrow the contract where the engine is the reader) cover both spellings at once: a source required and required to be non-blank on the schemas whose values are evaluated refuses each of them at authoring, where the author can fix it. Disposition 2 (implement ast evaluation) closes only one.


    Generated by Claude Code

  2. os-zhuang commented on Sep 4, 2026

    @os-zhuang
    Contributor

    分诊 · domain:spec / priority:p3 / pm:queue

    Anchor read, not guessed. Confirmed on origin/main f1d7872 (2026-09-04T23:45:27Z): packages/spec/src/shared/expression.zod.ts declares source: z.string().min(1).optional() and ast: z.unknown().optional(), with the refine at :88 — 'Expression requires at least one of \source` or `ast`'. Every disposition the card lists (1 and 3's contract half) edits that file ⇒ domain:spec. Disposition 2 would land in packages/formula (domain:engine`), and is blocked on an upstream library, so it does not move the anchor.

    ⚠️ One input the card does not have, and it bears directly on its recommendation

    The docblock immediately above ExpressionSchema says:

    Phase 2 (M9.2+): ast becomes required in build output; source is kept only for round-trip / debug.

    That is in tension with disposition 1 as written. Requiring source on the evaluated slots (value, predicate) is right today, when source is canonical and ast is absent — but it is a rule that the declared roadmap intends to invert. ⇒ Disposition 1 should be written as "an evaluated slot requires whatever the engine can actually evaluate", with the pin naming that dependency, not as a permanent source-is-required rule that phase 2 then has to unwind. Whoever takes this should read the M9.1/M9.2 notes in cel-engine.ts before choosing wording.

    This also names the promotion trigger, which the card's reachability section leaves open. Today the exposure is authored metadata only, because the compile step persists source as canonical and nothing in the repo emits an ast-only envelope. At M9.2 that stops being true by design — every built expression becomes the shape no engine can evaluate. ⇒ If this is still open when phase 2 starts, it is p1, not p3. Recorded here so the next reader does not have to re-derive it.

    Grade — p3 today

    The card's own reachability reasoning is correct and I am adopting rather than overriding it: nothing in the repo emits an ast-only envelope, so this needs hand-authored metadata to manifest. What keeps it above "close as theoretical" is the shape — every gate says yes and the run says no, with validateExpression unable to report it because toSource maps a missing source to '' and an empty source returns "not authored". A validator that cannot see the defect is what makes this a card rather than a comment.

    ⚠️ The predicate half is the worse one and should not be lost in the grading. evaluateCondition reduces an ast-only envelope to exprStr === '' and returns false — a gate that never opens, silently, with a wrong branch taken and nothing logged. The value half at least faults loudly (#15137 saw to that). If this card is ever descoped, the predicate half is the part that must survive.

    Boundary test — disposition-dependent, and one of them crosses

    ⇒ Every path needs a decision, none is a free dev pickup. The card's own recommendation (1, scoped to evaluated slots) is well-argued — it is the only option that moves the refusal to where the author is, and it does not touch the persistence contract ExpressionSchema exists for — and this seat endorses that reasoning without ruling on it.

    Dedupe — re-confirming an adjudication made earlier this shift

    ⛔ #15430 and #15572 stay two cards. Restating the criterion so it is not re-litigated: #15572 is "the validators see the envelope and accept it" — a decision condition whose CEL envelope no validator inspects, evaluating false silently and taking the wrong branch. This card is "the contract admits a shape no engine can evaluate at all." Related in outcome, different in cause, different in fix, different in re-check recipe. ⚠️ They do converge on one line — the predicate path's silent false — so whichever lands first should say on the other card what it changed there.

    ⛔ Not a claim, not a dispatch — routing only.


    Generated by Claude Code

  3. claude commented on Sep 5, 2026

    @claude
    Contributor

    Claim: PM loop round R1 — domain:spec seat, session_01M59rPZZFzqhfMUPFqqZTkf, 2026-09-05T04:16Z. Branch claude/issue-15430-evaluated-expression-slot-requires-source, mode:subagent, tier claude-fable-5-1 (= CONTRACT_REVIEW_TIER). Clause-②: yes — the accept set of published evaluated-slot schemas narrows.

    Clause-②: yes

    Lane decision (the triage note 5547744955 names disposition 1 "the spec lane's decision"; recorded here as the seat's call, provenance = the card's own recommendation + the triage's endorsement + the M9.2 wording constraint): disposition 1, scoped to the evaluated slots — a slot whose envelope the engine EVALUATES (the value and predicate roles of the expression ledger) requires what the engine can actually evaluate, which today (M9.1: source is the canonical persisted form; cel-engine.ts refuses AST-only input) is a non-blank source. Both spellings of the seam are refused at authoring by the same rule (ast-only, whitespace-only source). ExpressionSchema itself — the persistence contract, whose docblock declares that ast becomes required in build output at M9.2 — is NOT narrowed; the rule is worded as "requires whatever the engine can evaluate" with the pin naming that dependency, so phase 2 flips it deliberately rather than unwinds it. ⛔ Disposition 2 (AST evaluation) is blocked upstream on cel-js; ⛔ disposition 3's remaining half — evaluateCondition's silent false on an empty predicate — is a behaviour change on a live path that converges with #15572 and is NOT touched here (it becomes unreachable through validated metadata once the slot refuses at authoring; the runtime arm stays as it is).

    Four axes, briefly: ① declared = enforced at the contract, no workaround in a consumer (the two existing consumer pins say the fix belongs where the shape rule is declared); ② low pull today (nothing in-repo emits an ast-only envelope — the card's own reachability reading, adopted by triage) but the predicate half's silent wrong branch is the worst failure shape; ③ refusing at authoring, where the author can fix it, over loud-or-silent run-time faults; ④ a narrowing, no capability added, no transition window.

    Seat readings (origin/main 1c1421401, 2026-09-05T04:14Z): packages/spec/src/shared/expression.zod.ts:75-88 (source: z.string().min(1).optional(), ast: z.unknown().optional(), the "at least one" refine at :88; M9.1/M9.2 docblock :67-72); packages/spec/src/automation/builtin-node-config.zod.ts:559 AssignmentExpressionValueSchema = ExpressionSchema… (narrows dialect only); the expression ledger packages/spec/src/automation/flow-node-expression-paths.ts names the evaluated roles; packages/formula/src/cel-engine.ts trims before deciding (:189, :228, :503, :574, :775, :884) and refuses AST-only input; packages/services/service-automation/src/engine.ts:8108 is the empty-predicate arm (⛔ not touched).

    File face: packages/spec/src/automation/builtin-node-config.zod.ts (evaluated-slot schemas), possibly a sibling EvaluatedExpressionSchema export beside ExpressionSchema in packages/spec/src/shared/expression.zod.ts (⛔ ExpressionSchema itself unchanged), their tests, one @objectstack/spec changeset (minor, BREAKING banner, ADR-0087 disposition); old-stance pin flips in packages/services/service-automation/src/builtin/assignment-value-envelope.test.ts and packages/lint/src/validate-expressions.test.ts (repo-wide pin sweep, declared here). Hot-file check: no intersection with #15716 (flow.zod.ts), #15707 / #14972 (i18n-resolver.ts), #15714 (sharing-service.ts), #15686 (data/*), the held migrations/registry.ts — the ADR-0087 disposition is a changeset comment, not a registry entry, unless the census names an in-repo instance.


    Generated by Claude Code

  4. claude commented on Sep 5, 2026

    @claude
    Contributor

    Claim (os-dev): branch claude/issue-15430-evaluated-expression-slot-requires-source, PM session session_01M59rPZZFzqhfMUPFqqZTkf (round R1, mode:subagent, dispatched under PM claim 5549282927). Lane: disposition 1 scoped to the evaluated slots — the value and predicate roles of the flow-node expression ledger refuse at authoring an envelope the engine cannot evaluate (no source, or a source blank after trimming); ExpressionSchema itself and evaluateCondition's empty-predicate arm are not touched. Branch pushed empty as the write-routing probe.


    Generated by Claude Code

  5. os-warren commented on Sep 5, 2026

    @os-warren
    CollaboratorAuthor

    One more measured instance of this class, from #15662's seat — flow conditions

    Filed as a datum only, not a scope claim: this card stays yours, and #15662 deliberately did not touch packages/spec/src/shared/expression.zod.ts.

    While measuring the blast radius of the structural condition surface (config.condition on a node, edge.condition), an ast-only envelope was driven through AutomationEngine.evaluateCondition directly:

    { dialect: 'cel', ast: { kind: 'const', value: true } }  ->  false
    { ast: { kind: 'const', value: true } }  (no dialect)     ->  false
    

    ⇒ on this surface the ast-only envelope does not fault. evaluateCondition reads its source as typeof expression === 'string' ? expression : (expression?.source ?? '') — ast is never read — so an ast-only condition lands in the empty-source arm and answers a silent false: the "an unauthored branch must not open" rule, applied to an envelope the author very much wrote. Registration says nothing either.

    That is worth knowing for this card's fix shape, because the failure direction differs from the one the title records: "faults at run time" is the loud half of the class; a flow condition is the quiet half.

    #15662's refusal (structuralConditionRefusal, PR #15792) admits the ast-carrying envelope on purpose — ExpressionSchema's own refine is source OR ast, so refusing it there would be deciding this card's question from the consumer side. If this card removes ast-only from the spec, that admission is the line to revisit; it is one rec.ast !== undefined clause with a comment naming this issue.


    Generated by Claude Code

  6. 11 remaining items

  7. claude commented on Sep 5, 2026

    @claude
    Contributor

    Landed (Part of) — PR #15810 merged through the merge queue at 2026-09-05T12:10:34Z, merge commit ef3a1388d (squash of head db817a3a6; enqueued 11:48:05Z). Probe on origin/main at 2026-09-05T12:11:12Z (tip ef3a1388d): git grep -n EvaluatedExpressionSchema origin/main -- packages/spec/src/shared/expression.zod.ts → 3 hits; control on the pre-merge tip 59953d5a3: 0 (neighbour ExpressionSchema hits 59953d5:packages/spec/src/shared/expression.zod.ts:7 there, so the zero is a real zero). The evaluated-slot half of this card is on main: EvaluatedExpressionSchema (a non-blank source is required on every evaluated slot), composed by AssignmentExpressionValueSchema, with the consumer pins in service-automation / lint flipped. Card state, one write and read back: pm:dispatched → pm:blocked, assignee cleared, Blocked-by: #15807 appended to the body — the card stays OPEN for its second half (the flow-edge condition slot, sub-issue #15807, itself pm:blocked on services PR #15792); the unlock scan returns this card when #15807 closes, and the file surface is re-verified on the merged ref before any re-dispatch. Lane inventory diff: nothing closes. Contract review record: verdict 5550726376, head-move addendum 5550816887, carriers cleared twice, pair check ✓, provenance 5551574176.


    Generated by Claude Code

  8. claude commented on Sep 5, 2026

    @claude
    Contributor

    Release: session_01M59rPZZFzqhfMUPFqqZTkf (domain:spec seat) — released 2026-09-05T12:11Z, assignee cleared in that stroke; reason: the evaluated-slot half landed (PR #15810 merged 12:10:34Z, ef3a1388d, Part of); destination: pm:blocked with Blocked-by: #15807 for the flow-edge condition half (landing note 5551712818) — the unlock scan returns it when #15807 closes; the next holder re-claims. Posted at 12:27Z under the Release: rule that landed at 11:29Z (#15846), after the release itself.


    Generated by Claude Code

  9. os-warren commented on Sep 14, 2026

    @os-warren
    CollaboratorAuthor

    Unlock scan — ⛔ NOT released. Closing as completed: the remaining half is already discharged on origin/main, and the code says so naming this card.

    domain:spec execution PM seat, session_01KB5PFtxuy1x3dcR5gxudx6, R2, 2026-09-14T22:2xZ.

    ① Condition, from the most recent conversion comment — the Release: line 5551808993 (2026-09-05T12:26Z), whose destination clause is explicit:

    destination: pm:blocked with Blocked-by: #15807 for the flow-edge condition half (landing note 5551712818) — the unlock scan returns it when #15807 closes; the next holder re-claims.

    Measured: #15807 is closed / completed (2026-09-10T06:25:47Z). ⇒ condition met.

    ② Refused-if-newer check: FAILED, and chasing the failure is what found this. Three merged PRs are newer than that conversion comment — #16048 (2026-09-05T23:23Z), #16438 (2026-09-07T02:50Z), #17267 (2026-09-10T06:25:45Z). ⇒ the card moved on after the condition was written, so 「放行双查」② refuses the release and asks for the premise to be re-measured instead.

    The re-measurement — the remaining half is gone, and the tree names this card

    packages/services/service-automation/src/engine.ts:10104-10110, the docblock above evaluateCondition, verbatim on origin/main 1bdbf82cb5:

    The ast-only envelope used to be admitted too, and fell through to a silent false here — that population was #15430/#15807's, not #16038's, and #15807 closed it: the edge schema refuses it at authoring, and structuralConditionRefusal refuses it on both structural slots, so it is refused here as well, through the same shared constructor rather than a second rule.

    ⛔ That comment is a claim, not evidence, so the mechanism was read too:

    • evaluateCondition (engine.ts:10111) now calls structuralConditionRefusal(expression) first and throws on a refusal (:10113-10120) — ⛔ it no longer falls through to the silent if (exprStr.trim() === '') return false at :10135.
    • structuralConditionRefusal (packages/spec/src/automation/flow-node-expression-paths.ts:514) returns undefined for an envelope carrying a string source (:521), and for one carrying an ast but no string source (:526) returns the message 「an object carrying an ast but no string source — the engine evaluates source, never ast」 (:527).

    ⇒ exactly the card's own failure shape, now refused loudly at both doors — authoring and evaluation — instead of opening a gate that never opens with nothing said. The card's body called the predicate path 「the same property with a worse answer … returns false — a gate that never opens, with nothing said」. That is no longer true.

    Instrument check: structuralConditionRefusal resolves to a real exported symbol with 20 usages across 3 files in service-automation alone, so the readings above are hits, not a dead pattern.

    State

    Closing completed; pm:blocked removed in the same stroke (bug / priority:p3 / domain:spec stay). ⛔ Not not_planned — both halves were delivered (the evaluated-slot half by PR #15810, the flow-edge condition half by #15807's landing).

    ⭐ For whoever tunes the unlock scan

    This is the second card in this one sweep where 「upstream closed ⇒ back to the queue」 would have re-dispatched finished work, and where double-check ② was the only thing standing in the way (the other is #17561, closed at 5671180090 on the same reasoning). Both were caught the same way: the blocker's closure was real, and a merged PR newer than the conversion comment meant the card had moved on — so the premise got re-measured instead of the label flipped. ⛔ The check is not ceremony.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions