Skip to content

service-automation: evaluateCondition still throws a raw TypeError: exprStr.trim is not a function on a non-string envelope source — registration refuses the shape, evaluation faults unattributed #16038

Description

@zhuangjianguo

Found while implementing #15663, outside that card's fix surface and deliberately not fixed there. Filed by the os-dev execution seat, session 01ARYe3yQTQCUFm5qPYNgKaJ. Not assigned; domain:*, type and priority are triage's — this seat does not produce them.

The gap

AutomationEngine.evaluateCondition(expression, variables) (packages/services/service-automation/src/engine.ts:8266) derives its source text as

const exprStr = typeof expression === 'string' ? expression : ((expression as { source?: string })?.source ?? '');

and then calls exprStr.trim() fourteen lines down (:8280). The typeof guard covers only the bare-string arm. When the argument is an envelope whose source is present and not a string, exprStr becomes that non-string value and .trim() throws a bare TypeError.

Measured, driven (worktree at origin/main 2024eca4f)

new AutomationEngine(logger).evaluateCondition({ source: 1 }, new Map())
TypeError: exprStr.trim is not a function
    at AutomationEngine.evaluateCondition (packages/services/service-automation/src/engine.ts:8280:21)

Re-measured on the #15663 branch after that card's fix was in place: unchanged, still throwing. That is the point of filing it separately — see below.

Why #15663's fix does not reach it

#15663 guards the shared @objectstack/formula entry (toSource, read by validateExpression and inferExpressionType), so a non-string source becomes a returned refusal on errors[] there. This crash is in a different package at a different entry: evaluateCondition computes and trims exprStr itself, before it ever calls into @objectstack/formula. No guard at the formula entry can be reached in time. Two doors, two entries.

Why it is worth its own card

  • The reject set of registration and the reject set of evaluation are supposed to be ONE set. That is service-automation: evaluateCondition answers a silent false for a non-string predicate, and a non-string config.condition registers clean #15662's own stated principle, and it is quoted in structuralConditionRefusal's docblock. Registration now refuses this shape: structuralConditionRefusal rejects an object carrying neither a string source nor an ast, so registerFlow throws STRUCTURAL_CONDITION_SHAPE_REFUSAL naming the flow and the node. Evaluation still faults with an unattributed TypeError naming nothing — the two sets disagree in exactly the way that ruling closed at the producer.
  • evaluateCondition is a public method on an exported class, so a plugin can reach it directly regardless of what registerFlow admits.
  • There is at least one in-repo call site that hands it a value it does not own: packages/services/service-automation/src/builtin/logic-nodes.ts:73 calls engine.evaluateCondition({ dialect: 'cel', source: cond.expression }, variables) — whether cond.expression is guaranteed a string on every path into that executor was NOT measured here and is part of what triage should scope.

Why this seat did not fix it in passing

Two reasons, both deliberate:

  1. The fix shape triage ruled for formula: validateExpression throws a raw TypeError: source.trim is not a function, so a bad condition crashes registerFlow with an internal message instead of a located refusal #15663 is a guard at the shared formula entry, once; this is a second entry in the package the adjacent service-automation: evaluateCondition answers a silent false for a non-string predicate, and a non-string config.condition registers clean #15662 lane owns, and that fence was explicit in the dispatch.
  2. It is not mechanical — there are two defensible shapes, and picking one mid-PR would be a decision made by the wrong seat. Either evaluation throws the same STRUCTURAL_CONDITION_SHAPE_REFUSAL registration throws (one refusal, one text, the sets provably equal), or it routes the fault through ADR-0032 §1c's evaluation-fault path with the source attached (consistent with how every other unevaluable condition is reported here). The choice affects what an author sees at run time, so it belongs to triage.

⚠️ Un-measured, stated as a gap rather than left implicit: whether any caller of evaluateCondition relies on the current TypeError, and whether the same unguarded read exists on the sibling value path (evaluateValueEnvelope and friends) rather than only on the predicate path.

Related, not duplicates: #15430 and #15807 concern an ast-only envelope that validates and registers but no engine can evaluate — a different population from a non-string source.

Activity

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