diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index 49e30e2fe3..5e5a033084 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -33,7 +33,10 @@ 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`. There is **no deterministic mode on the wire**: a condition step is deterministic iff it carries `preRecordedArgs.optionConditions`. A deterministic gateway publishes with `aiDecision` stripped, so it arrives as `Manual` — an executor blind to the args degrades to a visible manual decision, never a silent AI one. + +- **Deterministic Condition** (`condition-step-executor.ts` + `deterministic-condition-evaluator.ts`, PRD-472) — evaluates build-time `preRecordedArgs.optionConditions` (wire-final operator names in `CONDITION_OPERATORS`; a value-bearing operator missing its `value` is rejected at the schema boundary — comparing against `undefined` can never be met, so the step would route to the fallback instead of surfacing the broken config) against Get Data outputs in the run history (`sourceStepId` + `fieldName` → read-record `executionResult.fields`). Top-to-bottom, first-match-wins, `and`/`or` aggregator; no match → `fallbackOption`. **A resolved value that is null = `met: null` = not met, never an error** (data must not fail a decision) — but an **unresolvable reference throws** `ConditionSourceNotLoadedError`: "no value was read" is a broken config, not data, and routing to the fallback would report a decision as taken when its input never arrived. Build-time validation cannot cover it, because a Get Data step may let the AI pick its fields. Same choice as every other step type (`FieldNotFoundError`, `RelationNotFoundError`, `ActionNotFoundError`). 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); an impossible calendar date is **not a date** (`Date.parse('2026-02-30')` rolls over to 2026-03-02, which would make a nonsensical config compare equal to a real value); 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/adapters/forest-server-workflow-port.ts b/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts index ef7ea8f755..1a25c46775 100644 --- a/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts +++ b/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts @@ -20,6 +20,7 @@ import toUpdateStepRequest from './step-outcome-to-update-step-mapper'; import withRetry from './with-retry'; import { DomainValidationError, + HydrationFailedError, InvalidStepDefinitionError, MalformedRunError, WorkflowExecutorError, @@ -71,14 +72,20 @@ export default class ForestServerWorkflowPort implements WorkflowPort { const dispatch = this.toDispatch(run); if (dispatch) pending.push(dispatch); } catch (error) { - if (error instanceof WorkflowExecutorError) { - malformed.push(this.toMalformedInfo(run, error)); - } else { - this.logger('Error', 'Failed to hydrate pending run — unexpected error', { - runId: run.id, - error: extractErrorMessage(error), - }); - } + // Reported whatever it is: an unreported failure leaves the run pending, so every poll + // returns it and fails again. + this.logger('Error', 'Failed to hydrate pending run', { + runId: run.id, + error: extractErrorMessage(error), + }); + malformed.push( + this.toMalformedInfo( + run, + error instanceof WorkflowExecutorError + ? error + : new HydrationFailedError(extractErrorMessage(error)), + ), + ); } } @@ -137,7 +144,8 @@ export default class ForestServerWorkflowPort implements WorkflowPort { run: ServerHydratedWorkflowRun, err: WorkflowExecutorError, ): MalformedRunInfo { - const pending = run.workflowHistory.at(-1) ?? null; + const history = Array.isArray(run.workflowHistory) ? run.workflowHistory : []; + const pending = history.at(-1) ?? null; return { runId: String(run.id), diff --git a/packages/workflow-executor/src/adapters/server-types.ts b/packages/workflow-executor/src/adapters/server-types.ts index 101fbd67c3..831479a87b 100644 --- a/packages/workflow-executor/src/adapters/server-types.ts +++ b/packages/workflow-executor/src/adapters/server-types.ts @@ -93,7 +93,7 @@ interface ServerWorkflowTaskLoadRelatedRecord extends ServerWorkflowTaskBase { executionType: | ServerStepExecutionTypeEnum.FullyAutomated | ServerStepExecutionTypeEnum.AutomatedWithConfirmation; - // Deterministic build-time config. Validated by the step-definition schema. + // Validated by the step-definition schema. preRecordedArgs?: { selectedRecordStepId?: string; relationName?: string }; } @@ -125,6 +125,21 @@ export interface ServerWorkflowCondition extends ServerWorkflowStepBase { executionType: ServerStepExecutionTypeEnum.Manual | ServerStepExecutionTypeEnum.FullyAutomated; prompt: string | null; automaticCompletion: false; + // Parsed server-side from `forest:optionConditions` (flowId → answer). Its presence is what makes + // the gateway deterministic. + 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/errors.ts b/packages/workflow-executor/src/errors.ts index 657afc5228..afd302ceb5 100644 --- a/packages/workflow-executor/src/errors.ts +++ b/packages/workflow-executor/src/errors.ts @@ -543,6 +543,28 @@ export class InvalidStepDefinitionError extends WorkflowExecutorError { } } +export class HydrationFailedError extends WorkflowExecutorError { + constructor(detail: string) { + super( + `Failed to hydrate run: ${detail}`, + 'This workflow run could not be prepared for execution. Please contact support.', + ); + } +} + +// A deterministic condition reads a field a Get Data step was supposed to have loaded. Build-time +// validation cannot always catch this: when that step lets the AI pick its fields, nobody knows +// which ones it will return until the run. Treating it as "not met" would route to the fallback and +// report success — the decision would look taken when its input never arrived. +export class ConditionSourceNotLoadedError extends WorkflowExecutorError { + constructor(fieldName: string, sourceStepId: string) { + super( + `Condition reads "${fieldName}" from step "${sourceStepId}", which did not load it`, + `This decision compares the field "${fieldName}", but the step it reads from did not load that field. Add it to that step's fields, or remove the condition.`, + ); + } +} + // Thrown when zod validation fails on a domain object produced internally (e.g. by the // run-to-pending-step mapper). Distinct from InvalidStepDefinitionError (which flags wire-format // bugs coming from the orchestrator) so the two can be triaged separately in Sentry. diff --git a/packages/workflow-executor/src/executors/condition-step-executor.ts b/packages/workflow-executor/src/executors/condition-step-executor.ts index 36580d0956..b770c23805 100644 --- a/packages/workflow-executor/src/executors/condition-step-executor.ts +++ b/packages/workflow-executor/src/executors/condition-step-executor.ts @@ -1,14 +1,28 @@ 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, + DeterministicConditionStep, +} from '../types/validated/step-definition'; 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 { + ConditionSourceNotLoadedError, + InvalidStepDefinitionError, + 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, + isDeterministicConditionStep, +} from '../types/validated/step-definition'; interface GatewayToolArgs { option: string | null; @@ -62,6 +76,20 @@ export default class ConditionStepExecutor extends BaseStepExecutor { const { stepDefinition: step, incomingPendingData } = this.context; + if (isDeterministicConditionStep(step)) { + // The config wins over an explicit human action, so say so: a mixed-version fleet can pause + // the step on an args-blind instance, and the user's click would otherwise vanish untraced. + if (incomingPendingData !== undefined) { + this.context.logger( + 'Warn', + 'Ignoring a submitted option: this decision is evaluated from its conditions', + this.logCtx, + ); + } + + 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 +120,97 @@ export default class ConditionStepExecutor extends BaseStepExecutor { + 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; + + // optionConditions and options come from two different server-side derivations; an option the + // orchestrator cannot route must fail here, not silently succeed and break the run downstream. + if (!step.options.includes(selectedOption)) { + const allowed = step.options.join(', '); + throw new InvalidStepDefinitionError( + `deterministic option "${selectedOption}" is not a valid choice (expected one of: ${allowed})`, + ); + } + + 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); + + // A missing *reference* is a broken config, not data: routing to the fallback would report a + // decision as taken when its input never arrived. Every other step type already throws here + // (FieldNotFoundError, RelationNotFoundError, ActionNotFoundError) — this one used to be the + // exception. A value that is present but null still counts as not met, per the spec. + if (!resolved.found) { + throw new ConditionSourceNotLoadedError(condition.fieldName, condition.sourceStepId); + } + + 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..67296e0e12 --- /dev/null +++ b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts @@ -0,0 +1,139 @@ +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}/; +const TIMEZONE_SUFFIX = /(Z|[+-]\d{2}:?\d{2})$/i; +// Sequelize hands back numeric/decimal/bigint columns as strings although datasource-sequelize +// maps them to the Number primitive, so the builder's JSON number meets a string at runtime. +const NUMERIC_STRING = /^-?\d+(\.\d+)?$/; + +// Date.parse rolls an out-of-range day over ("2026-02-30" becomes 2026-03-02), which would make a +// nonsensical config compare equal to a real date instead of being inert. +function isRealCalendarDate(datePart: string): boolean { + const parsed = new Date(`${datePart}T00:00:00.000Z`); + + return !Number.isNaN(parsed.getTime()) && parsed.toISOString().startsWith(datePart); +} + +function toTimestamp(value: unknown): number | null { + if (typeof value !== 'string' || !ISO_DATE_PREFIX.test(value)) return null; + if (!isRealCalendarDate(value.slice(0, 10))) return null; + + // Date.parse reads an offset-less datetime as host-local (date-only as UTC), which would route + // the same run differently per machine — pin every offset-less datetime to UTC. + const absolute = value.includes('T') && !TIMEZONE_SUFFIX.test(value) ? `${value}Z` : value; + const parsed = Date.parse(absolute); + + return Number.isNaN(parsed) ? null : parsed; +} + +function toNumber(value: unknown): number | null { + if (typeof value === 'number') return Number.isNaN(value) ? null : value; + + return typeof value === 'string' && NUMERIC_STRING.test(value) ? Number(value) : null; +} + +// Coercion only kicks in against a real number (always the build-time side): two numeric-looking +// strings stay strings, since the contract exposes ordering operators for Number/Date fields only. +function toNumberPair(actual: unknown, expected: unknown): [number, number] | null { + if (typeof actual !== 'number' && typeof expected !== 'number') return null; + + const actualNumber = toNumber(actual); + const expectedNumber = toNumber(expected); + + return actualNumber !== null && expectedNumber !== null ? [actualNumber, expectedNumber] : null; +} + +function scalarEqual(actual: unknown, expected: unknown): boolean | null { + if (actual === expected) return true; + + const numbers = toNumberPair(actual, expected); + if (numbers) return numbers[0] === numbers[1]; + + const actualTs = toTimestamp(actual); + const expectedTs = toTimestamp(expected); + if (actualTs !== null && expectedTs !== null) return actualTs === expectedTs; + + return typeof actual === typeof expected ? false : null; +} + +function isEqual(actual: unknown, expected: unknown): boolean | null { + if (Array.isArray(actual) && Array.isArray(expected)) { + return ( + actual.length === expected.length && + actual.every((item, index) => scalarEqual(item, expected[index]) === true) + ); + } + + if (Array.isArray(actual) || Array.isArray(expected)) return null; + + return scalarEqual(actual, expected); +} + +function compare(actual: unknown, expected: unknown): number | null { + const numbers = toNumberPair(actual, expected); + if (numbers) return numbers[0] - numbers[1]; + + 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) === 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"; + * - 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; + + return EVALUATORS[operator](actual, expected); +} 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..0a3f2cc284 100644 --- a/packages/workflow-executor/src/types/validated/step-definition.ts +++ b/packages/workflow-executor/src/types/validated/step-definition.ts @@ -40,17 +40,81 @@ const sharedFields = { // Use z.enum(EnumObject), not z.nativeEnum — the latter is deprecated in zod 4. const { Manual, AutomatedWithConfirmation, FullyAutomated } = StepExecutionMode; +// 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 VALUE_LESS_OPERATORS: readonly ConditionOperator[] = ['present', 'blank']; + +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(), + }) + // A value-bearing operator without its value compares against `undefined`: it can never be met, + // so the step would silently route to the fallback instead of reporting the broken config. + .superRefine((condition, ctx) => { + if (!VALUE_LESS_OPERATORS.includes(condition.operator) && condition.value === undefined) { + ctx.addIssue({ + code: 'custom', + path: ['value'], + message: `value is required for the "${condition.operator}" operator`, + }); + } + }); +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 (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. + // NO `.catch` — coercing an unknown mode to FullyAutomated would turn a gateway the builder made + // manual into an AI decision. executionType: z.enum([Manual, FullyAutomated]).default(FullyAutomated), options: z.array(z.string()).min(2), + // A malformed config is rejected here rather than dropped, so it can never degrade to a + // manual/AI decision. + preRecordedArgs: z + .object({ + optionConditions: z.array(OptionConditionsSchema).min(1), + fallbackOption: z.string().min(1), + }) + .optional(), }); export type ConditionStepDefinition = z.infer; +export type DeterministicConditionStep = ConditionStepDefinition & + Required>; + +// Carrying the conditions *is* the deterministic mode — no executionType says so. +export const isDeterministicConditionStep = ( + step: ConditionStepDefinition, +): step is DeterministicConditionStep => step.preRecordedArgs !== undefined; + export const ReadRecordStepDefinitionSchema = z.object({ ...sharedFields, type: z.literal(StepType.ReadRecord), diff --git a/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts b/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts index 97fb52c6b3..896e6d7265 100644 --- a/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts +++ b/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts @@ -313,21 +313,29 @@ describe('ForestServerWorkflowPort', () => { ); }); - it('logs and skips when the mapping throws a non-WorkflowExecutorError', async () => { + it('reports a non-WorkflowExecutorError as malformed instead of skipping the run', async () => { const logger = jest.fn(); const portWithLogger = new ForestServerWorkflowPort({ ...options, logger }); - // Simulate a non-domain error by passing a run whose workflowHistory will - // blow up a pure JS operation inside the mapper (missing `find` on non-array). - const brokenRun = { ...makeRun({ id: 111 }), workflowHistory: null as never }; + // A numeric selectedRecordId passes the mapper's truthiness guard, then blows up on .split + // — a TypeError, not a domain error, on a run whose history is otherwise intact. + const brokenRun = { ...makeRun({ id: 111 }), selectedRecordId: 42 as never }; mockQuery.mockResolvedValue([brokenRun]); const result = await portWithLogger.getAvailableRuns(); expect(result.pending).toEqual([]); - expect(result.malformed).toEqual([]); + expect(result.malformed).toEqual([ + expect.objectContaining({ + runId: '111', + stepId: expect.any(String), + stepIndex: expect.any(Number), + userMessage: + 'This workflow run could not be prepared for execution. Please contact support.', + }), + ]); expect(logger).toHaveBeenCalledWith( 'Error', - 'Failed to hydrate pending run — unexpected error', + 'Failed to hydrate pending run', expect.objectContaining({ runId: 111 }), ); }); 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..4e1b00f660 100644 --- a/packages/workflow-executor/test/adapters/step-definition-mapper.test.ts +++ b/packages/workflow-executor/test/adapters/step-definition-mapper.test.ts @@ -309,20 +309,66 @@ describe('toStepDefinition', () => { }); }); - // A newer orchestrator may send a deterministic 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', () => { + // 'deterministic' is now just another unknown mode: the wire contract dropped it. + it.each(['not-a-mode', 'deterministic'])( + 'should throw InvalidStepDefinitionError for the unknown executionType "%s" instead of coercing to Full AI', + executionType => { + const condition = makeCondition( + [ + { stepId: 's1', buttonText: null, answer: 'Yes' }, + { stepId: 's2', buttonText: null, answer: 'No' }, + ], + { executionType: executionType as ServerWorkflowCondition['executionType'] }, + ); + + expect(() => toStepDefinition(condition)).toThrow(InvalidStepDefinitionError); + expect(() => toStepDefinition(condition)).toThrow(/executionType/); + }, + ); + + it('should forward preRecordedArgs on a manual 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.Manual, preRecordedArgs }, + ); + + expect(toStepDefinition(condition)).toMatchObject({ + type: StepType.Condition, + executionType: StepExecutionMode.Manual, + options: ['Yes', 'No'], + preRecordedArgs, + }); + }); + + it('should throw InvalidStepDefinitionError for malformed preRecordedArgs rather than dropping them', () => { const condition = makeCondition( [ { stepId: 's1', buttonText: null, answer: 'Yes' }, { stepId: 's2', buttonText: null, answer: 'No' }, ], - { executionType: 'deterministic' as ServerWorkflowCondition['executionType'] }, + { + executionType: ServerStepExecutionTypeEnum.Manual, + preRecordedArgs: { optionConditions: [], fallbackOption: 'No' }, + }, ); expect(() => toStepDefinition(condition)).toThrow(InvalidStepDefinitionError); - expect(() => toStepDefinition(condition)).toThrow(/executionType/); + expect(() => toStepDefinition(condition)).toThrow(/optionConditions/); }); it('should throw InvalidStepDefinitionError when fewer than 2 options', () => { 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..71b7a3b0c8 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,43 @@ function makeStep(overrides: Partial = {}): ConditionSt }; } +type ConditionPreRecordedArgs = NonNullable; + +// Published with aiDecision stripped, so a deterministic gateway arrives as manual: the args make it deterministic, not the mode. +function makeDeterministicStep( + preRecordedArgs: ConditionPreRecordedArgs, + executionType: ConditionStepDefinition['executionType'] = StepExecutionMode.Manual, +): ConditionStepDefinition { + return makeStep({ + executionType, + 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,18 +459,517 @@ describe('ConditionStepExecutor', () => { }); }); + describe('deterministic evaluation driven by preRecordedArgs', () => { + 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('evaluates a manual condition instead of awaiting input, with no incomingPendingData', async () => { + const { context, mockModel, runStore } = makeDeterministicContext(amountArgs, [ + { name: 'amount', displayName: 'Amount', value: 150 }, + ]); + + expect(context.stepDefinition.executionType).toBe(StepExecutionMode.Manual); + expect(context.incomingPendingData).toBeUndefined(); + + 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', usedFallback: false }), + }), + ); + }); + + it('evaluates a fully-automated condition carrying preRecordedArgs without calling the AI', async () => { + const { context, mockModel, runStore } = makeDeterministicContext( + amountArgs, + [{ name: 'amount', displayName: 'Amount', value: 150 }], + { stepDefinition: makeDeterministicStep(amountArgs, StepExecutionMode.FullyAutomated) }, + ); + + 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(mockModel.invoke).not.toHaveBeenCalled(); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: expect.objectContaining({ + evaluations: [ + { option: 'High', outcome: 'matched', conditions: [{ index: 0, met: true }] }, + { option: 'Low', outcome: 'not-evaluated' }, + ], + }), + }), + ); + }); + + 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('fails loud when the source step never ran', async () => { + const unknownSource: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'High', + aggregator: 'and', + conditions: [ + { + sourceStepId: 'never-ran', + fieldName: 'amount', + operator: 'greater_than', + value: 100, + }, + ], + }, + ], + fallbackOption: 'Other', + }; + const { context, runStore } = makeDeterministicContext(unknownSource, [ + { name: 'amount', displayName: 'Amount', value: 150 }, + ]); + + const result = await new ConditionStepExecutor(context).execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome.error).toContain('did not load that field'); + expect(runStore.saveStepExecution).not.toHaveBeenCalled(); + }); + + // Build-time validation cannot catch this one: the Get Data step may let the AI pick its + // fields, so nobody knows which ones it returns until the run. + it('fails loud when the Get Data step failed to read the field', 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.status).toBe('error'); + expect(result.stepOutcome.error).toContain('did not load that field'); + expect(runStore.saveStepExecution).not.toHaveBeenCalled(); + }); + + 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('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 }, + ], + }, + ], + }), + }), + ); + }); + + // Not even present/blank get an answer out of a reference that was never loaded: "no value was + // read" is not the same claim as "the value is empty". + it.each(['blank', 'present'] as const)( + 'fails loud on %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.status).toBe('error'); + expect(result.stepOutcome.error).toContain('did not load that field'); + expect(runStore.saveStepExecution).not.toHaveBeenCalled(); + }, + ); + + 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 + .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 () => { + it('returns awaiting-input without preRecordedArgs, no AI and no save, when no incomingPendingData', async () => { const mockModel = makeMockModel(); const runStore = makeMockRunStore(); + const stepDefinition = makeStep({ executionType: StepExecutionMode.Manual }); const executor = new ConditionStepExecutor( - makeContext({ - model: mockModel.model, - runStore, - stepDefinition: makeStep({ executionType: StepExecutionMode.Manual }), - }), + makeContext({ model: mockModel.model, runStore, stepDefinition }), ); + expect(stepDefinition.preRecordedArgs).toBeUndefined(); + const result = await executor.execute(); expect(result.stepOutcome.status).toBe('awaiting-input'); 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..026bd1162e --- /dev/null +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -0,0 +1,275 @@ +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', () => { + expect(evaluateOperator('equal', true, 'true')).toBe(false); + expect(evaluateOperator('equal', 'abc', 100)).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, 6)).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); + }); + + 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', () => { + 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', 'abc', 'abd')).toBe(false); + expect(evaluateOperator('greater_than', true, 3)).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); + }); + + it('treats an impossible calendar date as not a date instead of rolling it over', () => { + expect(evaluateOperator('equal', '2026-03-02', '2026-02-30')).toBe(false); + expect(evaluateOperator('equal', '2026-05-01', '2026-04-31')).toBe(false); + expect(evaluateOperator('equal', '2025-03-01', '2025-02-29')).toBe(false); + expect(evaluateOperator('greater_than', '2026-02-30', '2026-01-01')).toBe(false); + expect(evaluateOperator('less_than_or_equal', '2026-01-01', '2026-02-30')).toBe(false); + expect(evaluateOperator('in', '2026-03-02', ['2026-02-30'])).toBe(false); + }); + + it('still accepts a leap day that exists', () => { + expect(evaluateOperator('equal', '2024-02-29', '2024-02-29T00:00:00.000Z')).toBe(true); + }); + + 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', () => { + 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, ['3'])).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('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); + }); + }); + + 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('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); + }); + }); +}); diff --git a/packages/workflow-executor/test/types/step-definition.test.ts b/packages/workflow-executor/test/types/step-definition.test.ts index ff4337d8ad..36a6dc8758 100644 --- a/packages/workflow-executor/test/types/step-definition.test.ts +++ b/packages/workflow-executor/test/types/step-definition.test.ts @@ -25,15 +25,237 @@ 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). - it('rejects an invalid executionType instead of coercing it', () => { - expect( - ConditionStepDefinitionSchema.safeParse({ ...base, executionType: 'deterministic' }).success, - ).toBe(false); + // 'deterministic' is now an unknown mode — and unknown modes must be rejected, not coerced. + it.each(['not-a-mode', 'deterministic'])( + 'rejects the unknown executionType "%s" instead of coercing it', + executionType => { + expect(ConditionStepDefinitionSchema.safeParse({ ...base, executionType }).success).toBe( + false, + ); + }, + ); + + it('rejects the removed "deterministic" mode even when preRecordedArgs are present', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'deterministic', + preRecordedArgs: { + optionConditions: [ + { + option: 'Yes', + aggregator: 'and', + conditions: [{ sourceStepId: 'get-data-1', fieldName: 'amount', operator: 'present' }], + }, + ], + fallbackOption: 'No', + }, + }); + + expect(result.success).toBe(false); + expect(JSON.stringify(!result.success && result.error.issues)).toContain('executionType'); + }); +}); + +describe('ConditionStepDefinitionSchema deterministic conditions', () => { + 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 manual with preRecordedArgs and round-trips them', () => { + const parsed = ConditionStepDefinitionSchema.parse({ + ...base, + executionType: 'manual', + preRecordedArgs, + }); + + expect(parsed.executionType).toBe(StepExecutionMode.Manual); + expect(parsed.preRecordedArgs).toEqual(preRecordedArgs); + }); + + it('accepts preRecordedArgs with no executionType at all', () => { + const parsed = ConditionStepDefinitionSchema.parse({ ...base, preRecordedArgs }); + + expect(parsed.executionType).toBe(StepExecutionMode.FullyAutomated); + expect(parsed.preRecordedArgs).toEqual(preRecordedArgs); + }); + + // value supplied so the failure is the operator enum, not the value-less-operator refinement. + it('rejects an unknown operator at the schema boundary', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'manual', + preRecordedArgs: { + ...preRecordedArgs, + optionConditions: [ + { + option: 'High value', + aggregator: 'and', + conditions: [ + { sourceStepId: 'get-data-1', fieldName: 'amount', operator: 'ilike', value: '%x%' }, + ], + }, + ], + }, + }); + + expect(result.success).toBe(false); + expect(JSON.stringify(!result.success && result.error.issues)).toContain('operator'); + }); + + // An empty reference resolves to "not found" → met: null → not met → the step routes to the + // fallback. Same silent-fallback failure the value-less-operator refinement exists to prevent. + it.each([ + ['sourceStepId', { sourceStepId: '', fieldName: 'amount' }], + ['fieldName', { sourceStepId: 'get-data-1', fieldName: '' }], + ])('rejects a condition with an empty %s', (_label, reference) => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + preRecordedArgs: { + ...preRecordedArgs, + optionConditions: [ + { + option: 'High value', + aggregator: 'and', + conditions: [{ ...reference, operator: 'equal', value: 1 }], + }, + ], + }, + }); + + expect(result.success).toBe(false); + expect(JSON.stringify(!result.success && result.error.issues)).toContain(_label); + }); + + it.each([ + ['option', { option: '', fallbackOption: 'Otherwise' }], + ['fallbackOption', { option: 'High value', fallbackOption: '' }], + ])( + 'rejects an empty %s, which no outgoing flow can answer', + (_label, { option, fallbackOption }) => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + preRecordedArgs: { + fallbackOption, + optionConditions: [ + { + option, + aggregator: 'and', + conditions: [ + { sourceStepId: 'get-data-1', fieldName: 'amount', operator: 'equal', value: 1 }, + ], + }, + ], + }, + }); + + expect(result.success).toBe(false); + expect(JSON.stringify(!result.success && result.error.issues)).toContain(_label); + }, + ); + + it('rejects preRecordedArgs missing fallbackOption', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'manual', + preRecordedArgs: { optionConditions: preRecordedArgs.optionConditions }, + }); + + expect(result.success).toBe(false); + }); + + it('rejects an option with an unknown aggregator', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'manual', + 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: 'manual', + preRecordedArgs: { + ...preRecordedArgs, + optionConditions: [{ option: 'High value', aggregator: 'and', conditions: [] }], + }, + }); + + expect(result.success).toBe(false); + }); + + it('rejects preRecordedArgs with zero optionConditions', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'manual', + preRecordedArgs: { ...preRecordedArgs, optionConditions: [] }, + }); + + expect(result.success).toBe(false); + expect(JSON.stringify(!result.success && result.error.issues)).toContain('optionConditions'); + }); + + it.each(['equal', 'not_equal', 'greater_than', 'in', 'contains'])( + 'rejects a "%s" condition with no value', + operator => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'manual', + preRecordedArgs: { + ...preRecordedArgs, + optionConditions: [ + { + option: 'High value', + aggregator: 'and', + conditions: [{ sourceStepId: 'get-data-1', fieldName: 'amount', operator }], + }, + ], + }, + }); + + expect(result.success).toBe(false); + }, + ); + + it('accepts a value-less condition for present/blank operators', () => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'manual', + 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 a condition with no preRecordedArgs at all', () => { expect( - ConditionStepDefinitionSchema.safeParse({ ...base, executionType: 'not-a-mode' }).success, - ).toBe(false); + ConditionStepDefinitionSchema.safeParse({ ...base, executionType: 'fully-automated' }) + .success, + ).toBe(true); }); });