fix(service-automation): evaluation refuses a malformed condition shape with registration's own refusal (#16438)

`evaluateCondition` derived `exprStr` with a `typeof` guard covering only the
bare-string arm, so an envelope whose `source` was present and not a string
became that value and `.trim()` threw a bare `TypeError` naming no flow, no
node and no expression. Two sibling arms shared the same unguarded read: a
value that is neither text nor envelope-shaped read as an EMPTY condition and
answered a silent `false` — on the same key a start node's trigger gate is read
from — and a malformed envelope under a non-predicate dialect answered `false`
one statement earlier still, at the dialect check.

#15662 closed this reject set at the producer; the evaluator was left
disagreeing with it in a different vocabulary. Per the maintainer's ruling
(decision batch #57, option A) evaluation now calls `structuralConditionRefusal`
— the SAME constructor `registerFlow` calls, not a second hand-written envelope
that could drift — as the method's first statement, above the dialect check so
all three arms are covered.

Controls pinned alongside: bare CEL text and both envelope spellings still
evaluate; an `ast`-only envelope still answers `false` (that population is
#15430/#15807's); a well-formed `cron` envelope still answers `false` rather
than being refused; absent/`null`/empty/whitespace conditions are still "not
authored"; and a malformed STRING still earns the brace trap or the §1c CEL
fault, never the shape refusal.


Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-warren

.changeset/evaluate-condition-shape-refusal.md

@@ -0,0 +1,17 @@
+---
+"@objectstack/service-automation": minor
+---
+
+`evaluateCondition` now refuses a malformed condition shape with the same `STRUCTURAL_CONDITION_SHAPE_REFUSAL` registration already raises — evaluation and registration share one refusal, so a shape that slipped past registration can never surface as a raw `TypeError` or as a silent `false`.
+
+#15662 closed the reject set at the producer: `registerFlow` refuses a structural condition (`config.condition` on a node, `edge.condition`) that is neither CEL text nor an expression envelope. The evaluator was left saying the opposite thing in a different vocabulary, and that half matters because `evaluateCondition` is a **public method on an exported class** — a plugin reaches it directly regardless of what `registerFlow` admits, and a flow stored before that gate landed replays through it.
+
+The unguarded read had three arms, all of them now refused by the shared `structuralConditionRefusal` — the same call `registerFlow` makes, not a second hand-written envelope that could drift from it:
+
+- an envelope whose `source` is present and **not a string** (`{ source: 1 }`, `{ dialect: 'cel', source: 1 }`) reached `.trim()` and threw `TypeError: exprStr.trim is not a function`, naming no flow, no node and no expression;
+- a value that is neither text nor envelope-shaped (`42`, `true`, `['a']`, `{}`, `{ dialect: 'cel' }`) was read as an **empty condition** and answered `false` — the "an unauthored branch must not open" rule applied to a value that was very much authored, on the same key a start node's **trigger gate** is read from;
+- a malformed envelope carrying a non-predicate dialect (`{ dialect: 'cron', source: 1 }`) answered `false` one statement earlier still, at the dialect check, never reaching the source derivation at all.
+
+**What still evaluates is unchanged, and is pinned as controls.** Bare CEL text and both envelope spellings evaluate exactly as before; an `ast`-only envelope still answers `false`; a well-formed non-predicate dialect (`{ dialect: 'cron', source: '0 0 * * *' }`) still answers `false` rather than being refused; absent, `null`, empty and whitespace-only conditions are still "not authored", not malformed. A malformed **string** still earns its own verdict — the brace trap or the ADR-0032 §1c CEL fault — never the shape refusal.
+
+An app whose stored flow carries one of the refused shapes in a node or edge `condition` now fails that run loudly with a message carrying the rule, instead of skipping a branch in silence or faulting unattributed; the fix is to write the condition as bare CEL text (`record.rating >= 4`) or as an expression envelope.

packages/services/service-automation/src/engine.ts

@@ -8709,8 +8709,45 @@ export class AutomationEngine implements IAutomationService {
      * throw: an explicit `dialect: 'cel'` is the author saying "this is CEL", and
      * `{…}` is a map literal there. The sniff only applies where the dialect was
      * never stated.
+     *
+     * ## The shape gate, shared with registration (#16038)
+     *
+     * The FIRST statement, above everything else, is `structuralConditionRefusal`
+     * — the same call `registerFlow` makes on the same slots, not a second
+     * hand-written envelope that would drift from it. #15662 closed the reject
+     * set at the producer; this closes it at the evaluator, so the two are one
+     * set by construction rather than by agreement. It matters because
+     * `evaluateCondition` is a public method on an exported class: a plugin
+     * reaches it directly regardless of what `registerFlow` admits, and a flow
+     * stored before that gate landed replays through here.
+     *
+     * It sits ABOVE the dialect check, not at the `exprStr` derivation, because
+     * the unguarded read has three arms and the derivation is only two of them:
+     * a non-string `source` threw a bare `TypeError: exprStr.trim is not a
+     * function` naming nothing; a value that is neither text nor envelope read
+     * as an EMPTY condition and answered `false` — on the same key a start
+     * node's trigger gate is read from; and a malformed envelope carrying a
+     * non-predicate dialect (`{ dialect: 'cron', source: 1 }`) answered `false`
+     * one statement earlier still, never reaching the derivation at all.
+     *
+     * What it does NOT refuse is what the constructor admits, and those are
+     * controls, not oversights: every string (a malformed one still earns the
+     * #1491 brace trap or the §1c CEL fault below), absent/`null`, and an
+     * envelope carrying a string `source` or an `ast` — the `ast`-only arm
+     * still falls through to `false`, since that population is #15430/#15807's
+     * and not this ruling's.
      */
     evaluateCondition(expression: string | { dialect?: string; source?: string; ast?: unknown }, variables: Map<string, unknown>): boolean {
+        const shapeRefusal = structuralConditionRefusal(expression);
+        if (shapeRefusal) {
+            // ADR-0032 §1d — the error carries its source. `structuralConditionRefusal`
+            // attributes an empty one for the shape it is refusing here (a non-string
+            // `source` cannot be the attribution), which is its documented choice.
+            throw new Error(
+                `condition evaluation error: ${shapeRefusal.message} — source: \`${shapeRefusal.source}\``,
+            );
+        }
+
         const isEnvelope = typeof expression === 'object' && expression != null && 'dialect' in expression;
         const dialect = isEnvelope ? (expression as { dialect?: string }).dialect : undefined;
         const exprStr = typeof expression === 'string' ? expression : ((expression as { source?: string })?.source ?? '');

packages/services/service-automation/src/structural-condition-shape.test.ts

@@ -144,3 +144,154 @@ describe('#15662 — a structural condition that is neither text nor an expressi
         });
     });
 });
