From 46ed7035a21f21c8a4cca12e65879c7b3ee0daa4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:08:16 +0000 Subject: [PATCH 1/4] =?UTF-8?q?wip(#15662):=20structural=20condition=20sha?= =?UTF-8?q?pe=20refusal=20=E2=80=94=20spec=20helper=20+=20both=20validator?= =?UTF-8?q?=20arms?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/lint/src/validate-expressions.ts | 50 +++++++++- .../services/service-automation/src/engine.ts | 40 +++++++- .../automation/flow-node-expression-paths.ts | 94 +++++++++++++++++++ 3 files changed, 176 insertions(+), 8 deletions(-) diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index e26427a804..7859235641 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -79,7 +79,12 @@ */ import { validateExpression, collectCelRootIdentifiers, parseCelToAst, SCOPE_ROOTS } from '@objectstack/formula'; -import { collectFlowGraphs, predicateSlotRefusal, resolveFlowNodeExpressions } from '@objectstack/spec/automation'; +import { + collectFlowGraphs, + predicateSlotRefusal, + resolveFlowNodeExpressions, + structuralConditionRefusal, +} from '@objectstack/spec/automation'; // [#15137] The `value`-role half. Same two published primitives the engine // composes at `registerFlow` (`AutomationEngine.valueEnvelopeRefusals`), in the // same order: the SHAPE rule lives in the spec's `AssignmentValueSchema` (it @@ -1156,12 +1161,44 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { } }; + /** + * [#15662] A STRUCTURAL condition (`config.condition` on a node, + * `edge.condition`) refused on SHAPE before anything reads a source out of + * it. The refusal is the spec's, shared with the engine's `registerFlow` + * pass: `error`, because that pass throws, and a shape build refuses must + * not pass author time. + * + * ⚠️ Deliberately NOT `predicateSlotRefusal`, which the declared-slot arm + * above uses. A ledger `predicate` slot is declared `z.string()`; neither + * structural slot is — `FlowEdgeSchema.condition` is + * `ExpressionInputSchema`, so an envelope is the shape the parse itself + * produces, and a node's `config` is an open `z.record` that passes one + * through verbatim. Both are admitted here; a value that is neither text + * nor an expression is not, because the evaluator reads it as an EMPTY + * condition and answers a silent `false`. + * + * @returns whether the slot was refused, so the caller can skip the + * value-reading passes that would otherwise re-report it as an empty one. + */ + const checkStructuralCondition = (where: string, raw: unknown): { refused: boolean } => { + if (raw == null) return { refused: false }; + const shapeRefusal = structuralConditionRefusal(raw); + if (shapeRefusal) { + issues.push({ where, message: shapeRefusal.message, source: shapeRefusal.source, severity: 'error' }); + return { refused: true }; + } + return { refused: false }; + }; + for (const graph of graphs) { const at = graph.scope ? `flow '${flowName}' · ${graph.scope}` : `flow '${flowName}'`; for (const node of graph.nodes as unknown as AnyRec[]) { const cfg = (node.config ?? {}) as AnyRec; - check(`${at} · node '${node.id}' (${node.type}) condition`, cfg.condition, objectName); - warnShadowedFieldReads(`${at} · node '${node.id}' (${node.type}) condition`, cfg.condition); + const nodeCondWhere = `${at} · node '${node.id}' (${node.type}) condition`; + if (!checkStructuralCondition(nodeCondWhere, cfg.condition).refused) { + check(nodeCondWhere, cfg.condition, objectName); + warnShadowedFieldReads(nodeCondWhere, cfg.condition); + } // Descriptor-declared expression slots (#4027). Before this, the traversal // hardcoded `condition` and assumed every other node string was a `{var}` @@ -1276,8 +1313,11 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { } } for (const edge of graph.edges as unknown as AnyRec[]) { - check(`${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition, objectName); - warnShadowedFieldReads(`${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition); + const edgeCondWhere = `${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`; + if (!checkStructuralCondition(edgeCondWhere, edge.condition).refused) { + check(edgeCondWhere, edge.condition, objectName); + warnShadowedFieldReads(edgeCondWhere, edge.condition); + } } // No `checkNullGuards` on node/edge conditions — and NOT for the reason // #4811 first recorded (#4811 re-measured it). The stated blocker was the diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index f8fc575ee2..3a44f8da78 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -25,7 +25,7 @@ import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, collectFlo // `validate-flow-trigger-readiness`, so the runtime cannot drift from what // authoring accepted. See `resolveTriggerBinding`. import { resolveFlowTriggerKind } from '@objectstack/spec/automation'; -import { predicateSlotRefusal, resolveFlowNodeExpressions } from '@objectstack/spec/automation'; +import { predicateSlotRefusal, resolveFlowNodeExpressions, structuralConditionRefusal } from '@objectstack/spec/automation'; // [#15137] The `value`-role half of the ledger. Both halves of "is this envelope // well-formed?" are IMPORTED, never re-spelled here: the shape rule is // `AssignmentValueSchema` (spec, #14149 — it refuses a non-`cel` dialect and the @@ -7108,6 +7108,40 @@ export class AutomationEngine implements IAutomationService { } }; + /** + * [#15662] The STRUCTURAL condition surfaces — `config.condition` on any + * node and `edge.condition` — refused on SHAPE before anything tries to + * read a source out of them. + * + * ⚠️ Not `predicateSlotRefusal`, the ledger arm's rule, and the + * difference is measured rather than assumed: `FlowEdgeSchema.condition` + * is `ExpressionInputSchema`, whose string arm transforms into + * `{ dialect: 'cel', source }`, so after `FlowSchema.parse` EVERY + * authored edge condition is an envelope — the ledger rule here would + * refuse every conditional edge in every flow. An envelope written at a + * node's `config.condition` is likewise passed through verbatim by the + * open `z.record` and evaluated correctly (#4336). Both are legitimate; + * `structuralConditionRefusal` admits them. + * + * What it refuses is the value that is neither text nor an expression. + * `evaluateCondition` reads `expression?.source ?? ''` and the + * empty-source arm answers `false` — "an unauthored branch must not + * open", applied to a value that was authored — so `42` / `true` / + * `['a']` registered clean and ran silently, on the same key the start + * node's trigger gate is read from. Same severity as a malformed + * predicate (this throws): the reject set of registration and the reject + * set of evaluation must be one set. + */ + const checkStructuralCondition = (where: string, raw: unknown): void => { + if (raw == null) return; + const shapeRefusal = structuralConditionRefusal(raw); + if (shapeRefusal) { + failures.push(` • ${where}: ${shapeRefusal.message}\n source: \`${shapeRefusal.source}\``); + return; + } + check(where, raw); + }; + // #4347 — every graph in the flow, not just the top-level arrays. An // ADR-0031 container keeps a whole sub-graph in its `config`, so // iterating `flow.nodes`/`flow.edges` checked PART of the flow while @@ -7120,7 +7154,7 @@ export class AutomationEngine implements IAutomationService { for (const node of graph.nodes) { const cfg = (node.config ?? {}) as Record; // start-node trigger gate + decision/branch predicates live in config.condition - check(`${at}node '${node.id}' (${node.type}) condition`, cfg.condition); + checkStructuralCondition(`${at}node '${node.id}' (${node.type}) condition`, cfg.condition); // Descriptor-declared expression slots (#4027). The ledger names them // per node type and carries the dialect each one takes, so a declared @@ -7177,7 +7211,7 @@ export class AutomationEngine implements IAutomationService { } } for (const edge of graph.edges) { - check(`${at}edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition as unknown); + checkStructuralCondition(`${at}edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition as unknown); } } diff --git a/packages/spec/src/automation/flow-node-expression-paths.ts b/packages/spec/src/automation/flow-node-expression-paths.ts index 6c8b487777..5e34032f4a 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.ts @@ -386,6 +386,100 @@ export function predicateSlotRefusal(value: unknown): { message: string; source: }; } +/** + * The one sentence a refused **structural** condition leads with (#15662) — + * `config.condition` on any node and `edge.condition`, the two predicate + * surfaces every flow has whether or not any ledger entry names them. + * + * ⚠️ Deliberately NOT {@link PREDICATE_SLOT_STRING_REFUSAL}. That one says + * "bare text, an envelope is not authorable" because a ledger `predicate` slot + * is *declared* `z.string()`. Neither structural slot is: + * + * - `FlowEdgeSchema.condition` is `ExpressionInputSchema`, whose string arm + * **transforms into** `{ dialect: 'cel', source }` — so after + * `FlowSchema.parse` EVERY authored edge condition is an envelope, and the + * ledger arm's rule applied here would refuse every conditional edge in + * every flow. + * - `FlowNodeSchema.config` is an open `z.record`, so an envelope written at + * `config.condition` is passed through by the parse verbatim and evaluated + * correctly by `evaluateCondition` (both spellings, by #4336's ruling). + * + * Both shapes are therefore legitimate here and this refusal admits them. What + * it refuses is the third population, which no layer ever admitted on purpose: + * a value that is neither text nor an expression. + */ +export const STRUCTURAL_CONDITION_SHAPE_REFUSAL = + 'A structural condition (`config.condition` on a node, `edge.condition`) holds either BARE CEL TEXT or an ' + + 'expression envelope — an object carrying a string `source`, or an `ast`. No other shape is authorable there.'; + +/** + * Why a value sitting in a structural condition slot is not authorable at all — + * the SINGLE notion both consumers apply, derived once (#15662). + * + * `undefined` — admitted — for: + * + * - every **string**, including a whitespace-only one. What a non-empty string + * *says* stays `validateExpression('predicate', …)`'s verdict, and a + * whitespace-only condition meaning `false` is consistent on both sides and + * is ruled correct, not a defect. + * - absent / `null`. "Not authored" is not a malformed predicate; both callers + * already return early on it, and this agrees rather than disagreeing. + * - an **expression envelope**: an object carrying a string `source`, or an + * `ast`. That is `ExpressionSchema`'s own rule (`.refine(e => e.source !== + * undefined || e.ast !== undefined)`), read here rather than re-derived, and + * it is the shape `FlowEdgeSchema` produces for every parsed edge condition. + * `dialect` is not required: an envelope without one is CEL, which is what + * `evaluateCondition` already does with it. + * + * ## What it refuses, and what that was doing before + * + * A number, a boolean, an array, or an object that is neither — `{ source: 1 }`, + * `{ dialect: 'cel' }` with no source and no ast, `{}`. `evaluateCondition` + * reads the source as `expression?.source ?? ''` and the empty-source arm + * returns **`false`**: the "an unauthored branch must not open" rule, applied to + * a value that was very much authored. Measured: `42`, `true` and `['a']` at a + * node's `config.condition` each registered clean, executed `success: true`, and + * said nothing anywhere — on the same key the **start node's trigger gate** is + * read from, so a flow could be silently gated shut forever. `{ source: 1 }` + * did not even get that far: it reached `exprStr.trim()` and threw a bare + * `TypeError` out of the validator. + * + * Refusing at the producer is the contract-first half: the flow does not + * register and `objectstack validate` locates it, rather than the reject set of + * registration and the reject set of evaluation being two different sets. + * + * @returns the refusal and the source to attribute it to, or `undefined` when + * the value is authorable and therefore this function's business is done. + */ +export function structuralConditionRefusal( + value: unknown, +): { message: string; source: string } | undefined { + if (value == null) return undefined; + if (typeof value === 'string') return undefined; + if (typeof value === 'object' && !Array.isArray(value)) { + const rec = value as { source?: unknown; ast?: unknown }; + if (typeof rec.source === 'string' || rec.ast !== undefined) return undefined; + } + const found = Array.isArray(value) + ? 'an array' + : typeof value === 'object' + ? 'an object carrying neither a string `source` nor an `ast`' + : `a ${typeof value}`; + // The envelope's own `source`, when it has one, so the finding still points at + // the text the author wrote rather than at an empty string. A non-string + // `source` (the `{ source: 1 }` case) is exactly what is being refused, so it + // cannot be the attribution. + const rawSource = (value as { source?: unknown }).source; + return { + message: + `${STRUCTURAL_CONDITION_SHAPE_REFUSAL} Found ${found}. Write the condition as bare CEL text ` + + '(e.g. `record.rating >= 4`), or as an expression envelope (`{ dialect: \'cel\', source: \'…\' }`). ' + + 'A value that is neither is read by the evaluator as an EMPTY condition, which answers `false` ' + + 'without saying anything — and on a start node that is the trigger gate.', + source: typeof rawSource === 'string' ? rawSource : '', + }; +} + /** * Descend `segments` through `node`, expanding a `key[]` segment over every * element of that array and a `*` segment over every own key of that object, From 13cf43452db98b37892f8d9fe566c624991945c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:17:55 +0000 Subject: [PATCH 2/4] wip(#15662): pins for both arms + controls --- .../lint/src/validate-expressions.test.ts | 128 +++++++++++++++- .../src/structural-condition-shape.test.ts | 143 ++++++++++++++++++ .../flow-node-expression-paths.test.ts | 67 ++++++++ 3 files changed, 337 insertions(+), 1 deletion(-) create mode 100644 packages/services/service-automation/src/structural-condition-shape.test.ts diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 8a3544333d..8d32323f26 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -11,7 +11,11 @@ import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec import { SharingRuleSchema } from '@objectstack/spec/security'; // [#15137] The published refusal sentence a `value`-slot finding must lead // with — asserted from the spec's own export, never re-spelled in a test. -import { ASSIGNMENT_VALUE_ENVELOPE_REFUSAL, PREDICATE_SLOT_STRING_REFUSAL } from '@objectstack/spec/automation'; +import { + ASSIGNMENT_VALUE_ENVELOPE_REFUSAL, + PREDICATE_SLOT_STRING_REFUSAL, + STRUCTURAL_CONDITION_SHAPE_REFUSAL, +} from '@objectstack/spec/automation'; import { validateStackExpressions, @@ -3794,3 +3798,125 @@ describe('assignment value envelope — located findings (#15137)', () => { expect(issues.filter((i) => i.where.includes('assignment value'))).toHaveLength(0); }); }); + +/** + * [#15662] The STRUCTURAL condition arm at author time — `objectstack + * validate`'s half of the refusal `registerFlow` throws. + * + * The gap: `evaluateCondition` reads its source as + * `typeof expression === 'string' ? expression : (expression?.source ?? '')`, + * so a value that is neither a string nor envelope-shaped lands in the + * empty-source arm and returns a silent `false`. Measured on `main`: `42`, + * `true` and `['a']` at a node's `config.condition` registered clean and ran + * `success: true` with nothing reported anywhere — on the same key the start + * node's TRIGGER GATE is read from. + * + * ⚠️ The rule is deliberately NOT `PREDICATE_SLOT_STRING_REFUSAL`. That arm's + * slots are declared `z.string()`; these are not. `FlowEdgeSchema.condition` is + * `ExpressionInputSchema`, so an envelope is a first-class authored spelling + * here, and a node's open `z.record` config passes one through verbatim. The + * controls below are those two shapes staying green. + */ +describe('structural condition shape (#15662)', () => { + const objects = [ + { name: 'crm_lead', fields: { rating: { type: 'number' }, status: { type: 'text' } } }, + ]; + + const flowWith = (opts: { startCondition?: unknown; decisionCondition?: unknown; edgeCondition?: unknown }) => ({ + objects, + flows: [{ + name: 'gate_flow', + nodes: [ + { + id: 'start', type: 'start', + config: { + objectName: 'crm_lead', + ...('startCondition' in opts ? { condition: opts.startCondition } : {}), + }, + }, + { + id: 'branch', type: 'decision', + config: { ...('decisionCondition' in opts ? { condition: opts.decisionCondition } : {}) }, + }, + ], + edges: [{ + id: 'e1', source: 'start', target: 'branch', + ...('edgeCondition' in opts ? { condition: opts.edgeCondition } : {}), + }], + }], + }); + + const condIssues = (opts: Parameters[0], site: string) => + validateStackExpressions(flowWith(opts)).filter((i) => i.where.includes(site)); + + it('RED CONTROL — the brace-trap string on this very arm still reports', () => { + // Not a shape violation, so it cannot be "the answer" to what is measured + // below; it proves this call reaches this slot and can emit. If it ever + // goes to zero, every zero in this block is void. + const control = condIssues({ decisionCondition: '{record.rating} >= 4' }, "node 'branch'"); + expect(control).toHaveLength(1); + expect(control[0].severity).toBe('error'); + expect(control[0].message).toContain('template brace'); + }); + + it('reports every measured silent value on a node condition, naming what it found', () => { + for (const [value, found] of [[42, 'Found a number'], [true, 'Found a boolean'], [['a'], 'Found an array']] as const) { + const issues = condIssues({ decisionCondition: value }, "node 'branch'"); + expect(issues).toHaveLength(1); + expect(issues[0].severity).toBe('error'); + expect(issues[0].message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true); + expect(issues[0].message).toContain(found); + expect(issues[0].where).toContain("flow 'gate_flow'"); + } + }); + + it('reports the same on the START node trigger gate', () => { + const issues = condIssues({ startCondition: 42 }, "node 'start'"); + expect(issues).toHaveLength(1); + expect(issues[0].message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true); + }); + + it('reports the same on an edge condition', () => { + const issues = condIssues({ edgeCondition: ['a'] }, "edge 'e1'"); + expect(issues).toHaveLength(1); + expect(issues[0].message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true); + }); + + it('refuses an object that is neither text nor an expression', () => { + const issues = condIssues({ decisionCondition: { source: 1 } }, "node 'branch'"); + expect(issues).toHaveLength(1); + expect(issues[0].message).toContain('neither a string `source` nor an `ast`'); + // A non-string `source` is what is being refused, so it is never the + // attribution. + expect(issues[0].source).toBe(''); + }); + + it('refuses ONCE — the value-reading passes do not re-report it as an empty condition', () => { + expect(condIssues({ decisionCondition: 42 }, 'condition')).toHaveLength(1); + }); + + describe('CONTROLS — the shapes that must stay accepted', () => { + it('an expression ENVELOPE on a node condition is legitimate here', () => { + expect(condIssues({ decisionCondition: { dialect: 'cel', source: 'record.rating >= 4' } }, "node 'branch'")) + .toHaveLength(0); + // No dialect — `evaluateCondition` reads it as CEL, so does this. + expect(condIssues({ decisionCondition: { source: 'record.rating >= 4' } }, "node 'branch'")) + .toHaveLength(0); + }); + + it('an envelope on an EDGE is legitimate — every parsed edge condition is one', () => { + expect(condIssues({ edgeCondition: { dialect: 'cel', source: 'record.rating >= 4' } }, "edge 'e1'")) + .toHaveLength(0); + expect(condIssues({ edgeCondition: 'record.rating >= 4' }, "edge 'e1'")).toHaveLength(0); + }); + + it('a whitespace-only STRING is untouched — ruled correct, not a defect', () => { + expect(condIssues({ decisionCondition: ' ' }, "node 'branch'")).toHaveLength(0); + expect(condIssues({ edgeCondition: ' ' }, "edge 'e1'")).toHaveLength(0); + }); + + it('a clean bare-CEL condition still passes', () => { + expect(condIssues({ decisionCondition: 'record.rating >= 4' }, "node 'branch'")).toHaveLength(0); + }); + }); +}); diff --git a/packages/services/service-automation/src/structural-condition-shape.test.ts b/packages/services/service-automation/src/structural-condition-shape.test.ts new file mode 100644 index 0000000000..8ad3667758 --- /dev/null +++ b/packages/services/service-automation/src/structural-condition-shape.test.ts @@ -0,0 +1,143 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15662 — a non-string, non-expression value in a STRUCTURAL condition slot + * (`config.condition` on any node, `edge.condition`) is refused at + * `registerFlow` instead of being read as an empty condition. + * + * `evaluateCondition` derives its source as + * `typeof expression === 'string' ? expression : (expression?.source ?? '')`. + * For a value that is neither a string nor envelope-shaped the read yields + * `undefined`, the `??` supplies `''`, and the empty-source arm returns + * `false` — the "an unauthored branch must not open" rule, applied to a value + * that was very much authored. Measured before the fix: `42`, `true` and + * `['a']` at a decision node's `config.condition` each REGISTERED clean and + * executed `success: true` with nothing said anywhere, and the same key on a + * `start` node is the TRIGGER GATE — a flow silently gated shut forever. + * + * ## Why this is not `PREDICATE_SLOT_STRING_REFUSAL` + * + * The ledger arm (#15572) refuses every non-string because those slots are + * declared `z.string()`. Neither structural slot is, and the difference was + * measured rather than assumed: + * + * - `FlowEdgeSchema.condition` is `ExpressionInputSchema`, whose string arm + * TRANSFORMS into `{ dialect: 'cel', source }` — so after `FlowSchema.parse` + * every authored edge condition is an envelope. The ledger rule applied here + * would refuse every conditional edge in every flow. + * - `FlowNodeSchema.config` is an open `z.record`, so an envelope written at + * `config.condition` is passed through verbatim by the parse and evaluated + * correctly by `evaluateCondition` (#4336's ruling: the dialect is decided + * by the SOURCE, so both spellings evaluate the same). + * + * Both are therefore controls that must stay GREEN here, not cases to refuse. + */ +import { describe, expect, it, vi } from 'vitest'; +import { STRUCTURAL_CONDITION_SHAPE_REFUSAL } from '@objectstack/spec/automation'; + +import { AutomationEngine } from './engine.js'; + +const silentLogger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as any; + +/** A two-node flow whose `condition` sites are the ones under test. */ +const flowWith = (opts: { startCondition?: unknown; decisionCondition?: unknown; edgeCondition?: unknown }) => ({ + name: 'gate_flow', + label: 'Gate Flow', + type: 'autolaunched', + status: 'active', + nodes: [ + { + id: 'start', type: 'start', label: 'Start', + config: { + objectName: 'lead', triggerType: 'record-after-update', + ...('startCondition' in opts ? { condition: opts.startCondition } : {}), + }, + }, + { + id: 'branch', type: 'decision', label: 'Branch', + config: { ...('decisionCondition' in opts ? { condition: opts.decisionCondition } : {}) }, + }, + ], + edges: [{ + id: 'e1', source: 'start', target: 'branch', + ...('edgeCondition' in opts ? { condition: opts.edgeCondition } : {}), + }], +}); + +const register = (flow: unknown) => () => new AutomationEngine(silentLogger).registerFlow('gate_flow', flow as never); + +/** The three values the card measured registering clean and answering silently. */ +const SILENT_VALUES: Array<[label: string, value: unknown, found: string]> = [ + ['a number', 42, 'Found a number'], + ['a boolean', true, 'Found a boolean'], + ['an array', ['a'], 'Found an array'], +]; + +describe('#15662 — a structural condition that is neither text nor an expression', () => { + describe('the decision/branch predicate (`config.condition`)', () => { + for (const [label, value, found] of SILENT_VALUES) { + it(`refuses ${label} at registerFlow`, () => { + expect(register(flowWith({ decisionCondition: value }))).toThrow(STRUCTURAL_CONDITION_SHAPE_REFUSAL); + expect(register(flowWith({ decisionCondition: value }))).toThrow(found); + }); + } + + it('names the node in the failure, so the author can find it', () => { + expect(register(flowWith({ decisionCondition: 42 }))).toThrow(/node 'branch' \(decision\) condition/); + }); + }); + + describe('the START node trigger gate — the reason this is not cosmetic', () => { + for (const [label, value] of SILENT_VALUES) { + it(`refuses ${label} on the trigger gate`, () => { + expect(register(flowWith({ startCondition: value }))).toThrow(STRUCTURAL_CONDITION_SHAPE_REFUSAL); + }); + } + + it("names the start node, not the decision one", () => { + expect(register(flowWith({ startCondition: ['a'] }))).toThrow(/node 'start' \(start\) condition/); + }); + }); + + it('refuses `{ source: 1 }` with the refusal instead of a bare TypeError', () => { + // Before the fix this reached `exprStr.trim()` and threw + // `TypeError: source.trim is not a function` out of the validator — a + // refusal by accident, with no location and no rule. + const thrown = (() => { try { register(flowWith({ decisionCondition: { source: 1 } }))(); } catch (e) { return e as Error; } })(); + expect(thrown).toBeDefined(); + expect(thrown!.message).toContain(STRUCTURAL_CONDITION_SHAPE_REFUSAL); + expect(thrown!.message).not.toContain('is not a function'); + }); + + describe('CONTROLS — the shapes that must stay accepted', () => { + it('an expression ENVELOPE at `config.condition` still registers (the blast-radius case)', () => { + expect(register(flowWith({ decisionCondition: { dialect: 'cel', source: '1 == 1' } }))).not.toThrow(); + expect(register(flowWith({ startCondition: { dialect: 'cel', source: '1 == 1' } }))).not.toThrow(); + // No dialect: `evaluateCondition` reads it as CEL, so does this. + expect(register(flowWith({ decisionCondition: { source: '1 == 1' } }))).not.toThrow(); + }); + + it('an envelope on an EDGE still registers — every parsed edge condition is one', () => { + // `FlowEdgeSchema` transforms the authored string into the + // envelope, so this is the shape the arm sees for a plain + // `condition: "…"` too. Both spellings, one control. + expect(register(flowWith({ edgeCondition: { dialect: 'cel', source: '1 == 1' } }))).not.toThrow(); + expect(register(flowWith({ edgeCondition: '1 == 1' }))).not.toThrow(); + }); + + it('a whitespace-only STRING still registers and still evaluates false', () => { + // Ruled correct, stated so nobody "fixes" it. + expect(register(flowWith({ decisionCondition: ' ' }))).not.toThrow(); + expect(new AutomationEngine(silentLogger).evaluateCondition(' ', new Map())).toBe(false); + }); + + it('a bare CEL string still registers, and the brace trap still throws', () => { + expect(register(flowWith({ decisionCondition: 'record.rating >= 4' }))).not.toThrow(); + // RED CONTROL — the pre-existing string verdict is untouched by the + // shape gate. If this ever goes green, the arm stopped reaching + // `check()` and every `not.toThrow()` above is void. + expect(register(flowWith({ decisionCondition: '{record.rating} >= 4' }))) + .toThrow(/template braces/); + }); + }); +}); diff --git a/packages/spec/src/automation/flow-node-expression-paths.test.ts b/packages/spec/src/automation/flow-node-expression-paths.test.ts index af4e05d062..2bce722247 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.test.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.test.ts @@ -22,6 +22,8 @@ import { resolveFlowNodeExpressions, predicateSlotRefusal, PREDICATE_SLOT_STRING_REFUSAL, + structuralConditionRefusal, + STRUCTURAL_CONDITION_SHAPE_REFUSAL, type FlowNodeExpressionPath, type FlowNodeExpressionRole, } from './flow-node-expression-paths.js'; @@ -242,4 +244,69 @@ describe('every pre-#14149 entry resolves byte-identically (the ratchet\'s fixtu expect(predicateSlotRefusal(42)?.source).toBe(''); }); }); + /** + * [#15662] The STRUCTURAL condition arm — `config.condition` on any node and + * `edge.condition`. + * + * The whole point of a second refusal is that it is NOT the ledger arm's + * rule, so the first test here is the one that would fail if somebody + * "unified" them: an expression envelope is legitimate on this arm and must + * be admitted, while `predicateSlotRefusal` refuses it. + */ + describe('structuralConditionRefusal (#15662)', () => { + it('is NOT predicateSlotRefusal — an envelope is legitimate here and refused there', () => { + const envelope = { dialect: 'cel', source: 'record.rating >= 4' }; + // The measured reason: `FlowEdgeSchema.condition` is + // `ExpressionInputSchema`, whose string arm transforms into exactly this + // shape, so after `FlowSchema.parse` every authored edge condition IS an + // envelope. The ledger rule here would refuse every conditional edge. + expect(structuralConditionRefusal(envelope)).toBeUndefined(); + expect(predicateSlotRefusal(envelope)).toBeDefined(); + }); + + it('admits every string — what it SAYS is validateExpression\'s business', () => { + expect(structuralConditionRefusal('record.rating >= 4')).toBeUndefined(); + expect(structuralConditionRefusal('{record.rating} >= 4')).toBeUndefined(); + // Ruled correct, not a defect: a whitespace-only STRING means "not + // authored" on both sides and stays so. + expect(structuralConditionRefusal(' ')).toBeUndefined(); + expect(structuralConditionRefusal('')).toBeUndefined(); + }); + + it('admits an absent condition — "not authored" is not a malformed one', () => { + expect(structuralConditionRefusal(undefined)).toBeUndefined(); + expect(structuralConditionRefusal(null)).toBeUndefined(); + }); + + it('admits an envelope with no dialect, and an ast-only one', () => { + // `evaluateCondition` already treats an envelope with no dialect as CEL, + // and `ExpressionSchema`'s own refine is `source` OR `ast` — read here, + // not re-derived. + expect(structuralConditionRefusal({ source: 'record.rating >= 4' })).toBeUndefined(); + expect(structuralConditionRefusal({ dialect: 'cel', ast: { kind: 'const' } })).toBeUndefined(); + }); + + it('refuses the values measured to register clean and answer a silent false', () => { + for (const [value, found] of [[42, 'a number'], [true, 'a boolean'], [['a'], 'an array']] as const) { + const refusal = structuralConditionRefusal(value); + expect(refusal?.message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true); + expect(refusal?.message).toContain(`Found ${found}`); + expect(refusal?.source).toBe(''); + } + }); + + it('refuses an object that is neither text nor an expression', () => { + // `{ source: 1 }` is the one that did not even reach the silent `false`: + // it threw a bare `TypeError: exprStr.trim is not a function`. + expect(structuralConditionRefusal({ source: 1 })?.message) + .toContain('neither a string `source` nor an `ast`'); + // An envelope carrying neither — `ExpressionSchema`'s refine rejects it + // too, and the evaluator reads it as an empty condition. + expect(structuralConditionRefusal({ dialect: 'cel' })).toBeDefined(); + expect(structuralConditionRefusal({})).toBeDefined(); + // A non-string `source` is exactly what is refused, so it is never the + // attribution. + expect(structuralConditionRefusal({ source: 1 })?.source).toBe(''); + }); + }); }); From 9b5eea3290755a4b0a480f2f818799ddccf05652 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:35:50 +0000 Subject: [PATCH 3/4] changeset + regenerated spec api-surface/export-origins shards for the two new automation exports --- .../structural-condition-shape-refused.md | 17 +++++++++++++++++ packages/spec/api-surface/automation.json | 2 ++ packages/spec/export-origins/automation.json | 2 ++ 3 files changed, 21 insertions(+) create mode 100644 .changeset/structural-condition-shape-refused.md diff --git a/.changeset/structural-condition-shape-refused.md b/.changeset/structural-condition-shape-refused.md new file mode 100644 index 0000000000..a357b8759b --- /dev/null +++ b/.changeset/structural-condition-shape-refused.md @@ -0,0 +1,17 @@ +--- +"@objectstack/spec": minor +"@objectstack/service-automation": minor +"@objectstack/lint": minor +--- + +A flow condition that is neither CEL text nor an expression is now refused at build time, instead of being read as an empty condition and answering a silent `false`. + +`evaluateCondition` derives its source as `typeof expression === 'string' ? expression : (expression?.source ?? '')`. For a value that is neither — a number, a boolean, an array — the read yields `undefined`, the `??` supplies `''`, and the empty-source arm returns **`false`**: the "an unauthored branch must not open" rule, applied to a value that was very much authored. Measured: a `decision` node carrying `config: { condition: 42 }` **registered clean** and executed `success: true` with nothing said at any layer; `{ source: 1 }` did not even get that far and threw a bare `TypeError: exprStr.trim is not a function` out of the validator. `config.condition` is also the key a **start node's trigger gate** is read from, so the same value could gate a whole flow shut forever with no signal to the author. + +- The new `structuralConditionRefusal` / `STRUCTURAL_CONDITION_SHAPE_REFUSAL` in `@objectstack/spec/automation` are the single shared notion of why, read by both validators so build time and author time cannot disagree about the shape. `registerFlow` throws, naming the node or edge and attributing the finding; `objectstack validate` reports the same refusal as a located `error`. + +**This is deliberately NOT the `predicate`-slot rule, and the difference is measured.** A ledger `predicate` slot (`decision.conditions[].expression`, a screen field's `visibleWhen`) is declared `z.string()`, so `PREDICATE_SLOT_STRING_REFUSAL` refuses every non-string including an envelope. Neither structural slot is declared that way: `FlowEdgeSchema.condition` is `ExpressionInputSchema`, whose string arm **transforms into** `{ dialect: 'cel', source }` — so after `FlowSchema.parse` every authored edge condition *is* an envelope — and `FlowNodeSchema.config` is an open `z.record` that passes an envelope written at `config.condition` through verbatim, where `evaluateCondition` evaluates it correctly. Both shapes stay accepted here; an envelope with no `dialect`, and an `ast`-carrying one (`ExpressionSchema`'s own `source`-or-`ast` rule), stay accepted too. + +**Strings are untouched, deliberately.** A whitespace-only condition still means "not authored" and still answers `false` on both sides — consistent behaviour, ruled correct, not a defect. What a non-empty string *says* is still `validateExpression('predicate', …)`'s verdict, brace trap and all. Only the shape moved. + +An app that authored a number, a boolean, an array or a source-less object in a node or edge `condition` now fails to register with a message naming the site; the fix is to write the condition as bare CEL text (`record.rating >= 4`) or as an expression envelope. diff --git a/packages/spec/api-surface/automation.json b/packages/spec/api-surface/automation.json index c430989812..3a9077df7e 100644 --- a/packages/spec/api-surface/automation.json +++ b/packages/spec/api-surface/automation.json @@ -192,6 +192,7 @@ "RetryPolicyParsed (type)", "RetryPolicySchema (const)", "SCHEMALESS_NODE_CONFIG_SCHEMAS (const)", + "STRUCTURAL_CONDITION_SHAPE_REFUSAL (const)", "ScheduleState (type)", "ScheduleStateParsed (type)", "ScheduleStateSchema (const)", @@ -263,6 +264,7 @@ "predicateSlotRefusal (function)", "resolveFlowNodeExpressions (function)", "resolveFlowTriggerKind (function)", + "structuralConditionRefusal (function)", "validateControlFlow (function)" ] } diff --git a/packages/spec/export-origins/automation.json b/packages/spec/export-origins/automation.json index e5eed32d4d..d1c390eac0 100644 --- a/packages/spec/export-origins/automation.json +++ b/packages/spec/export-origins/automation.json @@ -192,6 +192,7 @@ "RetryPolicyParsed": "src/shared/retry-policy.zod.ts#RetryPolicyParsed (type)", "RetryPolicySchema": "src/shared/retry-policy.zod.ts#RetryPolicySchema (const)", "SCHEMALESS_NODE_CONFIG_SCHEMAS": "src/automation/schemaless-node-config.zod.ts#SCHEMALESS_NODE_CONFIG_SCHEMAS (const)", + "STRUCTURAL_CONDITION_SHAPE_REFUSAL": "src/automation/flow-node-expression-paths.ts#STRUCTURAL_CONDITION_SHAPE_REFUSAL (const)", "ScheduleState": "src/automation/execution.zod.ts#ScheduleState (type)", "ScheduleStateParsed": "src/automation/execution.zod.ts#ScheduleStateParsed (type)", "ScheduleStateSchema": "src/automation/execution.zod.ts#ScheduleStateSchema (const)", @@ -263,6 +264,7 @@ "predicateSlotRefusal": "src/automation/flow-node-expression-paths.ts#predicateSlotRefusal (function)", "resolveFlowNodeExpressions": "src/automation/flow-node-expression-paths.ts#resolveFlowNodeExpressions (function)", "resolveFlowTriggerKind": "src/automation/flow-trigger-kind.ts#resolveFlowTriggerKind (function)", + "structuralConditionRefusal": "src/automation/flow-node-expression-paths.ts#structuralConditionRefusal (function)", "validateControlFlow": "src/automation/control-flow.zod.ts#validateControlFlow (function)" } } From 77142e3fa40a1357830e8cc496c55c0e558d2629 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 08:05:48 +0000 Subject: [PATCH 4/4] test(#15662): give the `{ source: 1 }` capture an explicit return type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TS7030 — the IIFE had a fall-through path with no return. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../src/structural-condition-shape.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/services/service-automation/src/structural-condition-shape.test.ts b/packages/services/service-automation/src/structural-condition-shape.test.ts index 8ad3667758..0f3f7288ba 100644 --- a/packages/services/service-automation/src/structural-condition-shape.test.ts +++ b/packages/services/service-automation/src/structural-condition-shape.test.ts @@ -103,7 +103,10 @@ describe('#15662 — a structural condition that is neither text nor an expressi // Before the fix this reached `exprStr.trim()` and threw // `TypeError: source.trim is not a function` out of the validator — a // refusal by accident, with no location and no rule. - const thrown = (() => { try { register(flowWith({ decisionCondition: { source: 1 } }))(); } catch (e) { return e as Error; } })(); + const thrown = ((): Error | undefined => { + try { register(flowWith({ decisionCondition: { source: 1 } }))(); } catch (e) { return e as Error; } + return undefined; + })(); expect(thrown).toBeDefined(); expect(thrown!.message).toContain(STRUCTURAL_CONDITION_SHAPE_REFUSAL); expect(thrown!.message).not.toContain('is not a function');