From 3f57e0af55d1143ba7694089743d47c9431a61e7 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 18 Aug 2026 22:09:50 +0200 Subject: [PATCH 1/2] feat(workflow-executor): evaluate deterministic condition steps without AI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decision steps in the new Automatic mode carry their branching logic as build-time preRecordedArgs (optionConditions + fallbackOption, wire-final operator names from the PRD-472 contract). The executor now resolves each condition's value from the run's Get Data outputs and evaluates top-to-bottom, first-match-wins — never calling the AI and never awaiting input, because the builder chose this mode precisely to remove AI judgement from the branch. A null/missing/unresolvable value is "not met" (met: null), never an error, and no match selects the fallback, so the step can never end undefined. The evaluation trace is persisted in executionParams for the run view; unknown operators are rejected at the schema boundary so a run never reaches evaluation with a comparison it cannot honor. Part of PRD-472 Co-Authored-By: Claude Opus 5 (1M context) --- packages/workflow-executor/CLAUDE.md | 4 +- .../src/adapters/server-types.ts | 21 +- .../src/adapters/step-definition-mapper.ts | 1 + .../src/executors/condition-step-executor.ts | 93 ++++- .../deterministic-condition-evaluator.ts | 127 ++++++ .../src/types/step-execution-data.ts | 19 +- .../src/types/validated/step-definition.ts | 72 +++- .../adapters/step-definition-mapper.test.ts | 46 ++- .../executors/condition-step-executor.test.ts | 367 +++++++++++++++++- .../deterministic-condition-evaluator.test.ts | 216 +++++++++++ .../test/types/step-definition.test.ts | 123 +++++- 11 files changed, 1068 insertions(+), 21 deletions(-) create mode 100644 packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts create mode 100644 packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index 49e30e2fe3..4d51699b59 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -33,7 +33,9 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p ## Step types & execution modes -`StepExecutionMode` (domain enum, mapped from the server contract in `step-definition-mapper.ts`): `Manual`, `AutomatedWithConfirmation`, `FullyAutomated`. +`StepExecutionMode` (domain enum, mapped from the server contract in `step-definition-mapper.ts`): `Manual`, `AutomatedWithConfirmation`, `FullyAutomated`, `Deterministic` (condition steps only). + +- **Deterministic Condition** (`condition-step-executor.ts` + `deterministic-condition-evaluator.ts`, PRD-472) — evaluates build-time `preRecordedArgs.optionConditions` (schema-required when the mode is deterministic; wire-final operator names in `CONDITION_OPERATORS`) against Get Data outputs in the run history (`sourceStepId` + `fieldName` → read-record `executionResult.fields`). Top-to-bottom, first-match-wins, `and`/`or` aggregator; unresolvable/null value = `met: null` = not met (**never an error**); no match → `fallbackOption`. Never calls AI, never awaits input. Evaluation trace persisted in `executionParams` (`evaluations`/`selectedOption`/`usedFallback`) for the run view; the fallback never appears in `evaluations`. - **Trigger Action** (`trigger-record-action-step-executor.ts`, `handleFirstCall`) — detects the form via `getActionForm` (full field list, not `getActionFormInfo`). Formless: `FullyAutomated` runs it *in the executor* via the audited agent; otherwise pauses. With a form: `Manual` pauses with the native form (no AI fill); `AutomatedWithConfirmation` AI-fills then pauses for the user to submit natively; `FullyAutomated` AI-fills (`fillFormWithAi`) and, if `filledForm.canExecute`, submits in the executor — falling back to `awaiting-input` (pause) when required fields are missing, or on `ActionFormValidationError`/`ActionRequiresApprovalError` (a human can finish those). `UnsupportedActionFormError` is declared/exported but **never thrown** in src. - **Pre-recorded args** — record steps accept `preRecordedArgs` to skip AI. Technical names (`fieldName`/`fieldNames`/`actionName`/`relationName`) are matched exactly via `findFieldByTechnicalName` (no fuzz); `resolveAiFieldName` (exact-then-normalized) is reserved for AI-returned display names. The source record differs by step: read-record/update-record use `selectedRecordStepIndex` (a runtime index, resolved in `resolveRecordRef`); trigger-action/load-related-record use `selectedRecordStepId` — a **stable BPMN step id** (or `WORKFLOW_START_STEP_ID` sentinel) resolved by `resolveSourceRecordRef`, chosen to survive the index shifts a revision causes. Partial args supported. Unresolvable → `FieldNotFoundError`/`ActionNotFoundError`/`RelationNotFoundError`; bad shape / out-of-range index → `InvalidPreRecordedArgsError`. diff --git a/packages/workflow-executor/src/adapters/server-types.ts b/packages/workflow-executor/src/adapters/server-types.ts index 7295b73ca3..4fe6e14f03 100644 --- a/packages/workflow-executor/src/adapters/server-types.ts +++ b/packages/workflow-executor/src/adapters/server-types.ts @@ -35,6 +35,7 @@ export enum ServerStepExecutionTypeEnum { Manual = 'manual', AutomatedWithConfirmation = 'automated-with-confirmation', FullyAutomated = 'fully-automated', + Deterministic = 'deterministic', } interface ServerWorkflowStepBase { @@ -122,9 +123,27 @@ export interface ServerWorkflowEnd extends ServerWorkflowStepBase { export interface ServerWorkflowCondition extends ServerWorkflowStepBase { type: ServerStepTypeEnum.Condition; - executionType: ServerStepExecutionTypeEnum.Manual | ServerStepExecutionTypeEnum.FullyAutomated; + executionType: + | ServerStepExecutionTypeEnum.Manual + | ServerStepExecutionTypeEnum.FullyAutomated + | ServerStepExecutionTypeEnum.Deterministic; prompt: string | null; automaticCompletion: false; + // Parsed from `forest:optionConditions` server-side (flowId → answer). Present when + // executionType is deterministic. Validated by the step-definition schema. + preRecordedArgs?: { + optionConditions: Array<{ + option: string; + aggregator: 'and' | 'or'; + conditions: Array<{ + sourceStepId: string; + fieldName: string; + operator: string; + value?: unknown; + }>; + }>; + fallbackOption: string; + }; } export interface ServerWorkflowEscalation extends ServerWorkflowStepBase { diff --git a/packages/workflow-executor/src/adapters/step-definition-mapper.ts b/packages/workflow-executor/src/adapters/step-definition-mapper.ts index bf809ed538..cd56b81cec 100644 --- a/packages/workflow-executor/src/adapters/step-definition-mapper.ts +++ b/packages/workflow-executor/src/adapters/step-definition-mapper.ts @@ -103,6 +103,7 @@ function mapCondition(condition: ServerWorkflowCondition): ConditionStepDefiniti executionType: condition.executionType, title: condition.title, options, + preRecordedArgs: condition.preRecordedArgs, }); } diff --git a/packages/workflow-executor/src/executors/condition-step-executor.ts b/packages/workflow-executor/src/executors/condition-step-executor.ts index 36580d0956..2b3fef9591 100644 --- a/packages/workflow-executor/src/executors/condition-step-executor.ts +++ b/packages/workflow-executor/src/executors/condition-step-executor.ts @@ -1,5 +1,9 @@ import type { StepExecutionResult } from '../types/execution-context'; -import type { ConditionStepDefinition } from '../types/validated/step-definition'; +import type { ConditionEvaluation, StepExecutionData } from '../types/step-execution-data'; +import type { + ConditionStepDefinition, + DeterministicCondition, +} from '../types/validated/step-definition'; import type { ConditionStepOutcome } from '../types/validated/step-outcome'; import { DynamicStructuredTool, HumanMessage, SystemMessage } from '@forestadmin/ai-proxy'; @@ -7,8 +11,9 @@ import { z } from 'zod'; import { StepStateError } from '../errors'; import BaseStepExecutor from './base-step-executor'; +import evaluateOperator from './deterministic-condition-evaluator'; import patchBodySchemas from '../http/pending-data-validators'; -import { StepExecutionMode } from '../types/validated/step-definition'; +import { StepExecutionMode, StepType } from '../types/validated/step-definition'; interface GatewayToolArgs { option: string | null; @@ -62,6 +67,12 @@ export default class ConditionStepExecutor extends BaseStepExecutor { const { stepDefinition: step, incomingPendingData } = this.context; + // Deterministic mode: pure evaluation of build-time conditions against the run's step + // history — never calls the AI, never awaits input. + if (step.executionType === StepExecutionMode.Deterministic) { + return this.evaluateDeterministically(step); + } + // Manual mode: the user picks the option from the frontend. Wait for their input // without ever calling the AI. const isManual = step.executionType === StepExecutionMode.Manual; @@ -92,6 +103,84 @@ export default class ConditionStepExecutor extends BaseStepExecutor { + // Guaranteed by the schema's superRefine for the deterministic mode. + const { optionConditions, fallbackOption } = step.preRecordedArgs!; + const stepExecutions = await this.context.runStore.getStepExecutions(this.context.runId); + + let matchedOption: string | undefined; + const evaluations = optionConditions.map(({ option, aggregator, conditions }) => { + if (matchedOption !== undefined) { + return { option, outcome: 'not-evaluated' } satisfies ConditionEvaluation; + } + + const results = conditions.map((condition, index) => ({ + index, + met: this.evaluateCondition(condition, stepExecutions), + })); + const matched = + aggregator === 'or' + ? results.some(result => result.met === true) + : results.every(result => result.met === true); + if (matched) matchedOption = option; + + return { + option, + outcome: matched ? 'matched' : 'not-matched', + conditions: results, + } satisfies ConditionEvaluation; + }); + + const usedFallback = matchedOption === undefined; + const selectedOption = matchedOption ?? fallbackOption; + + await this.context.runStore.saveStepExecution(this.context.runId, { + type: 'condition', + stepIndex: this.context.stepIndex, + executionParams: { evaluations, selectedOption, usedFallback }, + executionResult: { answer: selectedOption }, + }); + + return this.buildOutcomeResult({ status: 'success', selectedOption }); + } + + private evaluateCondition( + condition: DeterministicCondition, + stepExecutions: StepExecutionData[], + ): boolean | null { + const resolved = this.resolveConditionValue(condition, stepExecutions); + // Unresolvable reference (step never ran, field not read, read error) → not evaluable, even + // for present/blank — a value that was never read is not the same as a blank one. + if (!resolved.found) return null; + + return evaluateOperator(condition.operator, resolved.value, condition.value); + } + + // Same live-path + most-recent-occurrence resolution as resolveSourceRecordRef: previousSteps + // are already restricted to the live path, and in a loop the same step id repeats. + private resolveConditionValue( + condition: DeterministicCondition, + stepExecutions: StepExecutionData[], + ): { found: true; value: unknown } | { found: false } { + const matches = this.context.previousSteps.filter( + step => + step.stepDefinition.type === StepType.ReadRecord && + step.stepOutcome.stepId === condition.sourceStepId, + ); + const sourceStep = matches[matches.length - 1]; + if (!sourceStep) return { found: false }; + + const execution = this.resolveStepExecution(sourceStep, stepExecutions); + if (execution?.type !== 'read-record') return { found: false }; + + const field = execution.executionResult.fields.find(f => f.name === condition.fieldName); + if (!field || !('value' in field)) return { found: false }; + + return { found: true, value: field.value }; + } + private readUserChoice( step: ConditionStepDefinition, incomingPendingData: unknown, diff --git a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts new file mode 100644 index 0000000000..8501a2f38f --- /dev/null +++ b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts @@ -0,0 +1,127 @@ +import type { ConditionOperator } from '../types/validated/step-definition'; + +// Guard against Date.parse's laxity ("5" parses as a year in some engines): only strings that +// start like an ISO date are treated as dates. +const ISO_DATE_PREFIX = /^\d{4}-\d{2}-\d{2}/; + +function toTimestamp(value: unknown): number | null { + if (typeof value !== 'string' || !ISO_DATE_PREFIX.test(value)) return null; + + const parsed = Date.parse(value); + + return Number.isNaN(parsed) ? null : parsed; +} + +function scalarEqual(actual: unknown, expected: unknown): boolean { + if (actual === expected) return true; + + const actualTs = toTimestamp(actual); + const expectedTs = toTimestamp(expected); + + return actualTs !== null && expectedTs !== null && actualTs === expectedTs; +} + +function isEqual(actual: unknown, expected: unknown): boolean { + if (Array.isArray(actual) && Array.isArray(expected)) { + return ( + actual.length === expected.length && + actual.every((item, index) => scalarEqual(item, expected[index])) + ); + } + + if (Array.isArray(actual) || Array.isArray(expected)) return false; + + return scalarEqual(actual, expected); +} + +function compare(actual: unknown, expected: unknown): number | null { + if (typeof actual === 'number' && typeof expected === 'number') { + if (Number.isNaN(actual) || Number.isNaN(expected)) return null; + + return actual - expected; + } + + const actualTs = toTimestamp(actual); + const expectedTs = toTimestamp(expected); + if (actualTs !== null && expectedTs !== null) return actualTs - expectedTs; + + return null; +} + +function isPresent(value: unknown): boolean { + if (value === null || value === undefined) return false; + if (typeof value === 'string') return value.length > 0; + if (Array.isArray(value)) return value.length > 0; + + return true; +} + +function isMemberOf(list: unknown, candidate: unknown): boolean { + return Array.isArray(list) && list.some(item => scalarEqual(item, candidate)); +} + +/** + * Pure evaluation of one deterministic condition. Never throws for data reasons: + * - `null` = not evaluable (the resolved value is null/missing) — treated as "not met"; + * - a type-mismatched comparison (including for negated operators) is "not met" (`false`), + * so a broken config can never accidentally satisfy a condition. + */ +export default function evaluateOperator( + operator: ConditionOperator, + actual: unknown, + expected: unknown, +): boolean | null { + if (operator === 'present') return isPresent(actual); + if (operator === 'blank') return !isPresent(actual); + if (actual === null || actual === undefined) return null; + + switch (operator) { + case 'equal': + return isEqual(actual, expected); + case 'not_equal': + return !isEqual(actual, expected); + + case 'greater_than': { + const diff = compare(actual, expected); + + return diff !== null && diff > 0; + } + + case 'less_than': { + const diff = compare(actual, expected); + + return diff !== null && diff < 0; + } + + case 'greater_than_or_equal': { + const diff = compare(actual, expected); + + return diff !== null && diff >= 0; + } + + case 'less_than_or_equal': { + const diff = compare(actual, expected); + + return diff !== null && diff <= 0; + } + + case 'in': + return isMemberOf(expected, actual); + case 'not_in': + return Array.isArray(expected) && !isMemberOf(expected, actual); + case 'contains': + if (typeof actual === 'string' && typeof expected === 'string') { + return actual.includes(expected); + } + + return isMemberOf(actual, expected); + case 'not_contains': + if (typeof actual === 'string' && typeof expected === 'string') { + return !actual.includes(expected); + } + + return Array.isArray(actual) && !isMemberOf(actual, expected); + default: + return null; + } +} diff --git a/packages/workflow-executor/src/types/step-execution-data.ts b/packages/workflow-executor/src/types/step-execution-data.ts index 80da7c91cc..3d5a683123 100644 --- a/packages/workflow-executor/src/types/step-execution-data.ts +++ b/packages/workflow-executor/src/types/step-execution-data.ts @@ -27,9 +27,26 @@ export interface WithUserConfirmation = Record // -- Condition -- +export interface ConditionEvaluation { + option: string; + outcome: 'matched' | 'not-matched' | 'not-evaluated'; + /** Absent when outcome is 'not-evaluated'. `met: null` = the value could not be evaluated. */ + conditions?: Array<{ index: number; met: boolean | null }>; +} + +// Deterministic evaluation trace read by the run view (PRD-472 contract shape). The fallback +// never appears in `evaluations` — the front derives its display from `usedFallback`. +export interface DeterministicConditionExecutionParams { + evaluations: ConditionEvaluation[]; + selectedOption: string; + usedFallback: boolean; +} + export interface ConditionStepExecutionData extends BaseStepExecutionData { type: 'condition'; - executionParams: { answer: string | null; reasoning?: string }; + executionParams: + | { answer: string | null; reasoning?: string } + | DeterministicConditionExecutionParams; executionResult?: { answer: string }; } diff --git a/packages/workflow-executor/src/types/validated/step-definition.ts b/packages/workflow-executor/src/types/validated/step-definition.ts index dc80f540f5..4501a118be 100644 --- a/packages/workflow-executor/src/types/validated/step-definition.ts +++ b/packages/workflow-executor/src/types/validated/step-definition.ts @@ -18,6 +18,7 @@ export enum StepExecutionMode { Manual = 'manual', AutomatedWithConfirmation = 'automated-with-confirmation', FullyAutomated = 'fully-automated', + Deterministic = 'deterministic', } // Shared fields across all step types. executionType is intentionally excluded — @@ -38,17 +39,70 @@ const sharedFields = { }; // Use z.enum(EnumObject), not z.nativeEnum — the latter is deprecated in zod 4. -const { Manual, AutomatedWithConfirmation, FullyAutomated } = StepExecutionMode; +const { Manual, AutomatedWithConfirmation, FullyAutomated, Deterministic } = StepExecutionMode; -export const ConditionStepDefinitionSchema = z.object({ - ...sharedFields, - type: z.literal(StepType.Condition), - // NO `.catch` — coercing an unknown mode (e.g. a future `deterministic` from a newer - // orchestrator) to FullyAutomated would silently let the AI decide instead of the conditions - // the builder configured precisely because they don't trust the AI. - executionType: z.enum([Manual, FullyAutomated]).default(FullyAutomated), - options: z.array(z.string()).min(2), +// Wire-final operator names (PRD-472 cross-repo contract). An unknown operator is rejected here, +// at the schema boundary, so a run never reaches evaluation with a comparison it cannot honor. +export const CONDITION_OPERATORS = [ + 'equal', + 'not_equal', + 'present', + 'blank', + 'greater_than', + 'less_than', + 'greater_than_or_equal', + 'less_than_or_equal', + 'in', + 'not_in', + 'contains', + 'not_contains', +] as const; +export type ConditionOperator = (typeof CONDITION_OPERATORS)[number]; + +const DeterministicConditionSchema = z.object({ + /** Stable BPMN id of the upstream Get Data step whose output holds the value. */ + sourceStepId: z.string().min(1), + fieldName: z.string().min(1), + operator: z.enum(CONDITION_OPERATORS), + /** Absent for `present`/`blank`. */ + value: z.unknown().optional(), }); +export type DeterministicCondition = z.infer; + +const OptionConditionsSchema = z.object({ + option: z.string().min(1), + aggregator: z.enum(['and', 'or']), + conditions: z.array(DeterministicConditionSchema).min(1), +}); +export type OptionConditions = z.infer; + +export const ConditionStepDefinitionSchema = z + .object({ + ...sharedFields, + type: z.literal(StepType.Condition), + // NO `.catch` — coercing an unknown mode from a newer orchestrator to FullyAutomated would + // silently let the AI decide instead of the conditions the builder configured precisely + // because they don't trust the AI. + executionType: z.enum([Manual, FullyAutomated, Deterministic]).default(FullyAutomated), + /** Ordered — evaluation priority for the deterministic mode (top-to-bottom, first-match-wins). */ + options: z.array(z.string()).min(2), + preRecordedArgs: z + .object({ + optionConditions: z.array(OptionConditionsSchema).min(1), + fallbackOption: z.string().min(1), + }) + .optional(), + }) + // No silent fallback to manual/AI: a deterministic step without its conditions must fail loud. + .superRefine((step, ctx) => { + if (step.executionType === Deterministic && step.preRecordedArgs === undefined) { + ctx.addIssue({ + code: 'custom', + path: ['preRecordedArgs'], + message: 'preRecordedArgs is required when executionType is "deterministic"', + }); + } + }); export type ConditionStepDefinition = z.infer; export const ReadRecordStepDefinitionSchema = z.object({ diff --git a/packages/workflow-executor/test/adapters/step-definition-mapper.test.ts b/packages/workflow-executor/test/adapters/step-definition-mapper.test.ts index 398df8c12c..8808972a1d 100644 --- a/packages/workflow-executor/test/adapters/step-definition-mapper.test.ts +++ b/packages/workflow-executor/test/adapters/step-definition-mapper.test.ts @@ -309,7 +309,7 @@ describe('toStepDefinition', () => { }); }); - // A newer orchestrator may send a deterministic mode this executor version does not know. + // A newer orchestrator may send an execution mode this executor version does not know. // The `.catch(FullyAutomated)` that used to sit on the condition schema would have silently // handed the decision to the AI; the mapper must reject the run as malformed instead. it('should throw InvalidStepDefinitionError for an unknown executionType instead of coercing to Full AI', () => { @@ -318,13 +318,55 @@ describe('toStepDefinition', () => { { stepId: 's1', buttonText: null, answer: 'Yes' }, { stepId: 's2', buttonText: null, answer: 'No' }, ], - { executionType: 'deterministic' as ServerWorkflowCondition['executionType'] }, + { executionType: 'not-a-mode' as ServerWorkflowCondition['executionType'] }, ); expect(() => toStepDefinition(condition)).toThrow(InvalidStepDefinitionError); expect(() => toStepDefinition(condition)).toThrow(/executionType/); }); + it('should forward preRecordedArgs on a deterministic condition', () => { + const preRecordedArgs = { + optionConditions: [ + { + option: 'Yes', + aggregator: 'and' as const, + conditions: [ + { sourceStepId: 'get-data-1', fieldName: 'status', operator: 'equal', value: 'ok' }, + ], + }, + ], + fallbackOption: 'No', + }; + const condition = makeCondition( + [ + { stepId: 's1', buttonText: null, answer: 'Yes' }, + { stepId: 's2', buttonText: null, answer: 'No' }, + ], + { executionType: ServerStepExecutionTypeEnum.Deterministic, preRecordedArgs }, + ); + + expect(toStepDefinition(condition)).toMatchObject({ + type: StepType.Condition, + executionType: StepExecutionMode.Deterministic, + options: ['Yes', 'No'], + preRecordedArgs, + }); + }); + + it('should throw InvalidStepDefinitionError for a deterministic condition without preRecordedArgs', () => { + const condition = makeCondition( + [ + { stepId: 's1', buttonText: null, answer: 'Yes' }, + { stepId: 's2', buttonText: null, answer: 'No' }, + ], + { executionType: ServerStepExecutionTypeEnum.Deterministic }, + ); + + expect(() => toStepDefinition(condition)).toThrow(InvalidStepDefinitionError); + expect(() => toStepDefinition(condition)).toThrow(/preRecordedArgs/); + }); + it('should throw InvalidStepDefinitionError when fewer than 2 options', () => { const condition = makeCondition([{ stepId: 's1', buttonText: 'Only' }]); diff --git a/packages/workflow-executor/test/executors/condition-step-executor.test.ts b/packages/workflow-executor/test/executors/condition-step-executor.test.ts index 0242da473b..55d6cac1c6 100644 --- a/packages/workflow-executor/test/executors/condition-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/condition-step-executor.test.ts @@ -2,7 +2,8 @@ import type { ActivityLogPort } from '../../src/ports/activity-log-port'; import type { AgentPort } from '../../src/ports/agent-port'; import type { RunStore } from '../../src/ports/run-store'; import type { WorkflowPort } from '../../src/ports/workflow-port'; -import type { ExecutionContext } from '../../src/types/execution-context'; +import type { ExecutionContext, Step } from '../../src/types/execution-context'; +import type { FieldReadResult, StepExecutionData } from '../../src/types/step-execution-data'; import type { RecordRef } from '../../src/types/validated/collection'; import type { ConditionStepDefinition } from '../../src/types/validated/step-definition'; import type { ConditionStepOutcome } from '../../src/types/validated/step-outcome'; @@ -25,6 +26,39 @@ function makeStep(overrides: Partial = {}): ConditionSt }; } +type ConditionPreRecordedArgs = NonNullable; + +function makeDeterministicStep(preRecordedArgs: ConditionPreRecordedArgs): ConditionStepDefinition { + return makeStep({ + executionType: StepExecutionMode.Deterministic, + options: [ + ...preRecordedArgs.optionConditions.map(o => o.option), + preRecordedArgs.fallbackOption, + ], + preRecordedArgs, + }); +} + +function makeGetDataStep(stepId: string, stepIndex: number): Step { + return { + stepDefinition: { + type: StepType.ReadRecord, + executionType: StepExecutionMode.FullyAutomated, + }, + stepOutcome: { type: 'record', stepId, stepIndex, status: 'success' }, + }; +} + +function makeReadRecordExecution(stepIndex: number, fields: FieldReadResult[]): StepExecutionData { + return { + type: 'read-record', + stepIndex, + executionParams: { fields: fields.map(({ name, displayName }) => ({ name, displayName })) }, + executionResult: { fields }, + selectedRecordRef: { collectionName: 'orders', recordId: [1], stepIndex: 0 }, + }; +} + function makeMockRunStore(overrides: Partial = {}): RunStore { return { init: jest.fn().mockResolvedValue(undefined), @@ -421,6 +455,337 @@ describe('ConditionStepExecutor', () => { }); }); + describe('executionType=Deterministic', () => { + const amountArgs: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'High', + aggregator: 'and', + conditions: [ + { sourceStepId: 'get-1', fieldName: 'amount', operator: 'greater_than', value: 100 }, + ], + }, + { + option: 'Low', + aggregator: 'and', + conditions: [ + { + sourceStepId: 'get-1', + fieldName: 'amount', + operator: 'less_than_or_equal', + value: 100, + }, + ], + }, + ], + fallbackOption: 'Other', + }; + + function makeDeterministicContext( + preRecordedArgs: ConditionPreRecordedArgs, + fields: FieldReadResult[], + overrides: Partial> = {}, + ) { + const mockModel = makeMockModel(); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([makeReadRecordExecution(1, fields)]), + }); + const context = makeContext({ + model: mockModel.model, + runStore, + stepDefinition: makeDeterministicStep(preRecordedArgs), + previousSteps: [makeGetDataStep('get-1', 1)], + ...overrides, + }); + + return { context, mockModel, runStore }; + } + + it('selects the first matching option without calling the AI or awaiting input', async () => { + const { context, mockModel, runStore } = makeDeterministicContext(amountArgs, [ + { name: 'amount', displayName: 'Amount', value: 150 }, + ]); + const executor = new ConditionStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('success'); + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('High'); + expect(mockModel.bindTools).not.toHaveBeenCalled(); + expect(mockModel.invoke).not.toHaveBeenCalled(); + expect(runStore.saveStepExecution).toHaveBeenCalledWith('run-1', { + type: 'condition', + stepIndex: 0, + executionParams: { + evaluations: [ + { option: 'High', outcome: 'matched', conditions: [{ index: 0, met: true }] }, + { option: 'Low', outcome: 'not-evaluated' }, + ], + selectedOption: 'High', + usedFallback: false, + }, + executionResult: { answer: 'High' }, + }); + }); + + it('does not evaluate options after the first match, even if they would also match', async () => { + const bothMatch: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'First', + aggregator: 'and', + conditions: [{ sourceStepId: 'get-1', fieldName: 'amount', operator: 'present' }], + }, + { + option: 'Second', + aggregator: 'and', + conditions: [{ sourceStepId: 'get-1', fieldName: 'amount', operator: 'present' }], + }, + ], + fallbackOption: 'Other', + }; + const { context, runStore } = makeDeterministicContext(bothMatch, [ + { name: 'amount', displayName: 'Amount', value: 150 }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('First'); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: expect.objectContaining({ + evaluations: [ + { option: 'First', outcome: 'matched', conditions: [{ index: 0, met: true }] }, + { option: 'Second', outcome: 'not-evaluated' }, + ], + }), + }), + ); + }); + + it('requires all conditions with the and aggregator', async () => { + const andArgs: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'High paid', + aggregator: 'and', + conditions: [ + { sourceStepId: 'get-1', fieldName: 'amount', operator: 'greater_than', value: 100 }, + { sourceStepId: 'get-1', fieldName: 'status', operator: 'equal', value: 'paid' }, + ], + }, + ], + fallbackOption: 'Other', + }; + const { context, runStore } = makeDeterministicContext(andArgs, [ + { name: 'amount', displayName: 'Amount', value: 150 }, + { name: 'status', displayName: 'Status', value: 'pending' }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Other'); + expect(runStore.saveStepExecution).toHaveBeenCalledWith('run-1', { + type: 'condition', + stepIndex: 0, + executionParams: { + evaluations: [ + { + option: 'High paid', + outcome: 'not-matched', + conditions: [ + { index: 0, met: true }, + { index: 1, met: false }, + ], + }, + ], + selectedOption: 'Other', + usedFallback: true, + }, + executionResult: { answer: 'Other' }, + }); + }); + + it('matches with the or aggregator when any condition is met', async () => { + const orArgs: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'High or paid', + aggregator: 'or', + conditions: [ + { sourceStepId: 'get-1', fieldName: 'status', operator: 'equal', value: 'paid' }, + { sourceStepId: 'get-1', fieldName: 'amount', operator: 'greater_than', value: 100 }, + ], + }, + ], + fallbackOption: 'Other', + }; + const { context, runStore } = makeDeterministicContext(orArgs, [ + { name: 'amount', displayName: 'Amount', value: 150 }, + { name: 'status', displayName: 'Status', value: 'pending' }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('High or paid'); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: expect.objectContaining({ + evaluations: [ + { + option: 'High or paid', + outcome: 'matched', + conditions: [ + { index: 0, met: false }, + { index: 1, met: true }, + ], + }, + ], + }), + }), + ); + }); + + it('treats a null field value as not evaluable and falls back — never an error', async () => { + const { context, runStore } = makeDeterministicContext(amountArgs, [ + { name: 'amount', displayName: 'Amount', value: null }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect(result.stepOutcome.status).toBe('success'); + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Other'); + expect(runStore.saveStepExecution).toHaveBeenCalledWith('run-1', { + type: 'condition', + stepIndex: 0, + executionParams: { + evaluations: [ + { option: 'High', outcome: 'not-matched', conditions: [{ index: 0, met: null }] }, + { option: 'Low', outcome: 'not-matched', conditions: [{ index: 0, met: null }] }, + ], + selectedOption: 'Other', + usedFallback: true, + }, + executionResult: { answer: 'Other' }, + }); + }); + + it('treats an unresolvable source step as not evaluable and falls back', async () => { + const unknownSource: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'High', + aggregator: 'and', + conditions: [ + { + sourceStepId: 'never-ran', + fieldName: 'amount', + operator: 'greater_than', + value: 100, + }, + ], + }, + ], + fallbackOption: 'Other', + }; + const { context } = makeDeterministicContext(unknownSource, [ + { name: 'amount', displayName: 'Amount', value: 150 }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect(result.stepOutcome.status).toBe('success'); + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Other'); + }); + + it('treats a field the Get Data step failed to read as not evaluable', async () => { + const { context, runStore } = makeDeterministicContext(amountArgs, [ + { name: 'amount', displayName: 'Amount', error: 'Field not found: amount' }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Other'); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: expect.objectContaining({ + evaluations: expect.arrayContaining([ + { option: 'High', outcome: 'not-matched', conditions: [{ index: 0, met: null }] }, + ]), + }), + }), + ); + }); + + it('lets blank match a resolved null value (unlike an unresolvable one)', async () => { + const blankArgs: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'No amount', + aggregator: 'and', + conditions: [{ sourceStepId: 'get-1', fieldName: 'amount', operator: 'blank' }], + }, + ], + fallbackOption: 'Other', + }; + const { context } = makeDeterministicContext(blankArgs, [ + { name: 'amount', displayName: 'Amount', value: null }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('No amount'); + }); + + it('selects the fallback when no option matches', async () => { + const { context, runStore } = makeDeterministicContext(amountArgs, [ + { name: 'amount', displayName: 'Amount', value: 'not a number' }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect(result.stepOutcome.status).toBe('success'); + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Other'); + expect(runStore.saveStepExecution).toHaveBeenCalledWith('run-1', { + type: 'condition', + stepIndex: 0, + executionParams: { + evaluations: [ + { option: 'High', outcome: 'not-matched', conditions: [{ index: 0, met: false }] }, + { option: 'Low', outcome: 'not-matched', conditions: [{ index: 0, met: false }] }, + ], + selectedOption: 'Other', + usedFallback: true, + }, + executionResult: { answer: 'Other' }, + }); + }); + + it('uses the most recent occurrence of a repeated source step id (loop)', async () => { + const runStore = makeMockRunStore({ + getStepExecutions: jest + .fn() + .mockResolvedValue([ + makeReadRecordExecution(1, [{ name: 'amount', displayName: 'Amount', value: 50 }]), + makeReadRecordExecution(2, [{ name: 'amount', displayName: 'Amount', value: 150 }]), + ]), + }); + const context = makeContext({ + model: makeMockModel().model, + runStore, + stepDefinition: makeDeterministicStep(amountArgs), + previousSteps: [makeGetDataStep('get-1', 1), makeGetDataStep('get-1', 2)], + }); + + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('High'); + }); + }); + describe('executionType=Manual', () => { it('returns awaiting-input without calling AI or saving when no incomingPendingData', async () => { const mockModel = makeMockModel(); diff --git a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts new file mode 100644 index 0000000000..f1d0247ad7 --- /dev/null +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -0,0 +1,216 @@ +import evaluateOperator from '../../src/executors/deterministic-condition-evaluator'; + +describe('evaluateOperator', () => { + describe('null / missing actual value (never an error)', () => { + it.each([ + 'equal', + 'not_equal', + 'greater_than', + 'less_than', + 'greater_than_or_equal', + 'less_than_or_equal', + 'in', + 'not_in', + 'contains', + 'not_contains', + ] as const)('returns null (not evaluable) for %s on a null actual', operator => { + expect(evaluateOperator(operator, null, 'anything')).toBeNull(); + expect(evaluateOperator(operator, undefined, 'anything')).toBeNull(); + }); + }); + + describe('equal', () => { + it('matches identical scalars', () => { + expect(evaluateOperator('equal', 'active', 'active')).toBe(true); + expect(evaluateOperator('equal', 5, 5)).toBe(true); + expect(evaluateOperator('equal', false, false)).toBe(true); + }); + + it('rejects different scalars', () => { + expect(evaluateOperator('equal', 'active', 'inactive')).toBe(false); + }); + + it('rejects a type mismatch (no coercion)', () => { + expect(evaluateOperator('equal', 5, '5')).toBe(false); + expect(evaluateOperator('equal', true, 'true')).toBe(false); + }); + + it('matches ISO dates by timestamp, not by string', () => { + expect(evaluateOperator('equal', '2026-01-01T00:00:00Z', '2026-01-01T00:00:00.000Z')).toBe( + true, + ); + expect(evaluateOperator('equal', '2026-01-01T00:00:00Z', '2026-01-02T00:00:00Z')).toBe(false); + }); + + it('matches arrays elementwise in order', () => { + expect(evaluateOperator('equal', [1, 2], [1, 2])).toBe(true); + expect(evaluateOperator('equal', [1, 2], [2, 1])).toBe(false); + expect(evaluateOperator('equal', [1, 2], [1, 2, 3])).toBe(false); + }); + + it('rejects an array compared to a scalar', () => { + expect(evaluateOperator('equal', [1], 1)).toBe(false); + }); + }); + + describe('not_equal', () => { + it('matches different values', () => { + expect(evaluateOperator('not_equal', 'active', 'inactive')).toBe(true); + expect(evaluateOperator('not_equal', 5, '5')).toBe(true); + }); + + it('rejects identical values', () => { + expect(evaluateOperator('not_equal', 'active', 'active')).toBe(false); + expect( + evaluateOperator('not_equal', '2026-01-01T00:00:00Z', '2026-01-01T00:00:00.000Z'), + ).toBe(false); + }); + }); + + describe('present', () => { + it('matches non-empty values', () => { + expect(evaluateOperator('present', 'a', undefined)).toBe(true); + expect(evaluateOperator('present', 0, undefined)).toBe(true); + expect(evaluateOperator('present', false, undefined)).toBe(true); + expect(evaluateOperator('present', [1], undefined)).toBe(true); + }); + + it('rejects null, undefined, empty string and empty array', () => { + expect(evaluateOperator('present', null, undefined)).toBe(false); + expect(evaluateOperator('present', undefined, undefined)).toBe(false); + expect(evaluateOperator('present', '', undefined)).toBe(false); + expect(evaluateOperator('present', [], undefined)).toBe(false); + }); + }); + + describe('blank', () => { + it('matches null, undefined, empty string and empty array', () => { + expect(evaluateOperator('blank', null, undefined)).toBe(true); + expect(evaluateOperator('blank', undefined, undefined)).toBe(true); + expect(evaluateOperator('blank', '', undefined)).toBe(true); + expect(evaluateOperator('blank', [], undefined)).toBe(true); + }); + + it('rejects non-empty values including falsy ones', () => { + expect(evaluateOperator('blank', 'a', undefined)).toBe(false); + expect(evaluateOperator('blank', 0, undefined)).toBe(false); + expect(evaluateOperator('blank', false, undefined)).toBe(false); + }); + }); + + describe('numeric comparisons', () => { + it('greater_than compares numbers', () => { + expect(evaluateOperator('greater_than', 5, 3)).toBe(true); + expect(evaluateOperator('greater_than', 3, 5)).toBe(false); + expect(evaluateOperator('greater_than', 5, 5)).toBe(false); + }); + + it('less_than compares numbers', () => { + expect(evaluateOperator('less_than', 3, 5)).toBe(true); + expect(evaluateOperator('less_than', 5, 3)).toBe(false); + expect(evaluateOperator('less_than', 5, 5)).toBe(false); + }); + + it('greater_than_or_equal includes equality', () => { + expect(evaluateOperator('greater_than_or_equal', 5, 5)).toBe(true); + expect(evaluateOperator('greater_than_or_equal', 4, 5)).toBe(false); + }); + + it('less_than_or_equal includes equality', () => { + expect(evaluateOperator('less_than_or_equal', 5, 5)).toBe(true); + expect(evaluateOperator('less_than_or_equal', 6, 5)).toBe(false); + }); + + it('is not met on a type mismatch or non-comparable operands', () => { + expect(evaluateOperator('greater_than', 5, '3')).toBe(false); + expect(evaluateOperator('greater_than', '5', 3)).toBe(false); + expect(evaluateOperator('greater_than', 'abc', 'abd')).toBe(false); + expect(evaluateOperator('less_than', Number.NaN, 5)).toBe(false); + }); + }); + + describe('date comparisons', () => { + it('compares ISO strings as timestamps when both sides parse', () => { + expect(evaluateOperator('greater_than', '2026-02-01', '2026-01-01')).toBe(true); + expect(evaluateOperator('less_than', '2026-01-01T10:00:00Z', '2026-01-01T12:00:00Z')).toBe( + true, + ); + expect(evaluateOperator('greater_than_or_equal', '2026-01-01T00:00:00Z', '2026-01-01')).toBe( + true, + ); + expect(evaluateOperator('less_than_or_equal', '2026-01-02', '2026-01-01')).toBe(false); + }); + + it('is not met when one side does not parse as an ISO date', () => { + expect(evaluateOperator('greater_than', '2026-02-01', 'not a date')).toBe(false); + expect(evaluateOperator('less_than', 'not a date', '2026-02-01')).toBe(false); + }); + }); + + describe('in', () => { + it('matches when the value is in the list', () => { + expect(evaluateOperator('in', 'b', ['a', 'b'])).toBe(true); + expect(evaluateOperator('in', 2, [1, 2, 3])).toBe(true); + expect(evaluateOperator('in', '2026-01-01T00:00:00Z', ['2026-01-01T00:00:00.000Z'])).toBe( + true, + ); + }); + + it('rejects when the value is not in the list', () => { + expect(evaluateOperator('in', 'c', ['a', 'b'])).toBe(false); + expect(evaluateOperator('in', 2, ['2'])).toBe(false); + }); + + it('is not met when the expected value is not an array', () => { + expect(evaluateOperator('in', 'a', 'a')).toBe(false); + }); + }); + + describe('not_in', () => { + it('matches when the value is absent from the list', () => { + expect(evaluateOperator('not_in', 'c', ['a', 'b'])).toBe(true); + }); + + it('rejects when the value is in the list', () => { + expect(evaluateOperator('not_in', 'a', ['a', 'b'])).toBe(false); + }); + + it('is not met (never satisfied by mismatch) when the expected value is not an array', () => { + expect(evaluateOperator('not_in', 'a', 'b')).toBe(false); + }); + }); + + describe('contains', () => { + it('matches a substring on strings', () => { + expect(evaluateOperator('contains', 'hello world', 'world')).toBe(true); + expect(evaluateOperator('contains', 'hello', 'world')).toBe(false); + }); + + it('matches membership on arrays', () => { + expect(evaluateOperator('contains', ['a', 'b'], 'b')).toBe(true); + expect(evaluateOperator('contains', ['a', 'b'], 'c')).toBe(false); + }); + + it('is not met on a type mismatch', () => { + expect(evaluateOperator('contains', 5, '5')).toBe(false); + expect(evaluateOperator('contains', 'abc', 5)).toBe(false); + }); + }); + + describe('not_contains', () => { + it('matches when the substring is absent', () => { + expect(evaluateOperator('not_contains', 'hello', 'world')).toBe(true); + expect(evaluateOperator('not_contains', 'hello world', 'world')).toBe(false); + }); + + it('matches when the array does not contain the value', () => { + expect(evaluateOperator('not_contains', ['a'], 'b')).toBe(true); + expect(evaluateOperator('not_contains', ['a'], 'a')).toBe(false); + }); + + it('is not met (never satisfied by mismatch) on a type mismatch', () => { + expect(evaluateOperator('not_contains', 5, '5')).toBe(false); + expect(evaluateOperator('not_contains', 'abc', 5)).toBe(false); + }); + }); +}); diff --git a/packages/workflow-executor/test/types/step-definition.test.ts b/packages/workflow-executor/test/types/step-definition.test.ts index ff4337d8ad..0d517ee7b5 100644 --- a/packages/workflow-executor/test/types/step-definition.test.ts +++ b/packages/workflow-executor/test/types/step-definition.test.ts @@ -26,17 +26,132 @@ describe('ConditionStepDefinitionSchema executionType', () => { }); // No `.catch` on the enum: an unknown value must be rejected, not silently coerced to - // FullyAutomated (which would let the AI decide in place of a future deterministic mode). + // FullyAutomated (which would let the AI decide in place of the deterministic mode). it('rejects an invalid executionType instead of coercing it', () => { - expect( - ConditionStepDefinitionSchema.safeParse({ ...base, executionType: 'deterministic' }).success, - ).toBe(false); expect( ConditionStepDefinitionSchema.safeParse({ ...base, executionType: 'not-a-mode' }).success, ).toBe(false); }); }); +describe('ConditionStepDefinitionSchema deterministic mode', () => { + const base = { type: StepType.Condition as const, options: ['High value', 'Fallback'] }; + const preRecordedArgs = { + optionConditions: [ + { + option: 'High value', + aggregator: 'and', + conditions: [ + { sourceStepId: 'get-data-1', fieldName: 'amount', operator: 'greater_than', value: 100 }, + ], + }, + ], + fallbackOption: 'Fallback', + }; + + it('accepts deterministic with preRecordedArgs and round-trips them', () => { + const parsed = ConditionStepDefinitionSchema.parse({ + ...base, + executionType: 'deterministic', + preRecordedArgs, + }); + + expect(parsed.executionType).toBe(StepExecutionMode.Deterministic); + expect(parsed.preRecordedArgs).toEqual(preRecordedArgs); + }); + + it('rejects deterministic without preRecordedArgs — no silent fallback to AI', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'deterministic', + }); + + expect(result.success).toBe(false); + expect(JSON.stringify(!result.success && result.error.issues)).toContain('preRecordedArgs'); + }); + + it('rejects an unknown operator at the schema boundary', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'deterministic', + preRecordedArgs: { + ...preRecordedArgs, + optionConditions: [ + { + option: 'High value', + aggregator: 'and', + conditions: [{ sourceStepId: 'get-data-1', fieldName: 'amount', operator: 'ilike' }], + }, + ], + }, + }); + + expect(result.success).toBe(false); + }); + + it('rejects preRecordedArgs missing fallbackOption', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'deterministic', + preRecordedArgs: { optionConditions: preRecordedArgs.optionConditions }, + }); + + expect(result.success).toBe(false); + }); + + it('rejects an option with an unknown aggregator', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'deterministic', + preRecordedArgs: { + ...preRecordedArgs, + optionConditions: [{ ...preRecordedArgs.optionConditions[0], aggregator: 'xor' }], + }, + }); + + expect(result.success).toBe(false); + }); + + it('rejects an option with zero conditions', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'deterministic', + preRecordedArgs: { + ...preRecordedArgs, + optionConditions: [{ option: 'High value', aggregator: 'and', conditions: [] }], + }, + }); + + expect(result.success).toBe(false); + }); + + it('accepts a value-less condition for present/blank operators', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'deterministic', + preRecordedArgs: { + ...preRecordedArgs, + optionConditions: [ + { + option: 'High value', + aggregator: 'or', + conditions: [{ sourceStepId: 'get-data-1', fieldName: 'amount', operator: 'present' }], + }, + ], + }, + }); + + expect(result.success).toBe(true); + }); + + it('still accepts non-deterministic modes without preRecordedArgs', () => { + expect( + ConditionStepDefinitionSchema.safeParse({ ...base, executionType: 'fully-automated' }) + .success, + ).toBe(true); + }); +}); + describe('LoadRelatedRecordStepDefinitionSchema executionType', () => { const base = { type: StepType.LoadRelatedRecord as const }; From 8d4ec64ff1df01dea62c7fd1bca697589506848e Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Wed, 19 Aug 2026 08:29:59 +0200 Subject: [PATCH 2/2] fix(workflow-executor): make deterministic condition evaluation type-safe and routable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deterministic evaluator could turn a data or config mismatch into a silent misroute — a `status: 'success'` step carrying the wrong branch. - Numeric strings: Sequelize returns Postgres/MySQL `numeric`/`decimal`/`bigint` columns as strings while datasource-sequelize maps those types to the `Number` primitive, so the builder's `value: 100` met `"150.00"` at runtime and every comparison bailed → no option matched → silent fallback. A strictly numeric string is now coerced against a real number (both sides stay uncoerced when neither is a number, since ordering operators are Number/Date-only per the contract); `'abc'` vs `100` is still not evaluable. - Selected option: the deterministic path emitted `matchedOption ?? fallbackOption` without checking `step.options`, unlike the manual path. `optionConditions` and `options` are two different server-side derivations, so drift produced a success outcome the orchestrator cannot route and the run died far from the cause. It now throws `InvalidStepDefinitionError` before persisting anything. - `not_equal` contradicted the file's own documented policy ("a type mismatch can never satisfy a negated operator") by returning true on mismatch; equality is now tri-state, so a mismatch satisfies neither `equal` nor `not_equal`. - Timezone: an offset-less ISO datetime was parsed host-local, so "deterministic" evaluation varied per machine. Offset-less datetimes are read as UTC, pinned by a test that runs under a non-UTC TZ. - `contains`/`not_contains` were extended to array membership beyond the contract (§1: String only). Restricted back to strings — dead flexibility the builder never emits, and the "mismatch is never satisfied" invariant keeps it from misrouting. - Removed the unreachable `default` branch by replacing the operator switch with an exhaustive lookup keyed by `ConditionOperator`, so a new operator fails to compile instead of silently returning null (lint's `default-case` forbids a bare switch). Also asserts three spec behaviors that were unasserted: deterministic mode ignores `incomingPendingData`, `or` matches on a mix of not-evaluable and true, and an unresolvable reference satisfies neither `blank` nor `present`. Part of PRD-472 Co-Authored-By: Claude Opus 5 (1M context) --- packages/workflow-executor/CLAUDE.md | 3 +- .../src/executors/condition-step-executor.ts | 11 +- .../deterministic-condition-evaluator.ts | 125 ++++++++-------- .../executors/condition-step-executor.test.ts | 137 ++++++++++++++++++ .../deterministic-condition-evaluator.test.ts | 82 ++++++++--- 5 files changed, 277 insertions(+), 81 deletions(-) diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index 4d51699b59..929bb3e311 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -35,7 +35,8 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p `StepExecutionMode` (domain enum, mapped from the server contract in `step-definition-mapper.ts`): `Manual`, `AutomatedWithConfirmation`, `FullyAutomated`, `Deterministic` (condition steps only). -- **Deterministic Condition** (`condition-step-executor.ts` + `deterministic-condition-evaluator.ts`, PRD-472) — evaluates build-time `preRecordedArgs.optionConditions` (schema-required when the mode is deterministic; wire-final operator names in `CONDITION_OPERATORS`) against Get Data outputs in the run history (`sourceStepId` + `fieldName` → read-record `executionResult.fields`). Top-to-bottom, first-match-wins, `and`/`or` aggregator; unresolvable/null value = `met: null` = not met (**never an error**); no match → `fallbackOption`. Never calls AI, never awaits input. Evaluation trace persisted in `executionParams` (`evaluations`/`selectedOption`/`usedFallback`) for the run view; the fallback never appears in `evaluations`. +- **Deterministic Condition** (`condition-step-executor.ts` + `deterministic-condition-evaluator.ts`, PRD-472) — evaluates build-time `preRecordedArgs.optionConditions` (schema-required when the mode is deterministic; wire-final operator names in `CONDITION_OPERATORS`) against Get Data outputs in the run history (`sourceStepId` + `fieldName` → read-record `executionResult.fields`). Top-to-bottom, first-match-wins, `and`/`or` aggregator; unresolvable/null value = `met: null` = not met (**never an error**); no match → `fallbackOption`. Never calls AI, never awaits input (`incomingPendingData` is ignored — no user override). The selected option (match or fallback) is checked against `step.options` before persisting → `InvalidStepDefinitionError`, because `optionConditions` and `options` come from two different server-side derivations and an unroutable option must not travel as a "success". Evaluation trace persisted in `executionParams` (`evaluations`/`selectedOption`/`usedFallback`) for the run view; the fallback never appears in `evaluations`. + - Comparison semantics (`deterministic-condition-evaluator.ts`): a strictly numeric string is coerced **against a real number only** (Sequelize returns `numeric`/`decimal`/`bigint` as strings while the datasource types them `Number`); an offset-less ISO datetime is read as **UTC** (host-TZ parsing would make the same run route differently per machine); a type mismatch satisfies no operator, negated ones included (`not_equal(true, 'true')` = not met); `contains`/`not_contains` are strings-only, per the contract. - **Trigger Action** (`trigger-record-action-step-executor.ts`, `handleFirstCall`) — detects the form via `getActionForm` (full field list, not `getActionFormInfo`). Formless: `FullyAutomated` runs it *in the executor* via the audited agent; otherwise pauses. With a form: `Manual` pauses with the native form (no AI fill); `AutomatedWithConfirmation` AI-fills then pauses for the user to submit natively; `FullyAutomated` AI-fills (`fillFormWithAi`) and, if `filledForm.canExecute`, submits in the executor — falling back to `awaiting-input` (pause) when required fields are missing, or on `ActionFormValidationError`/`ActionRequiresApprovalError` (a human can finish those). `UnsupportedActionFormError` is declared/exported but **never thrown** in src. - **Pre-recorded args** — record steps accept `preRecordedArgs` to skip AI. Technical names (`fieldName`/`fieldNames`/`actionName`/`relationName`) are matched exactly via `findFieldByTechnicalName` (no fuzz); `resolveAiFieldName` (exact-then-normalized) is reserved for AI-returned display names. The source record differs by step: read-record/update-record use `selectedRecordStepIndex` (a runtime index, resolved in `resolveRecordRef`); trigger-action/load-related-record use `selectedRecordStepId` — a **stable BPMN step id** (or `WORKFLOW_START_STEP_ID` sentinel) resolved by `resolveSourceRecordRef`, chosen to survive the index shifts a revision causes. Partial args supported. Unresolvable → `FieldNotFoundError`/`ActionNotFoundError`/`RelationNotFoundError`; bad shape / out-of-range index → `InvalidPreRecordedArgsError`. diff --git a/packages/workflow-executor/src/executors/condition-step-executor.ts b/packages/workflow-executor/src/executors/condition-step-executor.ts index 2b3fef9591..b151d92775 100644 --- a/packages/workflow-executor/src/executors/condition-step-executor.ts +++ b/packages/workflow-executor/src/executors/condition-step-executor.ts @@ -9,7 +9,7 @@ import type { ConditionStepOutcome } from '../types/validated/step-outcome'; import { DynamicStructuredTool, HumanMessage, SystemMessage } from '@forestadmin/ai-proxy'; import { z } from 'zod'; -import { StepStateError } from '../errors'; +import { InvalidStepDefinitionError, StepStateError } from '../errors'; import BaseStepExecutor from './base-step-executor'; import evaluateOperator from './deterministic-condition-evaluator'; import patchBodySchemas from '../http/pending-data-validators'; @@ -136,6 +136,15 @@ export default class ConditionStepExecutor extends BaseStepExecutor scalarEqual(item, expected[index])) + actual.every((item, index) => scalarEqual(item, expected[index]) === true) ); } - if (Array.isArray(actual) || Array.isArray(expected)) return false; + if (Array.isArray(actual) || Array.isArray(expected)) return null; return scalarEqual(actual, expected); } function compare(actual: unknown, expected: unknown): number | null { - if (typeof actual === 'number' && typeof expected === 'number') { - if (Number.isNaN(actual) || Number.isNaN(expected)) return null; - - return actual - expected; - } + const numbers = toNumberPair(actual, expected); + if (numbers) return numbers[0] - numbers[1]; const actualTs = toTimestamp(actual); const expectedTs = toTimestamp(expected); @@ -57,9 +82,35 @@ function isPresent(value: unknown): boolean { } function isMemberOf(list: unknown, candidate: unknown): boolean { - return Array.isArray(list) && list.some(item => scalarEqual(item, candidate)); + return Array.isArray(list) && list.some(item => scalarEqual(item, candidate) === true); +} + +function ordering(satisfies: (diff: number) => boolean) { + return (actual: unknown, expected: unknown): boolean => { + const diff = compare(actual, expected); + + return diff !== null && satisfies(diff); + }; } +const EVALUATORS: Record< + Exclude, + (actual: unknown, expected: unknown) => boolean +> = { + equal: (actual, expected) => isEqual(actual, expected) === true, + not_equal: (actual, expected) => isEqual(actual, expected) === false, + greater_than: ordering(diff => diff > 0), + less_than: ordering(diff => diff < 0), + greater_than_or_equal: ordering(diff => diff >= 0), + less_than_or_equal: ordering(diff => diff <= 0), + in: (actual, expected) => isMemberOf(expected, actual), + not_in: (actual, expected) => Array.isArray(expected) && !isMemberOf(expected, actual), + contains: (actual, expected) => + typeof actual === 'string' && typeof expected === 'string' && actual.includes(expected), + not_contains: (actual, expected) => + typeof actual === 'string' && typeof expected === 'string' && !actual.includes(expected), +}; + /** * Pure evaluation of one deterministic condition. Never throws for data reasons: * - `null` = not evaluable (the resolved value is null/missing) — treated as "not met"; @@ -75,53 +126,5 @@ export default function evaluateOperator( if (operator === 'blank') return !isPresent(actual); if (actual === null || actual === undefined) return null; - switch (operator) { - case 'equal': - return isEqual(actual, expected); - case 'not_equal': - return !isEqual(actual, expected); - - case 'greater_than': { - const diff = compare(actual, expected); - - return diff !== null && diff > 0; - } - - case 'less_than': { - const diff = compare(actual, expected); - - return diff !== null && diff < 0; - } - - case 'greater_than_or_equal': { - const diff = compare(actual, expected); - - return diff !== null && diff >= 0; - } - - case 'less_than_or_equal': { - const diff = compare(actual, expected); - - return diff !== null && diff <= 0; - } - - case 'in': - return isMemberOf(expected, actual); - case 'not_in': - return Array.isArray(expected) && !isMemberOf(expected, actual); - case 'contains': - if (typeof actual === 'string' && typeof expected === 'string') { - return actual.includes(expected); - } - - return isMemberOf(actual, expected); - case 'not_contains': - if (typeof actual === 'string' && typeof expected === 'string') { - return !actual.includes(expected); - } - - return Array.isArray(actual) && !isMemberOf(actual, expected); - default: - return null; - } + return EVALUATORS[operator](actual, expected); } diff --git a/packages/workflow-executor/test/executors/condition-step-executor.test.ts b/packages/workflow-executor/test/executors/condition-step-executor.test.ts index 55d6cac1c6..7f6bf469f9 100644 --- a/packages/workflow-executor/test/executors/condition-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/condition-step-executor.test.ts @@ -764,6 +764,143 @@ describe('ConditionStepExecutor', () => { }); }); + it('ignores incomingPendingData: no user override, no awaiting-input', async () => { + const { context, mockModel, runStore } = makeDeterministicContext( + amountArgs, + [{ name: 'amount', displayName: 'Amount', value: 150 }], + { incomingPendingData: { selectedOption: 'Low' } }, + ); + + const result = await new ConditionStepExecutor(context).execute(); + + expect(result.stepOutcome.status).toBe('success'); + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('High'); + expect(mockModel.bindTools).not.toHaveBeenCalled(); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: expect.objectContaining({ selectedOption: 'High' }), + }), + ); + }); + + it('matches an or option when one condition is not evaluable and another is met', async () => { + const orArgs: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'Unknown or high', + aggregator: 'or', + conditions: [ + { sourceStepId: 'get-1', fieldName: 'status', operator: 'equal', value: 'paid' }, + { sourceStepId: 'get-1', fieldName: 'amount', operator: 'greater_than', value: 100 }, + ], + }, + ], + fallbackOption: 'Other', + }; + const { context, runStore } = makeDeterministicContext(orArgs, [ + { name: 'status', displayName: 'Status', value: null }, + { name: 'amount', displayName: 'Amount', value: 150 }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Unknown or high'); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: expect.objectContaining({ + evaluations: [ + { + option: 'Unknown or high', + outcome: 'matched', + conditions: [ + { index: 0, met: null }, + { index: 1, met: true }, + ], + }, + ], + }), + }), + ); + }); + + it.each(['blank', 'present'] as const)( + 'does not satisfy %s when the reference cannot be resolved at all', + async operator => { + const unresolvable: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'Matched', + aggregator: 'and', + conditions: [{ sourceStepId: 'never-ran', fieldName: 'amount', operator }], + }, + ], + fallbackOption: 'Other', + }; + const { context, runStore } = makeDeterministicContext(unresolvable, [ + { name: 'amount', displayName: 'Amount', value: 150 }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Other'); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: expect.objectContaining({ + evaluations: [ + { + option: 'Matched', + outcome: 'not-matched', + conditions: [{ index: 0, met: null }], + }, + ], + }), + }), + ); + }, + ); + + it('compares a decimal column returned as a string against the builder number', async () => { + const { context } = makeDeterministicContext(amountArgs, [ + { name: 'amount', displayName: 'Amount', value: '150.00' }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('High'); + }); + + it('fails loud when the matched option is not one of the step options', async () => { + const { context, runStore } = makeDeterministicContext(amountArgs, [ + { name: 'amount', displayName: 'Amount', value: 150 }, + ]); + const result = await new ConditionStepExecutor({ + ...context, + stepDefinition: { ...context.stepDefinition, options: ['Low', 'Other'] }, + }).execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome.error).toBe( + 'The workflow step configuration is invalid. Please check the workflow designer.', + ); + expect(runStore.saveStepExecution).not.toHaveBeenCalled(); + }); + + it('fails loud when the fallback option is not one of the step options', async () => { + const { context, runStore } = makeDeterministicContext(amountArgs, [ + { name: 'amount', displayName: 'Amount', value: 'not a number' }, + ]); + const result = await new ConditionStepExecutor({ + ...context, + stepDefinition: { ...context.stepDefinition, options: ['High', 'Low'] }, + }).execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(runStore.saveStepExecution).not.toHaveBeenCalled(); + }); + it('uses the most recent occurrence of a repeated source step id (loop)', async () => { const runStore = makeMockRunStore({ getStepExecutions: jest diff --git a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts index f1d0247ad7..b0cf3f3de7 100644 --- a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -30,9 +30,9 @@ describe('evaluateOperator', () => { expect(evaluateOperator('equal', 'active', 'inactive')).toBe(false); }); - it('rejects a type mismatch (no coercion)', () => { - expect(evaluateOperator('equal', 5, '5')).toBe(false); + it('rejects a type mismatch', () => { expect(evaluateOperator('equal', true, 'true')).toBe(false); + expect(evaluateOperator('equal', 'abc', 100)).toBe(false); }); it('matches ISO dates by timestamp, not by string', () => { @@ -56,7 +56,7 @@ describe('evaluateOperator', () => { describe('not_equal', () => { it('matches different values', () => { expect(evaluateOperator('not_equal', 'active', 'inactive')).toBe(true); - expect(evaluateOperator('not_equal', 5, '5')).toBe(true); + expect(evaluateOperator('not_equal', 5, 6)).toBe(true); }); it('rejects identical values', () => { @@ -65,6 +65,39 @@ describe('evaluateOperator', () => { evaluateOperator('not_equal', '2026-01-01T00:00:00Z', '2026-01-01T00:00:00.000Z'), ).toBe(false); }); + + it('is not satisfied by a type mismatch, like every other operator', () => { + expect(evaluateOperator('not_equal', true, 'true')).toBe(false); + expect(evaluateOperator('not_equal', 5, 'abc')).toBe(false); + expect(evaluateOperator('not_equal', ['a'], 'a')).toBe(false); + }); + }); + + describe('numeric strings (decimal/bigint columns come back as strings)', () => { + it('compares a numeric string against a number', () => { + expect(evaluateOperator('greater_than', '150.00', 100)).toBe(true); + expect(evaluateOperator('greater_than', '50.00', 100)).toBe(false); + expect(evaluateOperator('less_than', 100, '150.00')).toBe(true); + expect(evaluateOperator('greater_than_or_equal', '100', 100)).toBe(true); + expect(evaluateOperator('less_than_or_equal', '-3', 0)).toBe(true); + }); + + it('equates a numeric string with a number', () => { + expect(evaluateOperator('equal', '42', 42)).toBe(true); + expect(evaluateOperator('equal', 42, '42.0')).toBe(true); + expect(evaluateOperator('not_equal', '42', 42)).toBe(false); + expect(evaluateOperator('in', '150.00', [100, 150])).toBe(true); + }); + + it('leaves a non-numeric string uncoerced', () => { + expect(evaluateOperator('greater_than', 'abc', 100)).toBe(false); + expect(evaluateOperator('greater_than', '12abc', 100)).toBe(false); + expect(evaluateOperator('equal', '', 0)).toBe(false); + }); + + it('does not coerce when neither side is a number', () => { + expect(evaluateOperator('greater_than', '5', '3')).toBe(false); + }); }); describe('present', () => { @@ -122,9 +155,8 @@ describe('evaluateOperator', () => { }); it('is not met on a type mismatch or non-comparable operands', () => { - expect(evaluateOperator('greater_than', 5, '3')).toBe(false); - expect(evaluateOperator('greater_than', '5', 3)).toBe(false); expect(evaluateOperator('greater_than', 'abc', 'abd')).toBe(false); + expect(evaluateOperator('greater_than', true, 3)).toBe(false); expect(evaluateOperator('less_than', Number.NaN, 5)).toBe(false); }); }); @@ -145,6 +177,28 @@ describe('evaluateOperator', () => { expect(evaluateOperator('greater_than', '2026-02-01', 'not a date')).toBe(false); expect(evaluateOperator('less_than', 'not a date', '2026-02-01')).toBe(false); }); + + describe('on a host whose timezone is not UTC', () => { + const originalTz = process.env.TZ; + + beforeAll(() => { + process.env.TZ = 'Pacific/Kiritimati'; + }); + + afterAll(() => { + process.env.TZ = originalTz; + }); + + it('reads a datetime without an offset as UTC, not as host-local time', () => { + expect(evaluateOperator('equal', '2026-01-01T10:00:00', '2026-01-01T10:00:00Z')).toBe(true); + expect( + evaluateOperator('greater_than', '2026-01-01T12:00:00', '2026-01-01T11:00:00Z'), + ).toBe(true); + expect( + evaluateOperator('less_than', '2026-01-01T10:00:00', '2026-01-01T11:00:00+00:00'), + ).toBe(true); + }); + }); }); describe('in', () => { @@ -158,7 +212,7 @@ describe('evaluateOperator', () => { it('rejects when the value is not in the list', () => { expect(evaluateOperator('in', 'c', ['a', 'b'])).toBe(false); - expect(evaluateOperator('in', 2, ['2'])).toBe(false); + expect(evaluateOperator('in', 2, ['3'])).toBe(false); }); it('is not met when the expected value is not an array', () => { @@ -186,12 +240,8 @@ describe('evaluateOperator', () => { expect(evaluateOperator('contains', 'hello', 'world')).toBe(false); }); - it('matches membership on arrays', () => { - expect(evaluateOperator('contains', ['a', 'b'], 'b')).toBe(true); - expect(evaluateOperator('contains', ['a', 'b'], 'c')).toBe(false); - }); - - it('is not met on a type mismatch', () => { + it('is not met on anything but two strings (contract: String fields only)', () => { + expect(evaluateOperator('contains', ['a', 'b'], 'b')).toBe(false); expect(evaluateOperator('contains', 5, '5')).toBe(false); expect(evaluateOperator('contains', 'abc', 5)).toBe(false); }); @@ -203,12 +253,8 @@ describe('evaluateOperator', () => { expect(evaluateOperator('not_contains', 'hello world', 'world')).toBe(false); }); - it('matches when the array does not contain the value', () => { - expect(evaluateOperator('not_contains', ['a'], 'b')).toBe(true); - expect(evaluateOperator('not_contains', ['a'], 'a')).toBe(false); - }); - - it('is not met (never satisfied by mismatch) on a type mismatch', () => { + it('is not met (never satisfied by mismatch) on anything but two strings', () => { + expect(evaluateOperator('not_contains', ['a'], 'b')).toBe(false); expect(evaluateOperator('not_contains', 5, '5')).toBe(false); expect(evaluateOperator('not_contains', 'abc', 5)).toBe(false); });