+
+/**
+ * #16038 — the EVALUATION half of #15662's principle, ruled by the maintainer
+ * on 2026-09-06 (decision batch #57, option A): `evaluateCondition` refuses a
+ * malformed condition shape with the SAME `STRUCTURAL_CONDITION_SHAPE_REFUSAL`
+ * registration already raises, so a shape that slipped past registration —
+ * older stored data, or a direct caller of this public method on an exported
+ * class — can never surface as a raw `TypeError`, nor as a silent `false`.
+ *
+ * ## The enumeration, measured on `3e7ef9c23` before the fix
+ *
+ * There is ONE unguarded read — `exprStr`, derived at the top of
+ * `evaluateCondition` — and it has three distinct failure arms. Every helper
+ * the method delegates to (`templateHoles`, `celScope`,
+ * `refuseUnresolvedTemplateHole`, `refuseUnresolvedCelOperand`,
+ * `compareValues`) is handed `exprStr` and nothing else, so a guard above the
+ * derivation closes the whole delegation tree. One test per arm, below:
+ *
+ *  - **A — a raw `TypeError`.** An envelope whose `source` is PRESENT and not a
+ *    string, under a predicate dialect. `?? ''` covers only absent/`null`, so
+ *    `exprStr` becomes the non-string value and `.trim()` throws
+ *    `TypeError: exprStr.trim is not a function` — naming no flow, no node and
+ *    no expression. This is the reported arm.
+ *  - **B — a silent `false`.** A value that is neither text nor envelope-shaped:
+ *    the read yields `undefined`, `?? ''` supplies the empty source, and the
+ *    "an unauthored branch must not open" arm answers `false` for a value that
+ *    was very much authored — on the same key a start node's TRIGGER GATE is
+ *    read from.
+ *  - **C — a silent `false` one statement earlier.** A malformed envelope
+ *    carrying a NON-predicate dialect (`{ dialect: 'cron', source: 1 }`) returns
+ *    `false` at the dialect pre-check, BEFORE the trim. This arm is why the
+ *    guard is the method's first statement rather than a patch at the reported
+ *    line: a fix written at `exprStr` never reaches it.
+ *
+ * ## The sibling value path is NOT a site — measured, not assumed
+ *
+ * `evaluateValueEnvelope` already derives its verdict from
+ * `valueEnvelopeRefusals`, the same call `registerFlow` makes (#15137), so it
+ * is already this shape one door over with its own shared constructor. Driven
+ * before the fix, `{ source: 1 }`, `{ dialect: 'cel', source: 1 }`,
+ * `{ dialect: 'cel', source: {} }`, `{ ast, source: 1 }`, `{ dialect: 'cel' }`,
+ * `42`, `['a']` and `{}` each threw an ATTRIBUTED error leading with
+ * `ASSIGNMENT_VALUE_ENVELOPE_REFUSAL` or a located CEL fault — zero raw
+ * `TypeError`s. Nothing to move there, which is why nothing here does.
+ *
+ * ## No caller depended on the `TypeError`
+ *
+ * Also measured, because the ruling's landing shape turns on it: repo-wide,
+ * every occurrence of `is not a function` on this path is PROSE recording the
+ * pre-fix symptom, never an assertion and never a `catch` that branches. The
+ * two engine-internal callers (the start gate and the edge gate) call it bare,
+ * so a throw propagates to `execute()`'s catch and is recorded as a loud flow
+ * failure — ADR-0032 §1c's prescribed handling, not a regression.
+ */
+const REFUSED_AT_EVALUATION: Array<[label: string, value: unknown, arm: string]> = [
+    // Arm A — reached `.trim()` and threw a bare `TypeError`.
+    ['`{ source: 1 }` (the reproduction)', { source: 1 }, 'A'],
+    ['a `cel` envelope with a number source', { dialect: 'cel', source: 1 }, 'A'],
+    ['a `cel` envelope with an object source', { dialect: 'cel', source: {} }, 'A'],
+    ['a `template` envelope with a number source', { dialect: 'template', source: 1 }, 'A'],
+    // Arm C — returned `false` at the dialect pre-check, one statement earlier.
+    ['a non-predicate-dialect envelope with a number source', { dialect: 'cron', source: 1 }, 'C'],
+    // Arm B — returned `false` off the empty-source arm.
+    ['a number', 42, 'B'],
+    ['a boolean', true, 'B'],
+    ['an array', ['a'], 'B'],
+    ['an object that is neither', {}, 'B'],
+    ['an envelope with no source and no ast', { dialect: 'cel' }, 'B'],
+];
+
+describe('#16038 — evaluation refuses the same shapes registration does', () => {
+    const evaluate = (value: unknown) => () =>
+        new AutomationEngine(silentLogger).evaluateCondition(
+            value as never,
+            new Map<string, unknown>([['record', { rating: 5 }]]),
+        );
+
+    for (const [label, value, arm] of REFUSED_AT_EVALUATION) {
+        it(`refuses ${label} (arm ${arm})`, () => {
+            expect(evaluate(value)).toThrow(STRUCTURAL_CONDITION_SHAPE_REFUSAL);
+        });
+    }
+
+    it('never answers a malformed shape with a bare TypeError again', () => {
+        // The filed symptom, asserted as an absence: the refusal must REPLACE
+        // the `TypeError`, not sit beside it. Without this an implementation
+        // that threw the refusal only on the `false` arms would pass every
+        // assertion above except the arm-A rows.
+        for (const [, value] of REFUSED_AT_EVALUATION) {
+            expect(evaluate(value)).not.toThrow('is not a function');
+        }
+    });
+
+    it('attributes the refusal — ADR-0032 §1d, the error carries its source', () => {
+        expect(evaluate({ source: 1 })).toThrow(/source:/);
+    });
+
+    /**
+     * The property the ruling is actually about, asserted mechanically rather
+     * than described: ONE population walked through BOTH doors, refused by both
+     * with the same published sentence. Two hand-written envelopes that drifted
+     * apart would fail here while every per-site test above stayed green.
+     */
+    it('the reject set of registration and the reject set of evaluation are ONE set', () => {
+        for (const [label, value] of REFUSED_AT_EVALUATION) {
+            expect(register(flowWith({ decisionCondition: value })), `registration: ${label}`)
+                .toThrow(STRUCTURAL_CONDITION_SHAPE_REFUSAL);
+            expect(evaluate(value), `evaluation: ${label}`)
+                .toThrow(STRUCTURAL_CONDITION_SHAPE_REFUSAL);
+        }
+    });
+
+    describe('CONTROLS — the shapes evaluation must still answer, not refuse', () => {
+        it('bare CEL text, and both envelope spellings, still evaluate', () => {
+            expect(evaluate('record.rating >= 4')()).toBe(true);
+            expect(evaluate({ source: 'record.rating >= 4' })()).toBe(true);
+            expect(evaluate({ dialect: 'cel', source: 'record.rating >= 4' })()).toBe(true);
+        });
+
+        it('an `ast`-only envelope still answers `false` — that population is #15430/#15807', () => {
+            // `structuralConditionRefusal` admits an `ast`, so this must fall
+            // through to the empty-source arm exactly as before. If this ever
+            // throws, the guard swallowed a different card's population.
+            expect(evaluate({ dialect: 'cel', ast: { kind: 'const' } })()).toBe(false);
+        });
+
+        it('a WELL-FORMED non-predicate dialect still answers `false`, not a refusal', () => {
+            // The arm-C boundary: `cron` is not a boolean predicate here, but a
+            // string source makes the SHAPE authorable, so the pre-existing
+            // `false` stands. Only the malformed spelling moved.
+            expect(evaluate({ dialect: 'cron', source: '0 0 * * *' })()).toBe(false);
+        });
+
+        it('an unauthored condition is not a malformed one', () => {
+            expect(evaluate(null)()).toBe(false);
+            expect(evaluate(undefined)()).toBe(false);
+            expect(evaluate('')()).toBe(false);
+            expect(evaluate('   ')()).toBe(false);
+        });
+
+        it('a malformed STRING still earns its own verdict, not the shape refusal', () => {
+            // RED CONTROL — the shape gate must not shadow the #1491 brace trap
+            // or the §1c CEL fault. If this goes green the guard is refusing
+            // strings, which `structuralConditionRefusal` admits by design.
+            expect(evaluate({ dialect: 'cel', source: '{record.rating} >= 4' }))
+                .toThrow(/template braces|failed to evaluate as CEL/);
+            expect(evaluate({ dialect: 'cel', source: '{record.rating} >= 4' }))
+                .not.toThrow(STRUCTURAL_CONDITION_SHAPE_REFUSAL);
+        });
+    });
+});