From a0ec99cbed04067b3f163e514d1aa79839c46746 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Fri, 4 Sep 2026 18:05:24 +0200 Subject: [PATCH 01/10] feat(workflow-executor): evaluate the operators a list view filter offers, relative dates included MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deterministic Decision accepted 12 operators, three of which the editor could never build, and none of the date family beyond equality. A Decision on a date could not say "before", "after" or "in the previous 7 days", which is the first thing anyone writes on a date. The operator set is now the one a list view filter offers, 23 names: the three orphans (>=, <=, not_in) go, and 14 arrive — before/after, past/future, today/yesterday, previous_x_days(_to_date), before/after_x_hours_ago, starts_with/ends_with/i_contains, includes_all. Relative dates read an injected Clock, never the machine's. The orchestrator sends the project's timezone on the run (mapper falls back to UTC when unset or when the orchestrator predates it); the step executor builds one clock per step so every row sees the same instant. Day windows are half open in that zone, mirroring datasource-toolkit's time transforms, and a calendar date is read at midnight in that zone rather than UTC, otherwise Honolulu's own today would count as yesterday. i_contains folds case and keeps accents, measured against Postgres ILIKE. The trace persists evaluatedAt and timezone, without which a check on "previous 7 days" cannot be explained a day later. Product decisions recorded on PRD-1147: evaluation instant, project timezone, Postgres semantics for text, all operators in one delivery, and a retry that may route differently. Co-Authored-By: Claude Opus 5 (1M context) --- packages/workflow-executor/CLAUDE.md | 3 +- packages/workflow-executor/package.json | 2 + .../adapters/run-to-available-step-mapper.ts | 4 + .../src/adapters/server-types.ts | 2 + .../src/executors/condition-step-executor.ts | 18 +- .../deterministic-condition-evaluator.ts | 165 ++++++- .../src/executors/step-executor-factory.ts | 1 + .../src/types/execution-context.ts | 1 + .../src/types/step-execution-data.ts | 7 + .../src/types/validated/execution.ts | 2 + .../src/types/validated/step-definition.ts | 30 +- .../run-to-available-step-mapper.test.ts | 15 + .../test/executors/base-step-executor.test.ts | 1 + .../executors/condition-step-executor.test.ts | 79 +++- .../deterministic-condition-evaluator.test.ts | 401 +++++++++++------- .../executors/guidance-step-executor.test.ts | 1 + .../load-related-record-step-executor.test.ts | 1 + .../test/executors/mcp-step-executor.test.ts | 1 + .../read-record-step-executor.test.ts | 1 + .../executors/step-executor-factory.test.ts | 1 + ...rigger-record-action-step-executor.test.ts | 1 + .../update-record-step-executor.test.ts | 1 + .../integration/workflow-execution.test.ts | 1 + .../workflow-executor/test/runner.test.ts | 1 + 24 files changed, 559 insertions(+), 181 deletions(-) diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index 3c7d6e105b..172609d52b 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -36,7 +36,8 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p `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`. **No condition evaluation ever fails the step, data or reference alike**: a resolved value that is null is `met: null`, and a reference that read nothing is `met: null` + a `reason` (`source-step-not-reached` when no Get Data with that id ran on this path, `field-not-loaded` when one ran but the field is absent or errored), plus a `Warn`. Build-time validation cannot cover the second case (a Get Data step may let the AI pick its fields), so the run has to answer at runtime — and the way a decision step answers is by routing, unlike record steps (line above) which die on an unresolvable reference. What stops the fallback from passing for a decision the data took is the `reason`: the run view words it for the operator instead of showing a bare cross. "Never read" is not "read and empty", so `present`/`blank` get no answer out of it either. 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. + - **Operator set = what a list view filter offers** (PRD-1147), 23 names in `CONDITION_OPERATORS`; nothing the front registry cannot render (so no `>=`, `<=`, `not_in`). 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`), whole numbers exactly as BigInt; 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); text operators are strings-only, case sensitive except `i_contains`, which folds case and keeps accents like Postgres `ILIKE`. + - **Relative dates read an injected `Clock`** (`{ now, timezone }`), never the machine's: the step executor builds one per step (so every row of a Decision sees the same instant) from `ExecutionContext.timezone`, which the orchestrator sends as the **project's** zone (`AvailableStepExecution.timezone`, mapper falls back to `UTC` when unset or absent). The machine's zone is never used: the fleet runs several instances, and the same run must route the same on each. Day windows (`today`, `yesterday`, `previous_x_days`, `previous_x_days_to_date`) are half open `[start, end)` in that zone, mirroring `datasource-toolkit`'s time transforms; a **calendar date** (Dateonly) is read at midnight in that zone, otherwise Honolulu's own "today" would count as yesterday. The trace persists `evaluatedAt` + `timezone` alongside `evaluations`, since a relative condition cannot be explained later without the instant it was read at. A retry re-evaluates with a fresh clock and may route differently: accepted, by product decision. - **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. All four record steps pin the source by `selectedRecordStepId` — a **stable BPMN step id** (or the `WORKFLOW_START_STEP_ID` sentinel) resolved by `resolveSourceRecordRef`, chosen to survive the index shifts a revision causes; the editor writes it and treats it as a precondition for choosing fields. Presence, not truthiness, decides whether something is pinned -- `selectedRecordStepId`, `actionName` and `relationName` all check `!== undefined`. An empty value is a pin that lost its target, so it resolves to nothing and errors instead of falling back to the AI, which would silently pick a different record, run a different action, or follow a different relation than the step was configured to. Not reachable from the editor, which writes `stepId || undefined`. `selectedRecordStepIndex` (a runtime index, resolved in `resolveRecordRef`) survives only as a fallback on read-record/update-record, checked after the step id. Partial args supported. Unresolvable → `PinnedArgNotFoundError` when the name was pinned (`configuration`), `FieldNotFoundError`/`ActionNotFoundError`/`RelationNotFoundError` when the AI chose it (**unclassified** — a re-run may resolve differently, so nothing permanent can be asserted); bad shape / out-of-range index → `InvalidPreRecordedArgsError`. The split exists because the AI-facing messages tell the operator to rephrase the prompt — the right remedy for a name the AI chose, and unreachable for one the workflow fixed, where the step itself is the thing to edit. One class covers every pinned kind: the diagnosis differs per kind, the operator's remedy does not. Anything raised on a pinned value has to name the step. diff --git a/packages/workflow-executor/package.json b/packages/workflow-executor/package.json index 6cc5e6662b..4cec88b7b7 100644 --- a/packages/workflow-executor/package.json +++ b/packages/workflow-executor/package.json @@ -37,6 +37,7 @@ "jsonwebtoken": "^9.0.3", "koa": "^3.0.1", "koa-jwt": "^4.0.4", + "luxon": "^3.2.1", "pg": "^8.8.0", "picocolors": "^1.1.1", "sequelize": "^6.37.8", @@ -44,6 +45,7 @@ "zod": "4.3.6" }, "devDependencies": { + "@types/luxon": "^3.2.0", "@types/jsonwebtoken": "^9.0.10", "@types/koa": "^2.13.5", "@types/koa__router": "^12.0.4", diff --git a/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts b/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts index 942bd28899..cdec80def7 100644 --- a/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts +++ b/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts @@ -178,6 +178,10 @@ export default function toAvailableStepExecution( stepDefinition: toStepDefinition(pending.stepDefinition), previousSteps: toPreviousSteps(run.workflowHistory, pending.stepIndex), user: toStepUser(run.id, run.userProfile), + // UTC when the project has none set, and when the orchestrator is too old to send one: a + // relative date must resolve the same on every executor instance, so the machine's zone is + // never the fallback. + timezone: run.timezone || 'UTC', }; // Defense against mapper bugs: zod asserts the shape we produce is what the domain expects, diff --git a/packages/workflow-executor/src/adapters/server-types.ts b/packages/workflow-executor/src/adapters/server-types.ts index 831479a87b..6d9c262e1d 100644 --- a/packages/workflow-executor/src/adapters/server-types.ts +++ b/packages/workflow-executor/src/adapters/server-types.ts @@ -226,6 +226,8 @@ export interface ServerHydratedWorkflowRun { renderingId: number; lockedAt?: string | null; userProfile: ServerUserProfile; + /** The project's IANA zone. Absent from an orchestrator that predates it, null when unset. */ + timezone?: string | null; } // --- Update step request (POST /api/workflow-orchestrator/update-step) --- diff --git a/packages/workflow-executor/src/executors/condition-step-executor.ts b/packages/workflow-executor/src/executors/condition-step-executor.ts index 298d142a7b..645bf0801a 100644 --- a/packages/workflow-executor/src/executors/condition-step-executor.ts +++ b/packages/workflow-executor/src/executors/condition-step-executor.ts @@ -16,7 +16,7 @@ import { z } from 'zod'; import { InvalidStepDefinitionError, StepStateError } from '../errors'; import BaseStepExecutor from './base-step-executor'; -import evaluateOperator from './deterministic-condition-evaluator'; +import evaluateOperator, { type Clock } from './deterministic-condition-evaluator'; import patchBodySchemas from '../http/pending-data-validators'; import { StepExecutionMode, @@ -127,6 +127,9 @@ export default class ConditionStepExecutor extends BaseStepExecutor { const { optionConditions, fallbackOption } = step.preRecordedArgs; const stepExecutions = await this.context.runStore.getStepExecutions(this.context.runId); + // One clock for the whole step: every condition of every option reads the same instant, so + // "today" cannot flip between two rows evaluated a millisecond apart. + const clock: Clock = { now: new Date(), timezone: this.context.timezone }; let matchedOption: string | undefined; const evaluations = optionConditions.map(({ option, aggregator, conditions }) => { @@ -135,7 +138,7 @@ export default class ConditionStepExecutor extends BaseStepExecutor { - const { met, reason } = this.evaluateCondition(condition, stepExecutions); + const { met, reason } = this.evaluateCondition(condition, stepExecutions, clock); return { index, met, ...(reason && { reason }) }; }); @@ -167,7 +170,13 @@ export default class ConditionStepExecutor extends BaseStepExecutor scalarEqual(item, candidate) === true); +} + +function includesAll(actual: unknown, expected: unknown): boolean { + if (!Array.isArray(actual)) return false; + const wanted = Array.isArray(expected) ? expected : [expected]; - const results = list.map(item => scalarEqual(item, candidate)); - if (results.includes(true)) return true; + return wanted.every(item => isMemberOf(actual, item)); +} - return results.includes(null) ? null : false; +// Strings only: no coercion of a number into text, so a broken config can never accidentally +// satisfy a text operator. +function stringTest(satisfies: (actual: string, expected: string) => boolean) { + return (actual: unknown, expected: unknown): boolean => + typeof actual === 'string' && typeof expected === 'string' && satisfies(actual, expected); } function ordering(satisfies: (diff: number) => boolean) { @@ -139,22 +168,120 @@ function ordering(satisfies: (diff: number) => boolean) { }; } +// The builder pins a datetime for a Date column and a calendar date for a Dateonly one, so both +// sides share a kind and both go through toInstant: a calendar date is read in the project's zone +// on either side, which keeps "before 2026-03-01" meaning the same day the author had in mind. +function dateOrdering(satisfies: (actual: DateTime, expected: DateTime) => boolean) { + return (actual: unknown, expected: unknown, clock: Clock): boolean => { + const actualInstant = toInstant(actual, clock.timezone); + const expectedInstant = toInstant(expected, clock.timezone); + + return ( + actualInstant !== null && + expectedInstant !== null && + satisfies(actualInstant, expectedInstant) + ); + }; +} + +// Relative to the clock. A window is half open, [start, end): a value at the very start of today +// is today, a value at the very start of tomorrow is not. Mirrors datasource-toolkit's time +// transforms, which is what the list filter runs on, so both agree on what "today" covers. +type Window = (now: DateTime, value: unknown) => [start: DateTime, end: DateTime] | null; + +function days(value: unknown): number | null { + const count = toNumber(value); + + return count !== null && Number.isInteger(count) && count > 0 ? count : null; +} + +const WINDOWS: Record< + 'today' | 'yesterday' | 'previous_x_days' | 'previous_x_days_to_date', + Window +> = { + today: now => [now.startOf('day'), now.plus({ days: 1 }).startOf('day')], + yesterday: now => [now.minus({ days: 1 }).startOf('day'), now.startOf('day')], + previous_x_days: (now, value) => { + const count = days(value); + + return count === null ? null : [now.minus({ days: count }).startOf('day'), now.startOf('day')]; + }, + previous_x_days_to_date: (now, value) => { + const count = days(value); + + return count === null ? null : [now.minus({ days: count }).startOf('day'), now]; + }, +}; + +function within(name: keyof typeof WINDOWS) { + return (actual: unknown, expected: unknown, clock: Clock): boolean => { + const instant = toInstant(actual, clock.timezone); + if (instant === null) return false; + + const window = WINDOWS[name](DateTime.fromJSDate(clock.now).setZone(clock.timezone), expected); + if (window === null) return false; + + const [start, end] = window; + + return instant >= start && instant < end; + }; +} + +function relativeTo( + bound: (now: DateTime, value: unknown) => DateTime | null, + satisfies: (actual: DateTime, bound: DateTime) => boolean, +) { + return (actual: unknown, expected: unknown, clock: Clock): boolean => { + const instant = toInstant(actual, clock.timezone); + if (instant === null) return false; + + const reference = bound(DateTime.fromJSDate(clock.now).setZone(clock.timezone), expected); + + return reference !== null && satisfies(instant, reference); + }; +} + +function hoursAgo(now: DateTime, value: unknown): DateTime | null { + const count = toNumber(value); + + return count !== null && count >= 0 ? now.minus({ hours: count }) : null; +} + const EVALUATORS: Record< Exclude, - (actual: unknown, expected: unknown) => boolean + (actual: unknown, expected: unknown, clock: Clock) => 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) => memberOf(expected, actual) === true, - not_in: (actual, expected) => memberOf(expected, actual) === false, - 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), + in: (actual, expected) => isMemberOf(expected, actual), + includes_all: includesAll, + contains: stringTest((actual, expected) => actual.includes(expected)), + not_contains: stringTest((actual, expected) => !actual.includes(expected)), + starts_with: stringTest((actual, expected) => actual.startsWith(expected)), + ends_with: stringTest((actual, expected) => actual.endsWith(expected)), + // Case folded, accents kept: what Postgres ILIKE does ('É' ILIKE '%é%' holds, 'é' ILIKE '%e%' + // does not), which is the behaviour the list filter shows on the reference database. + i_contains: stringTest((actual, expected) => + actual.toLowerCase().includes(expected.toLowerCase()), + ), + before: dateOrdering((actual, expected) => actual < expected), + after: dateOrdering((actual, expected) => actual > expected), + past: relativeTo( + now => now, + (actual, bound) => actual < bound, + ), + future: relativeTo( + now => now, + (actual, bound) => actual > bound, + ), + before_x_hours_ago: relativeTo(hoursAgo, (actual, bound) => actual < bound), + after_x_hours_ago: relativeTo(hoursAgo, (actual, bound) => actual > bound), + today: within('today'), + yesterday: within('yesterday'), + previous_x_days: within('previous_x_days'), + previous_x_days_to_date: within('previous_x_days_to_date'), }; /** @@ -162,15 +289,17 @@ const EVALUATORS: Record< * - `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. + * Relative date operators read the injected clock, never the machine's. */ export default function evaluateOperator( operator: ConditionOperator, actual: unknown, expected: unknown, + clock: Clock, ): 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); + return EVALUATORS[operator](actual, expected, clock); } diff --git a/packages/workflow-executor/src/executors/step-executor-factory.ts b/packages/workflow-executor/src/executors/step-executor-factory.ts index 65e09ca151..30bd70cc0f 100644 --- a/packages/workflow-executor/src/executors/step-executor-factory.ts +++ b/packages/workflow-executor/src/executors/step-executor-factory.ts @@ -180,6 +180,7 @@ export default class StepExecutorFactory { stepDefinition: step.stepDefinition, previousSteps: step.previousSteps, user: step.user, + timezone: step.timezone, model: cfg.aiModelPort.getModel({ aiConfigName: step.stepDefinition.aiConfigName, userId: step.user.id, diff --git a/packages/workflow-executor/src/types/execution-context.ts b/packages/workflow-executor/src/types/execution-context.ts index 70d965a09f..77e800f173 100644 --- a/packages/workflow-executor/src/types/execution-context.ts +++ b/packages/workflow-executor/src/types/execution-context.ts @@ -33,6 +33,7 @@ export interface ExecutionContext readonly activityLog: ActivityLog; readonly runStore: RunStore; readonly user: StepUser; + readonly timezone: string; readonly schemaResolver: SchemaResolver; readonly previousSteps: ReadonlyArray>; readonly logger: Logger; diff --git a/packages/workflow-executor/src/types/step-execution-data.ts b/packages/workflow-executor/src/types/step-execution-data.ts index 15792ead6f..20b6b8daf7 100644 --- a/packages/workflow-executor/src/types/step-execution-data.ts +++ b/packages/workflow-executor/src/types/step-execution-data.ts @@ -48,6 +48,13 @@ export interface DeterministicConditionExecutionParams { evaluations: ConditionEvaluation[]; selectedOption: string; usedFallback: boolean; + /** + * The instant and zone the conditions were read against. A relative date makes the routing + * depend on the clock, so without these a check on "previous 7 days" cannot be explained a day + * later. + */ + evaluatedAt: string; + timezone: string; } export interface ConditionStepExecutionData extends BaseStepExecutionData { diff --git a/packages/workflow-executor/src/types/validated/execution.ts b/packages/workflow-executor/src/types/validated/execution.ts index 1b80e70963..a6807f817d 100644 --- a/packages/workflow-executor/src/types/validated/execution.ts +++ b/packages/workflow-executor/src/types/validated/execution.ts @@ -49,6 +49,8 @@ export const AvailableStepExecutionSchema = z stepDefinition: StepDefinitionSchema, previousSteps: z.array(StepSchema), user: StepUserSchema, + /** IANA zone the run's relative dates are read in: the project's, never the machine's. */ + timezone: z.string().min(1), }) .strict(); export type AvailableStepExecution = z.infer; diff --git a/packages/workflow-executor/src/types/validated/step-definition.ts b/packages/workflow-executor/src/types/validated/step-definition.ts index 0a3f2cc284..d82ed84132 100644 --- a/packages/workflow-executor/src/types/validated/step-definition.ts +++ b/packages/workflow-executor/src/types/validated/step-definition.ts @@ -42,6 +42,8 @@ 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. +// The set a list view filter offers, no more: every name here has a Filters operator in the front +// (which is what the builder can render) and a translation in the orchestrator's parser. export const CONDITION_OPERATORS = [ 'equal', 'not_equal', @@ -49,16 +51,34 @@ export const CONDITION_OPERATORS = [ 'blank', 'greater_than', 'less_than', - 'greater_than_or_equal', - 'less_than_or_equal', 'in', - 'not_in', + 'includes_all', 'contains', 'not_contains', + 'starts_with', + 'ends_with', + 'i_contains', + 'before', + 'after', + 'past', + 'future', + 'today', + 'yesterday', + 'previous_x_days', + 'previous_x_days_to_date', + 'before_x_hours_ago', + 'after_x_hours_ago', ] as const; export type ConditionOperator = (typeof CONDITION_OPERATORS)[number]; -const VALUE_LESS_OPERATORS: readonly ConditionOperator[] = ['present', 'blank']; +export const VALUE_LESS_OPERATORS: readonly ConditionOperator[] = [ + 'present', + 'blank', + 'past', + 'future', + 'today', + 'yesterday', +]; const DeterministicConditionSchema = z .object({ @@ -66,7 +86,7 @@ const DeterministicConditionSchema = z sourceStepId: z.string().min(1), fieldName: z.string().min(1), operator: z.enum(CONDITION_OPERATORS), - /** Absent for `present`/`blank`. */ + /** Absent for the value-less operators (present, blank, and the relative dates without a count). */ value: z.unknown().optional(), }) // A value-bearing operator without its value compares against `undefined`: it can never be met, diff --git a/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts b/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts index 12b3127618..04c2734786 100644 --- a/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts +++ b/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts @@ -126,9 +126,24 @@ describe('toAvailableStepExecution', () => { }, previousSteps: [], user: expect.objectContaining({ id: 7, email: 'alban@forestadmin.com' }), + timezone: 'UTC', }); }); + it('should forward the project timezone', () => { + const result = toAvailableStepExecution(makeRun({ timezone: 'Europe/Paris' })); + + expect(result?.timezone).toBe('Europe/Paris'); + }); + + // A relative date must resolve the same on every executor instance, so the machine's zone is + // never what an unset or absent value falls back to. + it.each([null, undefined, ''])('should fall back to UTC when the timezone is %p', timezone => { + const result = toAvailableStepExecution(makeRun({ timezone })); + + expect(result?.timezone).toBe('UTC'); + }); + it('should forward the run triggerType', () => { const run = makeRun({ triggerType: ServerWorkflowTriggerType.webhook }); diff --git a/packages/workflow-executor/test/executors/base-step-executor.test.ts b/packages/workflow-executor/test/executors/base-step-executor.test.ts index 1d4db6c2f5..dad5393a96 100644 --- a/packages/workflow-executor/test/executors/base-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/base-step-executor.test.ts @@ -168,6 +168,7 @@ function makeContext( }, schemaResolver: new SchemaResolver(schemaCache, workflowPort, runId, 1), previousSteps: [], + timezone: 'UTC', logger: makeMockLogger(), ...overrides, }; 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 10753c3bf0..0252cb2a7a 100644 --- a/packages/workflow-executor/test/executors/condition-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/condition-step-executor.test.ts @@ -122,6 +122,7 @@ function makeContext( permissionLevel: 'admin', tags: {}, }, + timezone: 'UTC', schemaResolver: new SchemaResolver(schemaCache, workflowPort, runId, 1), previousSteps: [], logger: jest.fn(), @@ -476,8 +477,8 @@ describe('ConditionStepExecutor', () => { { sourceStepId: 'get-1', fieldName: 'amount', - operator: 'less_than_or_equal', - value: 100, + operator: 'less_than', + value: 101, }, ], }, @@ -576,6 +577,8 @@ describe('ConditionStepExecutor', () => { ], selectedOption: 'High', usedFallback: false, + evaluatedAt: expect.any(String), + timezone: 'UTC', }, executionResult: { answer: 'High' }, }); @@ -655,6 +658,8 @@ describe('ConditionStepExecutor', () => { ], selectedOption: 'Other', usedFallback: true, + evaluatedAt: expect.any(String), + timezone: 'UTC', }, executionResult: { answer: 'Other' }, }); @@ -724,6 +729,8 @@ describe('ConditionStepExecutor', () => { ], selectedOption: 'Other', usedFallback: true, + evaluatedAt: expect.any(String), + timezone: 'UTC', }, executionResult: { answer: 'Other' }, }, @@ -774,6 +781,8 @@ describe('ConditionStepExecutor', () => { ], selectedOption: 'Other', usedFallback: true, + evaluatedAt: expect.any(String), + timezone: 'UTC', }, executionResult: { answer: 'Other' }, }); @@ -947,6 +956,8 @@ describe('ConditionStepExecutor', () => { ], selectedOption: 'Other', usedFallback: true, + evaluatedAt: expect.any(String), + timezone: 'UTC', }, executionResult: { answer: 'Other' }, }); @@ -1127,6 +1138,70 @@ describe('ConditionStepExecutor', () => { ); }); + // 2026-09-04T23:00Z is still 4 September in UTC but already 5 September in Paris. A record + // stamped 5 September 00:30 Paris is "today" only if the project zone, not UTC nor the + // machine's, is what the evaluator was handed. + it('reads relative dates in the context timezone and records the instant it used', async () => { + jest.useFakeTimers().setSystemTime(new Date('2026-09-04T23:00:00Z')); + const todayInParis: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'Today', + aggregator: 'and', + conditions: [{ sourceStepId: 'get-1', fieldName: 'signedAt', operator: 'today' }], + }, + ], + fallbackOption: 'Other', + }; + const { context, runStore } = makeDeterministicContext( + todayInParis, + [{ name: 'signedAt', displayName: 'Signed at', value: '2026-09-04T22:30:00Z' }], + { timezone: 'Europe/Paris' }, + ); + + try { + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Today'); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: expect.objectContaining({ + evaluatedAt: '2026-09-04T23:00:00.000Z', + timezone: 'Europe/Paris', + }), + }), + ); + } finally { + jest.useRealTimers(); + } + }); + + it('does not read a record from the next UTC day as today when the project zone is UTC', async () => { + jest.useFakeTimers().setSystemTime(new Date('2026-09-04T23:00:00Z')); + const todayArgs: ConditionPreRecordedArgs = { + optionConditions: [ + { + option: 'Today', + aggregator: 'and', + conditions: [{ sourceStepId: 'get-1', fieldName: 'signedAt', operator: 'today' }], + }, + ], + fallbackOption: 'Other', + }; + const { context } = makeDeterministicContext(todayArgs, [ + { name: 'signedAt', displayName: 'Signed at', value: '2026-09-05T00:30:00Z' }, + ]); + + try { + const result = await new ConditionStepExecutor(context).execute(); + + expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Other'); + } finally { + jest.useRealTimers(); + } + }); + 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 1126907c04..a61236229f 100644 --- a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -1,5 +1,16 @@ +import type { Clock } from '../../src/executors/deterministic-condition-evaluator'; +import type { ConditionOperator } from '../../src/types/validated/step-definition'; + import evaluateOperator from '../../src/executors/deterministic-condition-evaluator'; +// A fixed instant in a non-UTC zone: 2026-09-04 12:30 in Paris (UTC+2 in September), so any +// operator that silently read the machine's clock or UTC's day would give itself away. +const CLOCK: Clock = { now: new Date('2026-09-04T10:30:00Z'), timezone: 'Europe/Paris' }; + +function ev(operator: ConditionOperator, actual: unknown, expected?: unknown, clock = CLOCK) { + return evaluateOperator(operator, actual, expected, clock); +} + describe('evaluateOperator', () => { describe('null / missing actual value (never an error)', () => { it.each([ @@ -7,188 +18,181 @@ describe('evaluateOperator', () => { 'not_equal', 'greater_than', 'less_than', - 'greater_than_or_equal', - 'less_than_or_equal', 'in', - 'not_in', + 'includes_all', 'contains', 'not_contains', + 'starts_with', + 'ends_with', + 'i_contains', + 'before', + 'after', + 'past', + 'future', + 'today', + 'yesterday', + 'previous_x_days', + 'previous_x_days_to_date', + 'before_x_hours_ago', + 'after_x_hours_ago', ] 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(); + expect(ev(operator, null, 'anything')).toBeNull(); + expect(ev(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); + expect(ev('equal', 'active', 'active')).toBe(true); + expect(ev('equal', 5, 5)).toBe(true); + expect(ev('equal', false, false)).toBe(true); }); it('rejects different scalars', () => { - expect(evaluateOperator('equal', 'active', 'inactive')).toBe(false); + expect(ev('equal', 'active', 'inactive')).toBe(false); }); it('rejects a type mismatch', () => { - expect(evaluateOperator('equal', true, 'true')).toBe(false); - expect(evaluateOperator('equal', 'abc', 100)).toBe(false); + expect(ev('equal', true, 'true')).toBe(false); + expect(ev('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); + expect(ev('equal', '2026-01-01T00:00:00Z', '2026-01-01T00:00:00.000Z')).toBe(true); + expect(ev('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); + expect(ev('equal', [1, 2], [1, 2])).toBe(true); + expect(ev('equal', [1, 2], [2, 1])).toBe(false); + expect(ev('equal', [1, 2], [1, 2, 3])).toBe(false); }); it('rejects an array compared to a scalar', () => { - expect(evaluateOperator('equal', [1], 1)).toBe(false); + expect(ev('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); + expect(ev('not_equal', 'active', 'inactive')).toBe(true); + expect(ev('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); + expect(ev('not_equal', 'active', 'active')).toBe(false); + expect(ev('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); + expect(ev('not_equal', true, 'true')).toBe(false); + expect(ev('not_equal', 5, 'abc')).toBe(false); + expect(ev('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); + expect(ev('greater_than', '150.00', 100)).toBe(true); + expect(ev('greater_than', '50.00', 100)).toBe(false); + expect(ev('less_than', 100, '150.00')).toBe(true); + expect(ev('greater_than', '100', 99)).toBe(true); + expect(ev('less_than', '-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); + expect(ev('equal', '42', 42)).toBe(true); + expect(ev('equal', 42, '42.0')).toBe(true); + expect(ev('not_equal', '42', 42)).toBe(false); + expect(ev('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); + expect(ev('greater_than', 'abc', 100)).toBe(false); + expect(ev('greater_than', '12abc', 100)).toBe(false); + expect(ev('equal', '', 0)).toBe(false); }); it('does not coerce when neither side is a number', () => { - expect(evaluateOperator('greater_than', '5', '3')).toBe(false); + expect(ev('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); + expect(ev('present', 'a', undefined)).toBe(true); + expect(ev('present', 0, undefined)).toBe(true); + expect(ev('present', false, undefined)).toBe(true); + expect(ev('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); + expect(ev('present', null, undefined)).toBe(false); + expect(ev('present', undefined, undefined)).toBe(false); + expect(ev('present', '', undefined)).toBe(false); + expect(ev('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); + expect(ev('blank', null, undefined)).toBe(true); + expect(ev('blank', undefined, undefined)).toBe(true); + expect(ev('blank', '', undefined)).toBe(true); + expect(ev('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); + expect(ev('blank', 'a', undefined)).toBe(false); + expect(ev('blank', 0, undefined)).toBe(false); + expect(ev('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); + expect(ev('greater_than', 5, 3)).toBe(true); + expect(ev('greater_than', 3, 5)).toBe(false); + expect(ev('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); + expect(ev('less_than', 3, 5)).toBe(true); + expect(ev('less_than', 5, 3)).toBe(false); + expect(ev('less_than', 5, 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); + expect(ev('greater_than', 'abc', 'abd')).toBe(false); + expect(ev('greater_than', true, 3)).toBe(false); + expect(ev('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); + expect(ev('greater_than', '2026-02-01', '2026-01-01')).toBe(true); + expect(ev('less_than', '2026-01-01T10:00:00Z', '2026-01-01T12:00:00Z')).toBe(true); + expect(ev('greater_than', '2026-01-02T00:00:00Z', '2026-01-01')).toBe(true); + expect(ev('less_than', '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); + expect(ev('greater_than', '2026-02-01', 'not a date')).toBe(false); + expect(ev('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); + expect(ev('equal', '2026-03-02', '2026-02-30')).toBe(false); + expect(ev('equal', '2026-05-01', '2026-04-31')).toBe(false); + expect(ev('equal', '2025-03-01', '2025-02-29')).toBe(false); + expect(ev('greater_than', '2026-02-30', '2026-01-01')).toBe(false); + expect(ev('less_than', '2026-01-01', '2026-02-30')).toBe(false); + expect(ev('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); + expect(ev('equal', '2024-02-29', '2024-02-29T00:00:00.000Z')).toBe(true); }); describe('on a host whose timezone is not UTC', () => { @@ -203,13 +207,9 @@ describe('evaluateOperator', () => { }); 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); + expect(ev('equal', '2026-01-01T10:00:00', '2026-01-01T10:00:00Z')).toBe(true); + expect(ev('greater_than', '2026-01-01T12:00:00', '2026-01-01T11:00:00Z')).toBe(true); + expect(ev('less_than', '2026-01-01T10:00:00', '2026-01-01T11:00:00+00:00')).toBe(true); }); }); }); @@ -222,106 +222,205 @@ describe('evaluateOperator', () => { const twoPow53 = 9007199254740992; it('does not read a bigint string as equal to the number it rounds to', () => { - expect(evaluateOperator('equal', justAbove, twoPow53)).toBe(false); - expect(evaluateOperator('not_equal', justAbove, twoPow53)).toBe(true); + expect(ev('equal', justAbove, twoPow53)).toBe(false); + expect(ev('not_equal', justAbove, twoPow53)).toBe(true); }); it('orders a bigint string against a threshold it exceeds by one', () => { - expect(evaluateOperator('greater_than', justAbove, twoPow53)).toBe(true); - expect(evaluateOperator('less_than', justAbove, twoPow53)).toBe(false); - expect(evaluateOperator('greater_than_or_equal', justAbove, twoPow53)).toBe(true); + expect(ev('greater_than', justAbove, twoPow53)).toBe(true); + expect(ev('less_than', justAbove, twoPow53)).toBe(false); }); it('excludes it from a list it rounds into', () => { - expect(evaluateOperator('in', justAbove, [twoPow53])).toBe(false); - expect(evaluateOperator('not_in', justAbove, [twoPow53])).toBe(true); + expect(ev('in', justAbove, [twoPow53])).toBe(false); }); // Decimals have no BigInt to be read as, so they keep the Number path. it('leaves decimals on the number path', () => { - expect(evaluateOperator('equal', '150.00', 150)).toBe(true); - expect(evaluateOperator('greater_than', '150.50', 150)).toBe(true); - expect(evaluateOperator('equal', 1.5, 1.5)).toBe(true); + expect(ev('equal', '150.00', 150)).toBe(true); + expect(ev('greater_than', '150.50', 150)).toBe(true); + expect(ev('equal', 1.5, 1.5)).toBe(true); }); it('still compares ordinary whole numbers', () => { - expect(evaluateOperator('equal', '150', 150)).toBe(true); - expect(evaluateOperator('greater_than', 150, 100)).toBe(true); - expect(evaluateOperator('less_than', '-20', 0)).toBe(true); + expect(ev('equal', '150', 150)).toBe(true); + expect(ev('greater_than', 150, 100)).toBe(true); + expect(ev('less_than', '-20', 0)).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, - ); + expect(ev('in', 'b', ['a', 'b'])).toBe(true); + expect(ev('in', 2, [1, 2, 3])).toBe(true); + expect(ev('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); + expect(ev('in', 'c', ['a', 'b'])).toBe(false); + expect(ev('in', 2, ['3'])).toBe(false); }); it('is not met when the expected value is not an array', () => { - expect(evaluateOperator('in', 'a', 'a')).toBe(false); + expect(ev('in', 'a', 'a')).toBe(false); }); it('is not met when no member of the list is comparable to the value', () => { - expect(evaluateOperator('in', true, ['true'])).toBe(false); + expect(ev('in', true, ['true'])).toBe(false); + }); + }); + + describe('contains', () => { + it('matches a substring on strings', () => { + expect(ev('contains', 'hello world', 'world')).toBe(true); + expect(ev('contains', 'hello', 'world')).toBe(false); + }); + + it('is not met on anything but two strings (contract: String fields only)', () => { + expect(ev('contains', ['a', 'b'], 'b')).toBe(false); + expect(ev('contains', 5, '5')).toBe(false); + expect(ev('contains', 'abc', 5)).toBe(false); }); }); - describe('not_in', () => { - it('matches when the value is absent from the list', () => { - expect(evaluateOperator('not_in', 'c', ['a', 'b'])).toBe(true); + describe('not_contains', () => { + it('matches when the substring is absent', () => { + expect(ev('not_contains', 'hello', 'world')).toBe(true); + expect(ev('not_contains', 'hello world', 'world')).toBe(false); }); - 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) on anything but two strings', () => { + expect(ev('not_contains', ['a'], 'b')).toBe(false); + expect(ev('not_contains', 5, '5')).toBe(false); + expect(ev('not_contains', 'abc', 5)).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('string operators', () => { + it('starts_with and ends_with are case sensitive', () => { + expect(ev('starts_with', 'active', 'act')).toBe(true); + expect(ev('starts_with', 'active', 'Act')).toBe(false); + expect(ev('ends_with', 'active', 'ive')).toBe(true); + expect(ev('ends_with', 'active', 'IVE')).toBe(false); }); - // The negated operators are the ones a type mismatch could accidentally satisfy: nothing in the - // list is comparable to the value, so "absent from the list" is a claim we cannot make. - it('is not met when no member of the list is comparable to the value', () => { - expect(evaluateOperator('not_in', true, ['true'])).toBe(false); - expect(evaluateOperator('not_in', 'closed', ['active', 5])).toBe(false); + // What Postgres ILIKE does, measured on an en_US.utf8 database: 'É' ILIKE '%é%' holds, + // 'é' ILIKE '%e%' does not. Case is folded, accents are letters of their own. + it('i_contains folds case but keeps accents, like Postgres ILIKE', () => { + expect(ev('i_contains', 'ACTIVE', 'act')).toBe(true); + expect(ev('i_contains', 'É', 'é')).toBe(true); + expect(ev('i_contains', 'é', 'e')).toBe(false); }); - it('still matches against an empty list, which mismatches nothing', () => { - expect(evaluateOperator('not_in', 'c', [])).toBe(true); + it('never coerces a non-string into text', () => { + expect(ev('starts_with', 150, '15')).toBe(false); + expect(ev('i_contains', 'abc', 1)).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); + describe('includes_all', () => { + it('matches when every wanted value is a member', () => { + expect(ev('includes_all', ['a', 'b', 'c'], ['a', 'c'])).toBe(true); + expect(ev('includes_all', ['a', 'b'], 'a')).toBe(true); }); - 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); + it('rejects when one wanted value is missing, or the actual is not a list', () => { + expect(ev('includes_all', ['a'], ['a', 'b'])).toBe(false); + expect(ev('includes_all', 'a', ['a'])).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); + describe('before / after', () => { + it('orders two instants strictly', () => { + expect(ev('before', '2026-01-01T00:00:00Z', '2026-06-01T00:00:00Z')).toBe(true); + expect(ev('after', '2026-01-01T00:00:00Z', '2026-06-01T00:00:00Z')).toBe(false); + expect(ev('before', '2026-06-01T00:00:00Z', '2026-06-01T00:00:00Z')).toBe(false); + expect(ev('after', '2026-06-01T00:00:00Z', '2026-06-01T00:00:00Z')).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); + it('orders two calendar dates', () => { + expect(ev('before', '2026-03-01', '2026-03-02')).toBe(true); + expect(ev('after', '2026-03-01', '2026-03-02')).toBe(false); + }); + + it('is not met on a non-date or an impossible date', () => { + expect(ev('before', 'soon', '2026-06-01T00:00:00Z')).toBe(false); + expect(ev('before', '2026-02-30', '2026-06-01T00:00:00Z')).toBe(false); + }); + }); + + describe('past / future (relative to the clock)', () => { + it('compares against the injected instant, not the machine clock', () => { + expect(ev('past', '2026-09-04T10:00:00Z')).toBe(true); + expect(ev('past', '2026-09-04T11:00:00Z')).toBe(false); + expect(ev('future', '2026-09-04T11:00:00Z')).toBe(true); + expect(ev('future', '2026-09-04T10:00:00Z')).toBe(false); + }); + + it('is neither past nor future at the exact instant', () => { + expect(ev('past', '2026-09-04T10:30:00Z')).toBe(false); + expect(ev('future', '2026-09-04T10:30:00Z')).toBe(false); + }); + }); + + describe('before_x_hours_ago / after_x_hours_ago', () => { + it('measures from the injected instant', () => { + expect(ev('before_x_hours_ago', '2026-09-04T08:00:00Z', 2)).toBe(true); + expect(ev('before_x_hours_ago', '2026-09-04T09:00:00Z', 2)).toBe(false); + expect(ev('after_x_hours_ago', '2026-09-04T09:00:00Z', 2)).toBe(true); + expect(ev('after_x_hours_ago', '2026-09-04T08:00:00Z', 2)).toBe(false); + }); + + it('is not met on a count that is not a non-negative number', () => { + expect(ev('before_x_hours_ago', '2026-09-04T08:00:00Z', -1)).toBe(false); + expect(ev('before_x_hours_ago', '2026-09-04T08:00:00Z', 'two')).toBe(false); + }); + }); + + describe('day windows (today, yesterday, previous_x_days)', () => { + // Paris is UTC+2 here: its 4 September runs from 2026-09-03T22:00Z to 2026-09-04T22:00Z. + it('reads "today" in the clock timezone, not in UTC', () => { + expect(ev('today', '2026-09-03T23:00:00Z')).toBe(true); + expect(ev('today', '2026-09-03T21:00:00Z')).toBe(false); + expect(ev('today', '2026-09-04T21:59:59Z')).toBe(true); + expect(ev('today', '2026-09-04T22:00:00Z')).toBe(false); + }); + + it('reads "yesterday" as the previous project day', () => { + expect(ev('yesterday', '2026-09-03T21:00:00Z')).toBe(true); + expect(ev('yesterday', '2026-09-03T23:00:00Z')).toBe(false); + }); + + it('previous_x_days excludes today, previous_x_days_to_date includes it up to the instant', () => { + expect(ev('previous_x_days', '2026-09-01T12:00:00Z', 7)).toBe(true); + expect(ev('previous_x_days', '2026-09-04T09:00:00Z', 7)).toBe(false); + expect(ev('previous_x_days', '2026-08-27T21:00:00Z', 7)).toBe(false); + expect(ev('previous_x_days_to_date', '2026-09-04T09:00:00Z', 7)).toBe(true); + expect(ev('previous_x_days_to_date', '2026-09-04T11:00:00Z', 7)).toBe(false); + }); + + it('is not met on a count that is not a positive whole number', () => { + expect(ev('previous_x_days', '2026-09-01T12:00:00Z', 0)).toBe(false); + expect(ev('previous_x_days', '2026-09-01T12:00:00Z', 1.5)).toBe(false); + expect(ev('previous_x_days', '2026-09-01T12:00:00Z', 'seven')).toBe(false); + }); + + // A calendar date has no instant: read as UTC midnight it would fall before Honolulu's day + // even started, and the record's own "today" would be counted as yesterday. + it('reads a calendar date in the clock timezone', () => { + const honolulu: Clock = { + now: new Date('2026-09-04T05:00:00Z'), + timezone: 'Pacific/Honolulu', + }; + + expect(ev('today', '2026-09-03', undefined, honolulu)).toBe(true); + expect(ev('today', '2026-09-04', undefined, honolulu)).toBe(false); + expect(ev('yesterday', '2026-09-02', undefined, honolulu)).toBe(true); + }); + + it('is not met on a value that is not a date', () => { + expect(ev('today', 'now')).toBe(false); + expect(ev('today', 150)).toBe(false); }); }); }); diff --git a/packages/workflow-executor/test/executors/guidance-step-executor.test.ts b/packages/workflow-executor/test/executors/guidance-step-executor.test.ts index 3020f342ca..caea079ee6 100644 --- a/packages/workflow-executor/test/executors/guidance-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/guidance-step-executor.test.ts @@ -80,6 +80,7 @@ function makeContext( }, schemaResolver: new SchemaResolver(schemaCache, workflowPort, runId, 1), previousSteps: [], + timezone: 'UTC', logger: jest.fn(), ...overrides, }; diff --git a/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts b/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts index b4ff5d5daf..fac73207de 100644 --- a/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts @@ -211,6 +211,7 @@ function makeContext( }, schemaResolver: new SchemaResolver(schemaCache, workflowPort, runId, 1), previousSteps: [], + timezone: 'UTC', logger: jest.fn(), ...overrides, diff --git a/packages/workflow-executor/test/executors/mcp-step-executor.test.ts b/packages/workflow-executor/test/executors/mcp-step-executor.test.ts index cc03afe1ff..5bc850f39c 100644 --- a/packages/workflow-executor/test/executors/mcp-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/mcp-step-executor.test.ts @@ -126,6 +126,7 @@ function makeContext( }, schemaResolver: new SchemaResolver(schemaCache, workflowPort, runId, 1), previousSteps: [], + timezone: 'UTC', logger: jest.fn(), ...overrides, }; diff --git a/packages/workflow-executor/test/executors/read-record-step-executor.test.ts b/packages/workflow-executor/test/executors/read-record-step-executor.test.ts index a117e497f2..1e39c6d25c 100644 --- a/packages/workflow-executor/test/executors/read-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/read-record-step-executor.test.ts @@ -160,6 +160,7 @@ function makeContext( }, schemaResolver, previousSteps: [], + timezone: 'UTC', logger: jest.fn(), ...overrides, }; diff --git a/packages/workflow-executor/test/executors/step-executor-factory.test.ts b/packages/workflow-executor/test/executors/step-executor-factory.test.ts index 88e2019957..6684ef10f7 100644 --- a/packages/workflow-executor/test/executors/step-executor-factory.test.ts +++ b/packages/workflow-executor/test/executors/step-executor-factory.test.ts @@ -37,6 +37,7 @@ function makeStep(): AvailableStepExecution { permissionLevel: 'admin', tags: {}, }, + timezone: 'UTC', } as unknown as AvailableStepExecution; } diff --git a/packages/workflow-executor/test/executors/trigger-record-action-step-executor.test.ts b/packages/workflow-executor/test/executors/trigger-record-action-step-executor.test.ts index e8a71cbdab..25d8a60e83 100644 --- a/packages/workflow-executor/test/executors/trigger-record-action-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/trigger-record-action-step-executor.test.ts @@ -161,6 +161,7 @@ function makeContext( }, schemaResolver: new SchemaResolver(schemaCache, workflowPort, runId, 1), previousSteps: [], + timezone: 'UTC', logger: jest.fn(), ...overrides, }; diff --git a/packages/workflow-executor/test/executors/update-record-step-executor.test.ts b/packages/workflow-executor/test/executors/update-record-step-executor.test.ts index 4dec261274..e89a67f31c 100644 --- a/packages/workflow-executor/test/executors/update-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/update-record-step-executor.test.ts @@ -154,6 +154,7 @@ function makeContext( }, schemaResolver: new SchemaResolver(schemaCache, workflowPort, runId, 1), previousSteps: [], + timezone: 'UTC', logger: jest.fn(), ...overrides, }; diff --git a/packages/workflow-executor/test/integration/workflow-execution.test.ts b/packages/workflow-executor/test/integration/workflow-execution.test.ts index d40b5af2ae..c4f45fcb59 100644 --- a/packages/workflow-executor/test/integration/workflow-execution.test.ts +++ b/packages/workflow-executor/test/integration/workflow-execution.test.ts @@ -254,6 +254,7 @@ function buildPendingStep( runId: 'run-1', stepId: 'step-1', stepIndex: 0, + timezone: 'UTC', collectionId: 'col-1', triggerType: TriggerType.Manual, baseRecordRef: BASE_RECORD_REF, diff --git a/packages/workflow-executor/test/runner.test.ts b/packages/workflow-executor/test/runner.test.ts index dbcc8913dd..ecee094cc2 100644 --- a/packages/workflow-executor/test/runner.test.ts +++ b/packages/workflow-executor/test/runner.test.ts @@ -168,6 +168,7 @@ function makePendingStep( runId: 'run-1', stepId: 'step-1', stepIndex: 0, + timezone: 'UTC', collectionId: 'col-1', triggerType: TriggerType.Manual, baseRecordRef: { collectionName: 'customers', recordId: ['1'], stepIndex: 0 }, From 41e85f4acde45d8e0b6667ab8dd1aab5ca1bf5ca Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Fri, 4 Sep 2026 21:35:37 +0200 Subject: [PATCH 02/10] fix(workflow-executor): align day window bounds with the toolkit, validate the project zone The start of a day window is included for a calendar date and excluded for a datetime, as datasource-toolkit's time transforms do. An unknown IANA zone falls back to UTC. includes_all is never met on an empty list. The SQL datetime form with a space is read as UTC too. Co-Authored-By: Claude Fable 5.1 --- packages/workflow-executor/CLAUDE.md | 2 +- packages/workflow-executor/package.json | 2 +- .../adapters/run-to-available-step-mapper.ts | 10 +++-- .../deterministic-condition-evaluator.ts | 19 +++++---- .../src/types/validated/step-definition.ts | 2 +- .../run-to-available-step-mapper.test.ts | 11 +++-- .../executors/condition-step-executor.test.ts | 4 +- .../deterministic-condition-evaluator.test.ts | 42 +++++++++++++++++++ 8 files changed, 72 insertions(+), 20 deletions(-) diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index 172609d52b..ec2fa9dfbb 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -37,7 +37,7 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p - **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`. **No condition evaluation ever fails the step, data or reference alike**: a resolved value that is null is `met: null`, and a reference that read nothing is `met: null` + a `reason` (`source-step-not-reached` when no Get Data with that id ran on this path, `field-not-loaded` when one ran but the field is absent or errored), plus a `Warn`. Build-time validation cannot cover the second case (a Get Data step may let the AI pick its fields), so the run has to answer at runtime — and the way a decision step answers is by routing, unlike record steps (line above) which die on an unresolvable reference. What stops the fallback from passing for a decision the data took is the `reason`: the run view words it for the operator instead of showing a bare cross. "Never read" is not "read and empty", so `present`/`blank` get no answer out of it either. 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`. - **Operator set = what a list view filter offers** (PRD-1147), 23 names in `CONDITION_OPERATORS`; nothing the front registry cannot render (so no `>=`, `<=`, `not_in`). 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`), whole numbers exactly as BigInt; 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); text operators are strings-only, case sensitive except `i_contains`, which folds case and keeps accents like Postgres `ILIKE`. - - **Relative dates read an injected `Clock`** (`{ now, timezone }`), never the machine's: the step executor builds one per step (so every row of a Decision sees the same instant) from `ExecutionContext.timezone`, which the orchestrator sends as the **project's** zone (`AvailableStepExecution.timezone`, mapper falls back to `UTC` when unset or absent). The machine's zone is never used: the fleet runs several instances, and the same run must route the same on each. Day windows (`today`, `yesterday`, `previous_x_days`, `previous_x_days_to_date`) are half open `[start, end)` in that zone, mirroring `datasource-toolkit`'s time transforms; a **calendar date** (Dateonly) is read at midnight in that zone, otherwise Honolulu's own "today" would count as yesterday. The trace persists `evaluatedAt` + `timezone` alongside `evaluations`, since a relative condition cannot be explained later without the instant it was read at. A retry re-evaluates with a fresh clock and may route differently: accepted, by product decision. + - **Relative dates read an injected `Clock`** (`{ now, timezone }`), never the machine's: the step executor builds one per step (so every row of a Decision sees the same instant) from `ExecutionContext.timezone`, which the orchestrator sends as the **project's** zone (`AvailableStepExecution.timezone`, mapper falls back to `UTC` when unset or absent). The machine's zone is never used: the fleet runs several instances, and the same run must route the same on each. Day windows (`today`, `yesterday`, `previous_x_days`, `previous_x_days_to_date`) are computed in that zone with the exact bounds of `datasource-toolkit`'s time transforms: end excluded, start included for a calendar date and excluded for a datetime (the toolkit emits `GreaterThanOrEqual` for Dateonly, `GreaterThan` otherwise), so a record answers "today" the same way in a Decision and in a list filter; a **calendar date** (Dateonly) is read at midnight in that zone, otherwise Honolulu's own "today" would count as yesterday. An unknown zone name falls back to `UTC` in the mapper, like an absent one. The trace persists `evaluatedAt` + `timezone` alongside `evaluations`, since a relative condition cannot be explained later without the instant it was read at. A retry re-evaluates with a fresh clock and may route differently: accepted, by product decision. - **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. All four record steps pin the source by `selectedRecordStepId` — a **stable BPMN step id** (or the `WORKFLOW_START_STEP_ID` sentinel) resolved by `resolveSourceRecordRef`, chosen to survive the index shifts a revision causes; the editor writes it and treats it as a precondition for choosing fields. Presence, not truthiness, decides whether something is pinned -- `selectedRecordStepId`, `actionName` and `relationName` all check `!== undefined`. An empty value is a pin that lost its target, so it resolves to nothing and errors instead of falling back to the AI, which would silently pick a different record, run a different action, or follow a different relation than the step was configured to. Not reachable from the editor, which writes `stepId || undefined`. `selectedRecordStepIndex` (a runtime index, resolved in `resolveRecordRef`) survives only as a fallback on read-record/update-record, checked after the step id. Partial args supported. Unresolvable → `PinnedArgNotFoundError` when the name was pinned (`configuration`), `FieldNotFoundError`/`ActionNotFoundError`/`RelationNotFoundError` when the AI chose it (**unclassified** — a re-run may resolve differently, so nothing permanent can be asserted); bad shape / out-of-range index → `InvalidPreRecordedArgsError`. The split exists because the AI-facing messages tell the operator to rephrase the prompt — the right remedy for a name the AI chose, and unreachable for one the workflow fixed, where the step itself is the thing to edit. One class covers every pinned kind: the diagnosis differs per kind, the operator's remedy does not. Anything raised on a pinned value has to name the step. diff --git a/packages/workflow-executor/package.json b/packages/workflow-executor/package.json index 4cec88b7b7..ba73856c0b 100644 --- a/packages/workflow-executor/package.json +++ b/packages/workflow-executor/package.json @@ -45,10 +45,10 @@ "zod": "4.3.6" }, "devDependencies": { - "@types/luxon": "^3.2.0", "@types/jsonwebtoken": "^9.0.10", "@types/koa": "^2.13.5", "@types/koa__router": "^12.0.4", + "@types/luxon": "^3.2.0", "@types/sequelize": "^6.12.0", "sqlite3": "^6.0.1", "supertest": "^7.1.3" diff --git a/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts b/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts index cdec80def7..40918aa6d5 100644 --- a/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts +++ b/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts @@ -11,6 +11,7 @@ import type { StepOutcome, } from '../types/validated/step-outcome'; +import { IANAZone } from 'luxon'; import { z } from 'zod'; import { deserializeRecordId } from './record-id-serializer'; @@ -178,10 +179,11 @@ export default function toAvailableStepExecution( stepDefinition: toStepDefinition(pending.stepDefinition), previousSteps: toPreviousSteps(run.workflowHistory, pending.stepIndex), user: toStepUser(run.id, run.userProfile), - // UTC when the project has none set, and when the orchestrator is too old to send one: a - // relative date must resolve the same on every executor instance, so the machine's zone is - // never the fallback. - timezone: run.timezone || 'UTC', + // UTC when the project has none set, when the orchestrator is too old to send one, and when + // the name is not a zone Luxon knows: a relative date must resolve the same on every executor + // instance, so the machine's zone is never the fallback. The zone actually used is persisted + // with the evaluation, so the run view shows UTC rather than the name it fell back from. + timezone: run.timezone && IANAZone.isValidZone(run.timezone) ? run.timezone : 'UTC', }; // Defense against mapper bugs: zod asserts the shape we produce is what the domain expects, diff --git a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts index 02e0d146a4..0de359a572 100644 --- a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts +++ b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts @@ -33,8 +33,11 @@ function toTimestamp(value: unknown): number | 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; + // the same run differently per machine — pin every offset-less datetime to UTC. The SQL form + // with a space separator is a datetime too. + const datetime = value.replace(' ', 'T'); + const absolute = + datetime.includes('T') && !TIMEZONE_SUFFIX.test(datetime) ? `${datetime}Z` : datetime; const parsed = Date.parse(absolute); return Number.isNaN(parsed) ? null : parsed; @@ -150,7 +153,7 @@ function includesAll(actual: unknown, expected: unknown): boolean { if (!Array.isArray(actual)) return false; const wanted = Array.isArray(expected) ? expected : [expected]; - return wanted.every(item => isMemberOf(actual, item)); + return wanted.length > 0 && wanted.every(item => isMemberOf(actual, item)); } // Strings only: no coercion of a number into text, so a broken config can never accidentally @@ -184,9 +187,10 @@ function dateOrdering(satisfies: (actual: DateTime, expected: DateTime) => boole }; } -// Relative to the clock. A window is half open, [start, end): a value at the very start of today -// is today, a value at the very start of tomorrow is not. Mirrors datasource-toolkit's time -// transforms, which is what the list filter runs on, so both agree on what "today" covers. +// Relative to the clock. The end of a window is always excluded; its start is included for a +// calendar date and excluded for a datetime. That asymmetry is datasource-toolkit's (its time +// transforms emit GreaterThanOrEqual for a Dateonly column, GreaterThan otherwise), and the list +// filter runs on it, so the same record answers "today" the same way in both places. type Window = (now: DateTime, value: unknown) => [start: DateTime, end: DateTime] | null; function days(value: unknown): number | null { @@ -222,8 +226,9 @@ function within(name: keyof typeof WINDOWS) { if (window === null) return false; const [start, end] = window; + const afterStart = DATE_ONLY.test(actual as string) ? instant >= start : instant > start; - return instant >= start && instant < end; + return afterStart && instant < end; }; } diff --git a/packages/workflow-executor/src/types/validated/step-definition.ts b/packages/workflow-executor/src/types/validated/step-definition.ts index d82ed84132..46657c8962 100644 --- a/packages/workflow-executor/src/types/validated/step-definition.ts +++ b/packages/workflow-executor/src/types/validated/step-definition.ts @@ -71,7 +71,7 @@ export const CONDITION_OPERATORS = [ ] as const; export type ConditionOperator = (typeof CONDITION_OPERATORS)[number]; -export const VALUE_LESS_OPERATORS: readonly ConditionOperator[] = [ +const VALUE_LESS_OPERATORS: readonly ConditionOperator[] = [ 'present', 'blank', 'past', diff --git a/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts b/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts index 04c2734786..1970be3217 100644 --- a/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts +++ b/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts @@ -138,11 +138,14 @@ describe('toAvailableStepExecution', () => { // A relative date must resolve the same on every executor instance, so the machine's zone is // never what an unset or absent value falls back to. - it.each([null, undefined, ''])('should fall back to UTC when the timezone is %p', timezone => { - const result = toAvailableStepExecution(makeRun({ timezone })); + it.each([null, undefined, '', 'Mars/Olympus'])( + 'should fall back to UTC when the timezone is %p', + timezone => { + const result = toAvailableStepExecution(makeRun({ timezone })); - expect(result?.timezone).toBe('UTC'); - }); + expect(result?.timezone).toBe('UTC'); + }, + ); it('should forward the run triggerType', () => { const run = makeRun({ triggerType: ServerWorkflowTriggerType.webhook }); 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 0252cb2a7a..8ca1be4c1c 100644 --- a/packages/workflow-executor/test/executors/condition-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/condition-step-executor.test.ts @@ -1142,7 +1142,6 @@ describe('ConditionStepExecutor', () => { // stamped 5 September 00:30 Paris is "today" only if the project zone, not UTC nor the // machine's, is what the evaluator was handed. it('reads relative dates in the context timezone and records the instant it used', async () => { - jest.useFakeTimers().setSystemTime(new Date('2026-09-04T23:00:00Z')); const todayInParis: ConditionPreRecordedArgs = { optionConditions: [ { @@ -1160,6 +1159,7 @@ describe('ConditionStepExecutor', () => { ); try { + jest.useFakeTimers().setSystemTime(new Date('2026-09-04T23:00:00Z')); const result = await new ConditionStepExecutor(context).execute(); expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Today'); @@ -1178,7 +1178,6 @@ describe('ConditionStepExecutor', () => { }); it('does not read a record from the next UTC day as today when the project zone is UTC', async () => { - jest.useFakeTimers().setSystemTime(new Date('2026-09-04T23:00:00Z')); const todayArgs: ConditionPreRecordedArgs = { optionConditions: [ { @@ -1194,6 +1193,7 @@ describe('ConditionStepExecutor', () => { ]); try { + jest.useFakeTimers().setSystemTime(new Date('2026-09-04T23:00:00Z')); const result = await new ConditionStepExecutor(context).execute(); expect((result.stepOutcome as ConditionStepOutcome).selectedOption).toBe('Other'); 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 a61236229f..e7dfca9392 100644 --- a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -328,6 +328,12 @@ describe('evaluateOperator', () => { expect(ev('includes_all', ['a'], ['a', 'b'])).toBe(false); expect(ev('includes_all', 'a', ['a'])).toBe(false); }); + + // every() on an empty list is vacuously true, which would make a blank widget match every + // record instead of none. + it('is not met on an empty wanted list', () => { + expect(ev('includes_all', ['a'], [])).toBe(false); + }); }); describe('before / after', () => { @@ -371,6 +377,13 @@ describe('evaluateOperator', () => { expect(ev('after_x_hours_ago', '2026-09-04T08:00:00Z', 2)).toBe(false); }); + it('accepts zero and fractional hours, like the list filter widget', () => { + expect(ev('before_x_hours_ago', '2026-09-04T10:00:00Z', 0)).toBe(true); + expect(ev('before_x_hours_ago', '2026-09-04T11:00:00Z', 0)).toBe(false); + expect(ev('before_x_hours_ago', '2026-09-04T08:59:00Z', 1.5)).toBe(true); + expect(ev('before_x_hours_ago', '2026-09-04T09:01:00Z', 1.5)).toBe(false); + }); + it('is not met on a count that is not a non-negative number', () => { expect(ev('before_x_hours_ago', '2026-09-04T08:00:00Z', -1)).toBe(false); expect(ev('before_x_hours_ago', '2026-09-04T08:00:00Z', 'two')).toBe(false); @@ -391,6 +404,35 @@ describe('evaluateOperator', () => { expect(ev('yesterday', '2026-09-03T23:00:00Z')).toBe(false); }); + // The toolkit's transforms emit GreaterThan for a datetime and GreaterThanOrEqual for a + // calendar date: a record stamped exactly at midnight is "today" in a list filter only when + // the column is a Dateonly, and the Decision must not say otherwise. + it('treats the very start of the window like the list filter does', () => { + expect(ev('today', '2026-09-03T22:00:00Z')).toBe(false); + expect(ev('today', '2026-09-03T22:00:00.001Z')).toBe(true); + expect(ev('today', '2026-09-04')).toBe(true); + expect(ev('yesterday', '2026-09-03')).toBe(true); + }); + + it('reads the SQL datetime form with a space as UTC too', () => { + expect(ev('today', '2026-09-04 08:00:00')).toBe(true); + expect(ev('today', '2026-09-04 23:00:00')).toBe(false); + }); + + // Paris switched to summer time on 2026-03-29 at 02:00: that day is 23 hours long, and a + // window built from wall-clock arithmetic instead of zone-aware startOf would drift by an hour. + it('keeps the day bounds right across a DST change', () => { + const afterSwitch: Clock = { + now: new Date('2026-03-30T08:00:00Z'), + timezone: 'Europe/Paris', + }; + + expect(ev('yesterday', '2026-03-28T23:00:01Z', undefined, afterSwitch)).toBe(true); + expect(ev('yesterday', '2026-03-28T22:59:59Z', undefined, afterSwitch)).toBe(false); + expect(ev('yesterday', '2026-03-29T21:59:59Z', undefined, afterSwitch)).toBe(true); + expect(ev('yesterday', '2026-03-29T22:00:00Z', undefined, afterSwitch)).toBe(false); + }); + it('previous_x_days excludes today, previous_x_days_to_date includes it up to the instant', () => { expect(ev('previous_x_days', '2026-09-01T12:00:00Z', 7)).toBe(true); expect(ev('previous_x_days', '2026-09-04T09:00:00Z', 7)).toBe(false); From 34dff405792770332c3eed9e4a77e4b934af1315 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Fri, 4 Sep 2026 22:07:30 +0200 Subject: [PATCH 03/10] fix(workflow-executor): read a calendar date against the bound's day, parse Postgres datetime forms A mid-day bound (past, before_x_hours_ago, the end of previous_x_days_to_date) is read at the start of its day when the value is a calendar date, as the toolkit formats the bound with toISODate for a Dateonly column. The SQL datetime form with a bare-hour or detached offset parses again. Schema tests cover every operator of the contract. Co-Authored-By: Claude Fable 5.1 --- packages/workflow-executor/CLAUDE.md | 20 ++-- .../src/executors/condition-step-executor.ts | 3 +- .../deterministic-condition-evaluator.ts | 43 ++++--- .../src/types/step-execution-data.ts | 6 +- .../src/types/validated/execution.ts | 1 - .../src/types/validated/step-definition.ts | 1 - .../run-to-available-step-mapper.test.ts | 2 - .../executors/condition-step-executor.test.ts | 6 +- .../deterministic-condition-evaluator.test.ts | 54 +++++++-- .../test/types/step-definition.test.ts | 106 +++++++++++------- 10 files changed, 152 insertions(+), 90 deletions(-) diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index ec2fa9dfbb..9e45f0a7cc 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -6,7 +6,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Overview -`@forestadmin/workflow-executor` — a framework-agnostic TypeScript library that runs Forest Admin workflow **steps on the client's infrastructure**, next to the Forest Admin agent. The orchestrator sends only step *definitions* (metadata, never client data); this package fetches them, executes locally with access to client data, and reports outcomes back. +`@forestadmin/workflow-executor` — a framework-agnostic TypeScript library that runs Forest Admin workflow **steps on the client's infrastructure**, next to the Forest Admin agent. The orchestrator sends only step _definitions_ (metadata, never client data); this package fetches them, executes locally with access to client data, and reports outcomes back. **Why it exists:** workflows historically ran entirely in the **frontend** (BPMN parsing, run state machine, AI calls, tool execution). That blocks automation (scheduled / API-triggered / headless runs need a browser open). This package moves step execution to the backend. It must stay **behavior-ISO with the front** (`forestadmin/frontend`, `app/features/workflow/`): same tool schemas, AI interactions, fallback logic. @@ -15,6 +15,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ``` Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this pkg) ──▶ Agent (datasources) ``` + - **Front** — designs workflows, triggers runs, shows progress. Still executes the `awaiting-input` path (manual decisions, action forms). - **Orchestrator** (Forest server) — stores definitions, manages run state machines, dispatches steps. Never sees client data. - **Executor** (this pkg) — pulls pending steps, runs them locally, reports `StepOutcome`. **The privacy boundary lives here.** @@ -36,20 +37,21 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p `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`. **No condition evaluation ever fails the step, data or reference alike**: a resolved value that is null is `met: null`, and a reference that read nothing is `met: null` + a `reason` (`source-step-not-reached` when no Get Data with that id ran on this path, `field-not-loaded` when one ran but the field is absent or errored), plus a `Warn`. Build-time validation cannot cover the second case (a Get Data step may let the AI pick its fields), so the run has to answer at runtime — and the way a decision step answers is by routing, unlike record steps (line above) which die on an unresolvable reference. What stops the fallback from passing for a decision the data took is the `reason`: the run view words it for the operator instead of showing a bare cross. "Never read" is not "read and empty", so `present`/`blank` get no answer out of it either. 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`. - - **Operator set = what a list view filter offers** (PRD-1147), 23 names in `CONDITION_OPERATORS`; nothing the front registry cannot render (so no `>=`, `<=`, `not_in`). 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`), whole numbers exactly as BigInt; 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); text operators are strings-only, case sensitive except `i_contains`, which folds case and keeps accents like Postgres `ILIKE`. - - **Relative dates read an injected `Clock`** (`{ now, timezone }`), never the machine's: the step executor builds one per step (so every row of a Decision sees the same instant) from `ExecutionContext.timezone`, which the orchestrator sends as the **project's** zone (`AvailableStepExecution.timezone`, mapper falls back to `UTC` when unset or absent). The machine's zone is never used: the fleet runs several instances, and the same run must route the same on each. Day windows (`today`, `yesterday`, `previous_x_days`, `previous_x_days_to_date`) are computed in that zone with the exact bounds of `datasource-toolkit`'s time transforms: end excluded, start included for a calendar date and excluded for a datetime (the toolkit emits `GreaterThanOrEqual` for Dateonly, `GreaterThan` otherwise), so a record answers "today" the same way in a Decision and in a list filter; a **calendar date** (Dateonly) is read at midnight in that zone, otherwise Honolulu's own "today" would count as yesterday. An unknown zone name falls back to `UTC` in the mapper, like an absent one. The trace persists `evaluatedAt` + `timezone` alongside `evaluations`, since a relative condition cannot be explained later without the instant it was read at. A retry re-evaluates with a fresh clock and may route differently: accepted, by product decision. -- **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. + - **Operator set = what a list view filter offers** (PRD-1147), 23 names in `CONDITION_OPERATORS`; nothing the front registry cannot render (so no `>=`, `<=`, `not_in`). 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`), whole numbers exactly as BigInt; 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); text operators are strings-only, case sensitive except `i_contains`, which folds case and keeps accents like the list filter's Postgres `ILIKE` path (other dialects' collations diverge; Postgres is the contract). + - **Relative dates read an injected `Clock`** (`{ now, timezone }`), never the machine's: the step executor builds one per step (so every condition of every option sees the same instant) from `ExecutionContext.timezone`, which the orchestrator sends as the **project's** zone (`AvailableStepExecution.timezone`, mapper falls back to `UTC` when unset or absent). The machine's zone is never used: the fleet runs several instances, and the same run must route the same on each. Day windows (`today`, `yesterday`, `previous_x_days`, `previous_x_days_to_date`) are computed in that zone with the exact bounds of `datasource-toolkit`'s time transforms: end excluded, start included for a calendar date and excluded for a datetime (the toolkit emits `GreaterThanOrEqual` for Dateonly, `GreaterThan` otherwise), and a mid-day bound (`past`, `before_x_hours_ago`, the end of `previous_x_days_to_date`) read at the start of its day for a calendar date (the toolkit formats the bound with `toISODate` for a Dateonly column), so a record answers "today" the same way in a Decision and in a list filter; a **calendar date** (Dateonly) is read at midnight in that zone, otherwise Honolulu's own "today" would count as yesterday. An unknown zone name falls back to `UTC` in the mapper, like an absent one. The trace persists `evaluatedAt` + `timezone` alongside `evaluations`, since a relative condition cannot be explained later without the instant it was read at. A retry re-evaluates with a fresh clock and may route differently: accepted, by product decision. + +- **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. All four record steps pin the source by `selectedRecordStepId` — a **stable BPMN step id** (or the `WORKFLOW_START_STEP_ID` sentinel) resolved by `resolveSourceRecordRef`, chosen to survive the index shifts a revision causes; the editor writes it and treats it as a precondition for choosing fields. Presence, not truthiness, decides whether something is pinned -- `selectedRecordStepId`, `actionName` and `relationName` all check `!== undefined`. An empty value is a pin that lost its target, so it resolves to nothing and errors instead of falling back to the AI, which would silently pick a different record, run a different action, or follow a different relation than the step was configured to. Not reachable from the editor, which writes `stepId || undefined`. `selectedRecordStepIndex` (a runtime index, resolved in `resolveRecordRef`) survives only as a fallback on read-record/update-record, checked after the step id. Partial args supported. Unresolvable → `PinnedArgNotFoundError` when the name was pinned (`configuration`), `FieldNotFoundError`/`ActionNotFoundError`/`RelationNotFoundError` when the AI chose it (**unclassified** — a re-run may resolve differently, so nothing permanent can be asserted); bad shape / out-of-range index → `InvalidPreRecordedArgsError`. The split exists because the AI-facing messages tell the operator to rephrase the prompt — the right remedy for a name the AI chose, and unreachable for one the workflow fixed, where the step itself is the thing to edit. One class covers every pinned kind: the diagnosis differs per kind, the operator's remedy does not. Anything raised on a pinned value has to name the step. ## Invariants (read before changing executors) - **Privacy** — `StepOutcome` goes to the orchestrator and must **never** contain client data. Privacy-sensitive info (AI reasoning, record values) stays in `StepExecutionData` (RunStore, client-side only). - **Error hierarchy** (`errors.ts`): - - *Step-execution errors* extend `WorkflowExecutorError` → caught by `base-step-executor.ts`, turned into `stepOutcome.error`. Never reach HTTP. - - *Boundary errors* (`ConfigurationError`, `PendingDataNotFoundError`, `AgentProbeError`, …) extend plain `Error` → caught at HTTP/Runner layer. They must **not** extend `WorkflowExecutorError` (or the base executor would swallow them). - - `WorkflowExecutorError` carries `message` (technical, logs) **and** `userMessage` (end-user, surfaced via `stepOutcome.error`). New subclasses must set a distinct, jargon-free `userMessage`. Request-level errors extend a *category* (`NotFoundError`/`AccessDeniedError`/`UnavailableError`) so `toHttpError` maps status by category — no per-error binding. - - **Error kind** — `errorKind` (`operator`/`configuration`/`system`) classifies what kind of failure a step error is. It does not encode ownership: the front maps `configuration`/`system` to admin-phrased copy as a reasonable default, which is a front-end choice, not a property of the enum. One abstract per classified kind declares it once (`WorkflowOperatorError`, `WorkflowConfigurationError`, each setting `static defaultErrorKind`); a new member joins a family by extending it, and an error extending neither stays unclassified. The throw site overrides only where the same error can be either kind (`SourceRecordMissingError`, on whether a candidate was offered). Unset ⇒ absent from the outcome ⇒ the front frames it as it always has, so leaving a new error unclassified is safe. `errorSourceStepIndex` names the step an error is *about*, by index rather than step id — a LinkTo loop repeats ids, so only the index identifies the iteration. + - _Step-execution errors_ extend `WorkflowExecutorError` → caught by `base-step-executor.ts`, turned into `stepOutcome.error`. Never reach HTTP. + - _Boundary errors_ (`ConfigurationError`, `PendingDataNotFoundError`, `AgentProbeError`, …) extend plain `Error` → caught at HTTP/Runner layer. They must **not** extend `WorkflowExecutorError` (or the base executor would swallow them). + - `WorkflowExecutorError` carries `message` (technical, logs) **and** `userMessage` (end-user, surfaced via `stepOutcome.error`). New subclasses must set a distinct, jargon-free `userMessage`. Request-level errors extend a _category_ (`NotFoundError`/`AccessDeniedError`/`UnavailableError`) so `toHttpError` maps status by category — no per-error binding. + - **Error kind** — `errorKind` (`operator`/`configuration`/`system`) classifies what kind of failure a step error is. It does not encode ownership: the front maps `configuration`/`system` to admin-phrased copy as a reasonable default, which is a front-end choice, not a property of the enum. One abstract per classified kind declares it once (`WorkflowOperatorError`, `WorkflowConfigurationError`, each setting `static defaultErrorKind`); a new member joins a family by extending it, and an error extending neither stays unclassified. The throw site overrides only where the same error can be either kind (`SourceRecordMissingError`, on whether a candidate was offered). Unset ⇒ absent from the outcome ⇒ the front frames it as it always has, so leaving a new error unclassified is safe. `errorSourceStepIndex` names the step an error is _about_, by index rather than step id — a LinkTo loop repeats ids, so only the index identifies the iteration. - Both fields ride `context` on the update-step request and are read back by `run-to-available-step-mapper`, which drops an off-vocabulary value instead of passing it on: it would fail `AvailableStepExecutionSchema.parse` and take the whole run down. The front equality-matches `operator`, so a widened enum degrades an older front rather than breaking it. - The orchestrator merges step `context` shallowly and an unclassified error omits `errorKind` rather than nulling it, so a kind written on a step index cannot be cleared server-side. That is safe only because no index ever receives two error reports: an error sets `done: true`, a done step is never re-dispatched, and every start appends a fresh index with an empty context. If a retryable error is ever left pending instead of done, this stops holding. - `errorKind` is unrelated to ai-proxy's `McpLoadFailureKind` (`auth`/`connection`/`unknown`, reported per server on the MCP `failures` channel): that one says where a tool load broke, this one says what kind of failure the step hit. They are deliberately separate vocabularies — don't map one onto the other. @@ -64,7 +66,7 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p - **Graceful shutdown** — `stop()` drains in-flight steps (`idle → running → draining → stopped`), `stopTimeoutMs` default 30s, HTTP stays up during drain. Signal handling is the consumer's job. Trigger-started chains fall under the same drain as polled ones: a deploy mid-chain truncates them at `stopTimeoutS` (they used to be held open by the unanswered request). - **Logging** — `Logger = (level, message, context?) => void`. `BaseStepExecutor` stamps `logCtx` (runId/stepId/stepIndex/stepType); type-specific ids via `getExtraLogContext()`. `createConsoleLogger`/`createPrettyLogger(minLevel)` factories; CLI level from `LOG_LEVEL` (default `Info`). ai-proxy's logger takes the cause as a third `Error` argument instead of a context object, so both AI adapters bridge it with `toAiProxyLogger` (flattens to `{ error, cause, stack }` — an `Error`'s own properties are non-enumerable and would vanish from the emitted line — and swallows a throwing host logger, which ai-proxy calls from inside its catch blocks). - **MCP load failures come from the `failures` channel** — `RemoteToolFetcher` loads through `loadRemoteToolsWithFailures` and reports what the providers classified (`server`/`kind`/`error`); never infer failure from absent tools, which flags a healthy server exposing none. `loadFailed` drives the 503 on `GET /list-mcp-tools`, so a wrong inference is user-visible. -- **Config comes from the boundary, never `process.env`** — no executor *config* is read from `process.env` outside `cli-core`: every knob is parsed there (standalone) or injected as an option (`ExecutorOptions` / the agent's `addWorkflowExecutor` options), and the check for a value is `Boolean(options.x)`, not `process.env`. Runtime-mode flags — `NODE_ENV` (forceAiError prod-guard, token-endpoint dev check) and the `OTEL_*` observability vars in `tracing.ts` — are the deliberate exception. This keeps the executor identically configurable standalone and embedded, and testable without mutating env. (Regression fixed once: `FOREST_EXECUTOR_ENCRYPTION_KEY` was read in `crypto/` — now injected via `executorEncryptionKey`.) +- **Config comes from the boundary, never `process.env`** — no executor _config_ is read from `process.env` outside `cli-core`: every knob is parsed there (standalone) or injected as an option (`ExecutorOptions` / the agent's `addWorkflowExecutor` options), and the check for a value is `Boolean(options.x)`, not `process.env`. Runtime-mode flags — `NODE_ENV` (forceAiError prod-guard, token-endpoint dev check) and the `OTEL_*` observability vars in `tracing.ts` — are the deliberate exception. This keeps the executor identically configurable standalone and embedded, and testable without mutating env. (Regression fixed once: `FOREST_EXECUTOR_ENCRYPTION_KEY` was read in `crypto/` — now injected via `executorEncryptionKey`.) - **AI** — import every AI type (`BaseChatModel`, `DynamicStructuredTool`, `SystemMessage`/`HumanMessage`, `RemoteTool`/`ToolConfig`) from `@forestadmin/ai-proxy`, **not** `@langchain/core` (which is not a dependency). `ExecutionContext.model` is a `BaseChatModel`. The only langchain mention in src is a comment in `cli.ts` about transitively loading `@langchain/openai`. ## Commands diff --git a/packages/workflow-executor/src/executors/condition-step-executor.ts b/packages/workflow-executor/src/executors/condition-step-executor.ts index 645bf0801a..ff191253c5 100644 --- a/packages/workflow-executor/src/executors/condition-step-executor.ts +++ b/packages/workflow-executor/src/executors/condition-step-executor.ts @@ -127,8 +127,7 @@ export default class ConditionStepExecutor extends BaseStepExecutor { const { optionConditions, fallbackOption } = step.preRecordedArgs; const stepExecutions = await this.context.runStore.getStepExecutions(this.context.runId); - // One clock for the whole step: every condition of every option reads the same instant, so - // "today" cannot flip between two rows evaluated a millisecond apart. + // One clock per step, so "today" cannot flip between the first and the last condition. const clock: Clock = { now: new Date(), timezone: this.context.timezone }; let matchedOption: string | undefined; diff --git a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts index 0de359a572..6c61d1ad86 100644 --- a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts +++ b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts @@ -2,9 +2,7 @@ import type { ConditionOperator } from '../types/validated/step-definition'; import { DateTime } from 'luxon'; -// The instant and zone a relative condition is read against. Injected by the step executor rather -// than read here, so the evaluator stays a pure function of its inputs: two runs handed the same -// clock answer the same, and a test can pin any moment it likes. +// Injected rather than read here so the evaluator stays a pure function of its inputs. export interface Clock { now: Date; timezone: string; @@ -33,9 +31,14 @@ function toTimestamp(value: unknown): number | 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. The SQL form - // with a space separator is a datetime too. - const datetime = value.replace(' ', 'T'); + // the same run differently per machine — pin every offset-less datetime to UTC. Postgres emits + // the SQL form ("2026-09-04 08:00:00+02"): space separator, bare-hour offset that Date.parse + // only accepts as hh:mm once the separator is a T. + const datetime = value + .trim() + .replace(/\s+/, 'T') + .replace(/\s+/g, '') + .replace(/(:\d{2}(?:\.\d+)?)([+-]\d{2})$/, '$1$2:00'); const absolute = datetime.includes('T') && !TIMEZONE_SUFFIX.test(datetime) ? `${datetime}Z` : datetime; const parsed = Date.parse(absolute); @@ -45,14 +48,23 @@ function toTimestamp(value: unknown): number | null { // A calendar date has no instant of its own, so it is read at midnight in the project's zone: that // is what makes "today" the project's today for a Dateonly column, whatever the machine's or UTC's -// day is at that moment. A datetime is an instant already and keeps the UTC pinning above. +// day is at that moment. A datetime is an absolute instant already (the UTC pinning above). function toInstant(value: unknown, timezone: string): DateTime | null { if (typeof value !== 'string' || !isRealCalendarDate(value.slice(0, 10))) return null; if (DATE_ONLY.test(value)) return DateTime.fromISO(value, { zone: timezone }); const timestamp = toTimestamp(value); - return timestamp === null ? null : DateTime.fromMillis(timestamp); + return timestamp === null ? null : DateTime.fromMillis(timestamp, { zone: timezone }); +} + +// The toolkit compares a Dateonly column against the bound's calendar date (its time transforms +// format the bound with toISODate for that column type), so a bound falling mid-day is read at the +// start of its day when the value is a calendar date: "past" on a Dateonly excludes today, as the +// list filter does. The kind is told from the value's shape where the toolkit reads the column +// type, so parity holds as long as a Dateonly serialises as a bare YYYY-MM-DD. +function alignToValueKind(actual: unknown, bound: DateTime): DateTime { + return DATE_ONLY.test(actual as string) ? bound.startOf('day') : bound; } function toNumber(value: unknown): number | null { @@ -156,8 +168,6 @@ function includesAll(actual: unknown, expected: unknown): boolean { return wanted.length > 0 && wanted.every(item => isMemberOf(actual, item)); } -// Strings only: no coercion of a number into text, so a broken config can never accidentally -// satisfy a text operator. function stringTest(satisfies: (actual: string, expected: string) => boolean) { return (actual: unknown, expected: unknown): boolean => typeof actual === 'string' && typeof expected === 'string' && satisfies(actual, expected); @@ -172,8 +182,8 @@ function ordering(satisfies: (diff: number) => boolean) { } // The builder pins a datetime for a Date column and a calendar date for a Dateonly one, so both -// sides share a kind and both go through toInstant: a calendar date is read in the project's zone -// on either side, which keeps "before 2026-03-01" meaning the same day the author had in mind. +// sides share a kind and a calendar date is read in the project's zone on either side. The toolkit +// reads a bare date in the server's zone here; the project's is the deliberate choice. function dateOrdering(satisfies: (actual: DateTime, expected: DateTime) => boolean) { return (actual: unknown, expected: unknown, clock: Clock): boolean => { const actualInstant = toInstant(actual, clock.timezone); @@ -225,7 +235,7 @@ function within(name: keyof typeof WINDOWS) { const window = WINDOWS[name](DateTime.fromJSDate(clock.now).setZone(clock.timezone), expected); if (window === null) return false; - const [start, end] = window; + const [start, end] = window.map(bound => alignToValueKind(actual, bound)); const afterStart = DATE_ONLY.test(actual as string) ? instant >= start : instant > start; return afterStart && instant < end; @@ -242,7 +252,7 @@ function relativeTo( const reference = bound(DateTime.fromJSDate(clock.now).setZone(clock.timezone), expected); - return reference !== null && satisfies(instant, reference); + return reference !== null && satisfies(instant, alignToValueKind(actual, reference)); }; } @@ -266,8 +276,8 @@ const EVALUATORS: Record< not_contains: stringTest((actual, expected) => !actual.includes(expected)), starts_with: stringTest((actual, expected) => actual.startsWith(expected)), ends_with: stringTest((actual, expected) => actual.endsWith(expected)), - // Case folded, accents kept: what Postgres ILIKE does ('É' ILIKE '%é%' holds, 'é' ILIKE '%e%' - // does not), which is the behaviour the list filter shows on the reference database. + // Case folded, accents kept: the Postgres ILIKE path of the list filter ('É' ILIKE '%é%' holds, + // 'é' ILIKE '%e%' does not). MySQL and SQLite collations diverge; Postgres is the contract. i_contains: stringTest((actual, expected) => actual.toLowerCase().includes(expected.toLowerCase()), ), @@ -294,7 +304,6 @@ const EVALUATORS: Record< * - `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. - * Relative date operators read the injected clock, never the machine's. */ export default function evaluateOperator( operator: ConditionOperator, diff --git a/packages/workflow-executor/src/types/step-execution-data.ts b/packages/workflow-executor/src/types/step-execution-data.ts index 20b6b8daf7..57c30db6d9 100644 --- a/packages/workflow-executor/src/types/step-execution-data.ts +++ b/packages/workflow-executor/src/types/step-execution-data.ts @@ -48,11 +48,7 @@ export interface DeterministicConditionExecutionParams { evaluations: ConditionEvaluation[]; selectedOption: string; usedFallback: boolean; - /** - * The instant and zone the conditions were read against. A relative date makes the routing - * depend on the clock, so without these a check on "previous 7 days" cannot be explained a day - * later. - */ + // Without the instant, a check on "previous 7 days" cannot be explained a day later. evaluatedAt: string; timezone: string; } diff --git a/packages/workflow-executor/src/types/validated/execution.ts b/packages/workflow-executor/src/types/validated/execution.ts index a6807f817d..872be57761 100644 --- a/packages/workflow-executor/src/types/validated/execution.ts +++ b/packages/workflow-executor/src/types/validated/execution.ts @@ -49,7 +49,6 @@ export const AvailableStepExecutionSchema = z stepDefinition: StepDefinitionSchema, previousSteps: z.array(StepSchema), user: StepUserSchema, - /** IANA zone the run's relative dates are read in: the project's, never the machine's. */ timezone: z.string().min(1), }) .strict(); diff --git a/packages/workflow-executor/src/types/validated/step-definition.ts b/packages/workflow-executor/src/types/validated/step-definition.ts index 46657c8962..2e56665825 100644 --- a/packages/workflow-executor/src/types/validated/step-definition.ts +++ b/packages/workflow-executor/src/types/validated/step-definition.ts @@ -86,7 +86,6 @@ const DeterministicConditionSchema = z sourceStepId: z.string().min(1), fieldName: z.string().min(1), operator: z.enum(CONDITION_OPERATORS), - /** Absent for the value-less operators (present, blank, and the relative dates without a count). */ value: z.unknown().optional(), }) // A value-bearing operator without its value compares against `undefined`: it can never be met, diff --git a/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts b/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts index 1970be3217..c8b9bfff45 100644 --- a/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts +++ b/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts @@ -136,8 +136,6 @@ describe('toAvailableStepExecution', () => { expect(result?.timezone).toBe('Europe/Paris'); }); - // A relative date must resolve the same on every executor instance, so the machine's zone is - // never what an unset or absent value falls back to. it.each([null, undefined, '', 'Mars/Olympus'])( 'should fall back to UTC when the timezone is %p', timezone => { 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 8ca1be4c1c..5ed4f3ffe4 100644 --- a/packages/workflow-executor/test/executors/condition-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/condition-step-executor.test.ts @@ -1139,8 +1139,8 @@ describe('ConditionStepExecutor', () => { }); // 2026-09-04T23:00Z is still 4 September in UTC but already 5 September in Paris. A record - // stamped 5 September 00:30 Paris is "today" only if the project zone, not UTC nor the - // machine's, is what the evaluator was handed. + // stamped 2026-09-05T00:30Z is "today" in Paris and tomorrow in UTC, so the option is taken + // only if the project zone is what the evaluator was handed. it('reads relative dates in the context timezone and records the instant it used', async () => { const todayInParis: ConditionPreRecordedArgs = { optionConditions: [ @@ -1154,7 +1154,7 @@ describe('ConditionStepExecutor', () => { }; const { context, runStore } = makeDeterministicContext( todayInParis, - [{ name: 'signedAt', displayName: 'Signed at', value: '2026-09-04T22:30:00Z' }], + [{ name: 'signedAt', displayName: 'Signed at', value: '2026-09-05T00:30:00Z' }], { timezone: 'Europe/Paris' }, ); 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 e7dfca9392..d28c3b3d80 100644 --- a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -211,6 +211,15 @@ describe('evaluateOperator', () => { expect(ev('greater_than', '2026-01-01T12:00:00', '2026-01-01T11:00:00Z')).toBe(true); expect(ev('less_than', '2026-01-01T10:00:00', '2026-01-01T11:00:00+00:00')).toBe(true); }); + + // Date.parse falls back to a host-local legacy parser on the SQL form, which only a non-UTC + // host tells apart from the pinning. + it('reads the SQL datetime forms Postgres emits', () => { + expect(ev('equal', '2026-09-04 08:00:00', '2026-09-04T08:00:00Z')).toBe(true); + expect(ev('equal', '2026-09-04 08:00:00+02', '2026-09-04T06:00:00Z')).toBe(true); + expect(ev('equal', '2026-09-04 08:00:00 +02:00', '2026-09-04T06:00:00Z')).toBe(true); + expect(ev('equal', '2026-09-04 08:00:00.123', '2026-09-04T08:00:00.123Z')).toBe(true); + }); }); }); @@ -308,6 +317,7 @@ describe('evaluateOperator', () => { // 'é' ILIKE '%e%' does not. Case is folded, accents are letters of their own. it('i_contains folds case but keeps accents, like Postgres ILIKE', () => { expect(ev('i_contains', 'ACTIVE', 'act')).toBe(true); + expect(ev('i_contains', 'active', 'ACT')).toBe(true); expect(ev('i_contains', 'É', 'é')).toBe(true); expect(ev('i_contains', 'é', 'e')).toBe(false); }); @@ -349,6 +359,18 @@ describe('evaluateOperator', () => { expect(ev('after', '2026-03-01', '2026-03-02')).toBe(false); }); + // Honolulu is UTC-10: its 1 March starts at 10:00Z, so a calendar date read as UTC midnight + // would sit before that instant instead of after it. + it('reads a calendar date in the clock timezone when compared to an instant', () => { + const honolulu: Clock = { + now: new Date('2026-09-04T05:00:00Z'), + timezone: 'Pacific/Honolulu', + }; + + expect(ev('after', '2026-03-01', '2026-03-01T05:00:00Z', honolulu)).toBe(true); + expect(ev('before', '2026-03-01', '2026-03-01T05:00:00Z', honolulu)).toBe(false); + }); + it('is not met on a non-date or an impossible date', () => { expect(ev('before', 'soon', '2026-06-01T00:00:00Z')).toBe(false); expect(ev('before', '2026-02-30', '2026-06-01T00:00:00Z')).toBe(false); @@ -414,11 +436,6 @@ describe('evaluateOperator', () => { expect(ev('yesterday', '2026-09-03')).toBe(true); }); - it('reads the SQL datetime form with a space as UTC too', () => { - expect(ev('today', '2026-09-04 08:00:00')).toBe(true); - expect(ev('today', '2026-09-04 23:00:00')).toBe(false); - }); - // Paris switched to summer time on 2026-03-29 at 02:00: that day is 23 hours long, and a // window built from wall-clock arithmetic instead of zone-aware startOf would drift by an hour. it('keeps the day bounds right across a DST change', () => { @@ -431,19 +448,28 @@ describe('evaluateOperator', () => { expect(ev('yesterday', '2026-03-28T22:59:59Z', undefined, afterSwitch)).toBe(false); expect(ev('yesterday', '2026-03-29T21:59:59Z', undefined, afterSwitch)).toBe(true); expect(ev('yesterday', '2026-03-29T22:00:00Z', undefined, afterSwitch)).toBe(false); + + const onSwitchDay: Clock = { + now: new Date('2026-03-29T12:00:00Z'), + timezone: 'Europe/Paris', + }; + expect(ev('today', '2026-03-29T21:59:59Z', undefined, onSwitchDay)).toBe(true); + expect(ev('today', '2026-03-29T22:00:00Z', undefined, onSwitchDay)).toBe(false); + expect(ev('today', '2026-03-29T22:30:00Z', undefined, onSwitchDay)).toBe(false); }); it('previous_x_days excludes today, previous_x_days_to_date includes it up to the instant', () => { expect(ev('previous_x_days', '2026-09-01T12:00:00Z', 7)).toBe(true); expect(ev('previous_x_days', '2026-09-04T09:00:00Z', 7)).toBe(false); - expect(ev('previous_x_days', '2026-08-27T21:00:00Z', 7)).toBe(false); + expect(ev('previous_x_days', '2026-08-27T22:00:01Z', 7)).toBe(true); + expect(ev('previous_x_days', '2026-08-27T21:59:59Z', 7)).toBe(false); expect(ev('previous_x_days_to_date', '2026-09-04T09:00:00Z', 7)).toBe(true); expect(ev('previous_x_days_to_date', '2026-09-04T11:00:00Z', 7)).toBe(false); }); it('is not met on a count that is not a positive whole number', () => { expect(ev('previous_x_days', '2026-09-01T12:00:00Z', 0)).toBe(false); - expect(ev('previous_x_days', '2026-09-01T12:00:00Z', 1.5)).toBe(false); + expect(ev('previous_x_days', '2026-09-03T06:00:00Z', 1.5)).toBe(false); expect(ev('previous_x_days', '2026-09-01T12:00:00Z', 'seven')).toBe(false); }); @@ -464,5 +490,19 @@ describe('evaluateOperator', () => { expect(ev('today', 'now')).toBe(false); expect(ev('today', 150)).toBe(false); }); + + // The list filter compares a Dateonly column to the bound's calendar date, so a record dated + // today is neither "in the past" nor "before 2 hours ago" nor within the days to date. + it('reads a mid-day bound at the start of its day for a calendar date, like the list filter', () => { + expect(ev('past', '2026-09-04')).toBe(false); + expect(ev('past', '2026-09-03')).toBe(true); + expect(ev('future', '2026-09-04')).toBe(false); + expect(ev('future', '2026-09-05')).toBe(true); + expect(ev('before_x_hours_ago', '2026-09-04', 2)).toBe(false); + expect(ev('before_x_hours_ago', '2026-09-03', 2)).toBe(true); + expect(ev('previous_x_days_to_date', '2026-09-04', 7)).toBe(false); + expect(ev('previous_x_days_to_date', '2026-09-03', 7)).toBe(true); + expect(ev('previous_x_days_to_date', '2026-09-04T09:00:00Z', 7)).toBe(true); + }); }); }); diff --git a/packages/workflow-executor/test/types/step-definition.test.ts b/packages/workflow-executor/test/types/step-definition.test.ts index 36a6dc8758..7baf0bd844 100644 --- a/packages/workflow-executor/test/types/step-definition.test.ts +++ b/packages/workflow-executor/test/types/step-definition.test.ts @@ -90,27 +90,29 @@ describe('ConditionStepDefinitionSchema deterministic conditions', () => { }); // 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%' }, - ], - }, - ], - }, - }); + // The last three were in the contract before PRD-1147 and are refused since. + it.each(['ilike', 'not_in', 'greater_than_or_equal', 'less_than_or_equal'])( + 'rejects the unknown operator "%s" at the schema boundary', + operator => { + const result = ConditionStepDefinitionSchema.safeParse({ + ...base, + executionType: 'manual', + preRecordedArgs: { + ...preRecordedArgs, + optionConditions: [ + { + option: 'High value', + aggregator: 'and', + conditions: [{ sourceStepId: 'get-data-1', fieldName: 'amount', operator, value: 1 }], + }, + ], + }, + }); - expect(result.success).toBe(false); - expect(JSON.stringify(!result.success && result.error.issues)).toContain('operator'); - }); + 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. @@ -210,8 +212,45 @@ describe('ConditionStepDefinitionSchema deterministic conditions', () => { 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', + it.each([ + 'equal', + 'not_equal', + 'greater_than', + 'less_than', + 'in', + 'includes_all', + 'contains', + 'not_contains', + 'starts_with', + 'ends_with', + 'i_contains', + 'before', + 'after', + 'previous_x_days', + 'previous_x_days_to_date', + 'before_x_hours_ago', + 'after_x_hours_ago', + ])('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.each(['present', 'blank', 'past', 'future', 'today', 'yesterday'])( + 'accepts a value-less "%s" condition', operator => { const result = ConditionStepDefinitionSchema.safeParse({ ...base, @@ -221,36 +260,17 @@ describe('ConditionStepDefinitionSchema deterministic conditions', () => { optionConditions: [ { option: 'High value', - aggregator: 'and', + aggregator: 'or', conditions: [{ sourceStepId: 'get-data-1', fieldName: 'amount', operator }], }, ], }, }); - expect(result.success).toBe(false); + expect(result.success).toBe(true); }, ); - 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: 'fully-automated' }) From 6af27b215dae31293045e43284b8570f3ecc616b Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 7 Sep 2026 09:37:18 +0200 Subject: [PATCH 04/10] fix(workflow-executor): refuse a timezone that is not an IANA zone at the boundary --- packages/workflow-executor/src/types/validated/execution.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/workflow-executor/src/types/validated/execution.ts b/packages/workflow-executor/src/types/validated/execution.ts index 872be57761..0cee16ba8c 100644 --- a/packages/workflow-executor/src/types/validated/execution.ts +++ b/packages/workflow-executor/src/types/validated/execution.ts @@ -1,3 +1,4 @@ +import { IANAZone } from 'luxon'; import { z } from 'zod'; import { RecordRefSchema } from './collection'; @@ -49,7 +50,10 @@ export const AvailableStepExecutionSchema = z stepDefinition: StepDefinitionSchema, previousSteps: z.array(StepSchema), user: StepUserSchema, - timezone: z.string().min(1), + // Refused rather than trusted: Luxon reads an unknown zone as invalid and every relative + // condition would then quietly not match. The mapper normalises to UTC, so this only ever + // fires on a mapper bug. + timezone: z.string().refine(IANAZone.isValidZone), }) .strict(); export type AvailableStepExecution = z.infer; From ce474ea078996e11464ed06ef93f0bff15eb98a5 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 7 Sep 2026 10:18:17 +0200 Subject: [PATCH 05/10] docs(workflow-executor): record that the UTC timezone fallback is the main path --- packages/workflow-executor/CLAUDE.md | 2 +- .../src/adapters/run-to-available-step-mapper.ts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index 9e45f0a7cc..81fb6a6a7e 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -39,7 +39,7 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p - **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`. **No condition evaluation ever fails the step, data or reference alike**: a resolved value that is null is `met: null`, and a reference that read nothing is `met: null` + a `reason` (`source-step-not-reached` when no Get Data with that id ran on this path, `field-not-loaded` when one ran but the field is absent or errored), plus a `Warn`. Build-time validation cannot cover the second case (a Get Data step may let the AI pick its fields), so the run has to answer at runtime — and the way a decision step answers is by routing, unlike record steps (line above) which die on an unresolvable reference. What stops the fallback from passing for a decision the data took is the `reason`: the run view words it for the operator instead of showing a bare cross. "Never read" is not "read and empty", so `present`/`blank` get no answer out of it either. 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`. - **Operator set = what a list view filter offers** (PRD-1147), 23 names in `CONDITION_OPERATORS`; nothing the front registry cannot render (so no `>=`, `<=`, `not_in`). 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`), whole numbers exactly as BigInt; 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); text operators are strings-only, case sensitive except `i_contains`, which folds case and keeps accents like the list filter's Postgres `ILIKE` path (other dialects' collations diverge; Postgres is the contract). - - **Relative dates read an injected `Clock`** (`{ now, timezone }`), never the machine's: the step executor builds one per step (so every condition of every option sees the same instant) from `ExecutionContext.timezone`, which the orchestrator sends as the **project's** zone (`AvailableStepExecution.timezone`, mapper falls back to `UTC` when unset or absent). The machine's zone is never used: the fleet runs several instances, and the same run must route the same on each. Day windows (`today`, `yesterday`, `previous_x_days`, `previous_x_days_to_date`) are computed in that zone with the exact bounds of `datasource-toolkit`'s time transforms: end excluded, start included for a calendar date and excluded for a datetime (the toolkit emits `GreaterThanOrEqual` for Dateonly, `GreaterThan` otherwise), and a mid-day bound (`past`, `before_x_hours_ago`, the end of `previous_x_days_to_date`) read at the start of its day for a calendar date (the toolkit formats the bound with `toISODate` for a Dateonly column), so a record answers "today" the same way in a Decision and in a list filter; a **calendar date** (Dateonly) is read at midnight in that zone, otherwise Honolulu's own "today" would count as yesterday. An unknown zone name falls back to `UTC` in the mapper, like an absent one. The trace persists `evaluatedAt` + `timezone` alongside `evaluations`, since a relative condition cannot be explained later without the instant it was read at. A retry re-evaluates with a fresh clock and may route differently: accepted, by product decision. + - **Relative dates read an injected `Clock`** (`{ now, timezone }`), never the machine's: the step executor builds one per step (so every condition of every option sees the same instant) from `ExecutionContext.timezone`, which the orchestrator sends as the **project's** zone (`AvailableStepExecution.timezone`, mapper falls back to `UTC` when unset, absent or not an IANA name). **`UTC` is the main path, not an edge case**: 0.7% of projects carry a timezone (635/85457, measured September 2026), so a Decision's "today" is usually UTC's day while the list filter shows the browser's. The machine's zone is never used: the fleet runs several instances, and the same run must route the same on each. Day windows (`today`, `yesterday`, `previous_x_days`, `previous_x_days_to_date`) are computed in that zone with the exact bounds of `datasource-toolkit`'s time transforms: end excluded, start included for a calendar date and excluded for a datetime (the toolkit emits `GreaterThanOrEqual` for Dateonly, `GreaterThan` otherwise), and a mid-day bound (`past`, `before_x_hours_ago`, the end of `previous_x_days_to_date`) read at the start of its day for a calendar date (the toolkit formats the bound with `toISODate` for a Dateonly column), so a record answers "today" the same way in a Decision and in a list filter; a **calendar date** (Dateonly) is read at midnight in that zone, otherwise Honolulu's own "today" would count as yesterday. An unknown zone name falls back to `UTC` in the mapper, like an absent one. The trace persists `evaluatedAt` + `timezone` alongside `evaluations`, since a relative condition cannot be explained later without the instant it was read at. A retry re-evaluates with a fresh clock and may route differently: accepted, by product decision. - **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. All four record steps pin the source by `selectedRecordStepId` — a **stable BPMN step id** (or the `WORKFLOW_START_STEP_ID` sentinel) resolved by `resolveSourceRecordRef`, chosen to survive the index shifts a revision causes; the editor writes it and treats it as a precondition for choosing fields. Presence, not truthiness, decides whether something is pinned -- `selectedRecordStepId`, `actionName` and `relationName` all check `!== undefined`. An empty value is a pin that lost its target, so it resolves to nothing and errors instead of falling back to the AI, which would silently pick a different record, run a different action, or follow a different relation than the step was configured to. Not reachable from the editor, which writes `stepId || undefined`. `selectedRecordStepIndex` (a runtime index, resolved in `resolveRecordRef`) survives only as a fallback on read-record/update-record, checked after the step id. Partial args supported. Unresolvable → `PinnedArgNotFoundError` when the name was pinned (`configuration`), `FieldNotFoundError`/`ActionNotFoundError`/`RelationNotFoundError` when the AI chose it (**unclassified** — a re-run may resolve differently, so nothing permanent can be asserted); bad shape / out-of-range index → `InvalidPreRecordedArgsError`. The split exists because the AI-facing messages tell the operator to rephrase the prompt — the right remedy for a name the AI chose, and unreachable for one the workflow fixed, where the step itself is the thing to edit. One class covers every pinned kind: the diagnosis differs per kind, the operator's remedy does not. Anything raised on a pinned value has to name the step. diff --git a/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts b/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts index 40918aa6d5..9d2c35e148 100644 --- a/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts +++ b/packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts @@ -183,6 +183,10 @@ export default function toAvailableStepExecution( // the name is not a zone Luxon knows: a relative date must resolve the same on every executor // instance, so the machine's zone is never the fallback. The zone actually used is persisted // with the evaluation, so the run view shows UTC rather than the name it fell back from. + // This branch is the main path, not an edge case: 635 of 85457 projects carry a timezone + // (0.7%, measured on production in September 2026), so almost every Decision reads its + // relative dates in UTC — an hour or two away from the day the list filter shows the same + // user, since that one follows the browser. timezone: run.timezone && IANAZone.isValidZone(run.timezone) ? run.timezone : 'UTC', }; From e9b0085036e9afcb4096923629faa81c8895fd0d Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 7 Sep 2026 14:50:18 +0200 Subject: [PATCH 06/10] fix(workflow-executor): order a time of day, which no operator could compare The builder offers is greater than and is less than on a Time column, and both were never met: compare had no branch for a time of day, so the step routed to the fallback on every record with nothing in the trace to explain it. --- .../deterministic-condition-evaluator.ts | 21 +++++++++++++++++ .../deterministic-condition-evaluator.test.ts | 23 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts index 6c61d1ad86..90d2f63308 100644 --- a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts +++ b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts @@ -16,6 +16,8 @@ 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+)?$/; +// A Time column holds a zero-padded time of day, which nothing else in this file can order. +const TIME_OF_DAY = /^\d{2}:\d{2}(:\d{2}(\.\d+)?)?$/; const INTEGER_STRING = /^-?\d+$/; // Date.parse rolls an out-of-range day over ("2026-02-30" becomes 2026-03-02), which would make a @@ -135,6 +137,14 @@ function isEqual(actual: unknown, expected: unknown): boolean | null { return scalarEqual(actual, expected); } +// Zero padding makes a time of day sort chronologically as text, so the seconds are filled in +// rather than parsed: "08:30" and "08:30:00" are the same instant of the day and must compare equal. +function toTimeOfDay(value: unknown): string | null { + if (typeof value !== 'string' || !TIME_OF_DAY.test(value)) return null; + + return value.length === 5 ? `${value}:00` : value; +} + function compare(actual: unknown, expected: unknown): number | null { const integers = compareIntegers(actual, expected); if (integers !== null) return integers; @@ -146,6 +156,17 @@ function compare(actual: unknown, expected: unknown): number | null { const expectedTs = toTimestamp(expected); if (actualTs !== null && expectedTs !== null) return actualTs - expectedTs; + // Without this, "is greater than" on a Time column can never be met: the builder offers the + // operator, and the step would route to the fallback on every record without saying why. + const actualTime = toTimeOfDay(actual); + const expectedTime = toTimeOfDay(expected); + + if (actualTime !== null && expectedTime !== null) { + if (actualTime === expectedTime) return 0; + + return actualTime > expectedTime ? 1 : -1; + } + return null; } 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 d28c3b3d80..8f2f96874b 100644 --- a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -169,6 +169,29 @@ describe('evaluateOperator', () => { }); }); + describe('times of day (Time columns)', () => { + // The builder offers "is greater than" on a Time column, so an unordered time of day would + // route every record to the fallback with nothing in the trace to explain it. + it('orders a time of day', () => { + expect(ev('greater_than', '08:30:00', '07:00:00')).toBe(true); + expect(ev('greater_than', '08:30:00', '09:00:00')).toBe(false); + expect(ev('less_than', '08:30:00', '09:00:00')).toBe(true); + expect(ev('less_than', '23:59:59', '00:00:00')).toBe(false); + }); + + it('fills in the seconds rather than parsing, so 08:30 and 08:30:00 are one instant', () => { + expect(ev('greater_than', '08:30', '08:30:00')).toBe(false); + expect(ev('less_than', '08:30', '08:30:00')).toBe(false); + expect(ev('greater_than', '08:31', '08:30:59')).toBe(true); + }); + + it('is not met against something that is not a time of day', () => { + expect(ev('greater_than', '08:30:00', 7)).toBe(false); + expect(ev('greater_than', '08:30:00', 'morning')).toBe(false); + expect(ev('greater_than', '2026-09-04T08:30:00Z', '07:00:00')).toBe(false); + }); + }); + describe('date comparisons', () => { it('compares ISO strings as timestamps when both sides parse', () => { expect(ev('greater_than', '2026-02-01', '2026-01-01')).toBe(true); From 01e2b138132c953d12cb2c664ba3cbb7f6f47435 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 7 Sep 2026 15:05:47 +0200 Subject: [PATCH 07/10] fix(workflow-executor): read 08:30 and 08:30:00 as the same time of day The builder's time widget writes the hour and minutes only while a Time column holds the seconds too, so is, is not and is in on a Time column could never be met from the editor. --- .../deterministic-condition-evaluator.ts | 21 ++++++++++++------- .../deterministic-condition-evaluator.test.ts | 5 +++++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts index 90d2f63308..7af7d4d4d9 100644 --- a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts +++ b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts @@ -108,6 +108,14 @@ function compareIntegers(actual: unknown, expected: unknown): number | null { return actualInteger > expectedInteger ? 1 : -1; } +// Zero padding makes a time of day sort chronologically as text, so the seconds are filled in +// rather than parsed: "08:30" and "08:30:00" are the same instant of the day and must compare equal. +function toTimeOfDay(value: unknown): string | null { + if (typeof value !== 'string' || !TIME_OF_DAY.test(value)) return null; + + return value.length === 5 ? `${value}:00` : value; +} + function scalarEqual(actual: unknown, expected: unknown): boolean | null { if (actual === expected) return true; @@ -121,6 +129,11 @@ function scalarEqual(actual: unknown, expected: unknown): boolean | null { const expectedTs = toTimestamp(expected); if (actualTs !== null && expectedTs !== null) return actualTs === expectedTs; + // The builder's time widget writes "08:30" while the column holds "08:30:00". + const actualTime = toTimeOfDay(actual); + const expectedTime = toTimeOfDay(expected); + if (actualTime !== null && expectedTime !== null) return actualTime === expectedTime; + return typeof actual === typeof expected ? false : null; } @@ -137,14 +150,6 @@ function isEqual(actual: unknown, expected: unknown): boolean | null { return scalarEqual(actual, expected); } -// Zero padding makes a time of day sort chronologically as text, so the seconds are filled in -// rather than parsed: "08:30" and "08:30:00" are the same instant of the day and must compare equal. -function toTimeOfDay(value: unknown): string | null { - if (typeof value !== 'string' || !TIME_OF_DAY.test(value)) return null; - - return value.length === 5 ? `${value}:00` : value; -} - function compare(actual: unknown, expected: unknown): number | null { const integers = compareIntegers(actual, expected); if (integers !== null) return integers; 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 8f2f96874b..fbcd3a3155 100644 --- a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -179,7 +179,12 @@ describe('evaluateOperator', () => { expect(ev('less_than', '23:59:59', '00:00:00')).toBe(false); }); + // The builder's time widget writes "08:30" while the column holds "08:30:00": without the + // seconds filled in, "is" on a Time column could never be met from the editor. it('fills in the seconds rather than parsing, so 08:30 and 08:30:00 are one instant', () => { + expect(ev('equal', '08:30:00', '08:30')).toBe(true); + expect(ev('not_equal', '08:30:00', '08:30')).toBe(false); + expect(ev('in', '08:30:00', ['07:00', '08:30'])).toBe(true); expect(ev('greater_than', '08:30', '08:30:00')).toBe(false); expect(ev('less_than', '08:30', '08:30:00')).toBe(false); expect(ev('greater_than', '08:31', '08:30:59')).toBe(true); From 949ca0a6027de11602478a063128fdeadd23f164 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 7 Sep 2026 20:43:19 +0200 Subject: [PATCH 08/10] feat(workflow-executor): evaluate the calendar period operators too previous week, month, quarter and year, and the same four up to the clock, with the toolkit's bounds: the whole previous period end excluded, or the current one from its start to now. --- packages/workflow-executor/CLAUDE.md | 4 +- .../deterministic-condition-evaluator.ts | 41 ++++++++++++++++++- .../src/types/validated/step-definition.ts | 16 ++++++++ .../deterministic-condition-evaluator.test.ts | 37 +++++++++++++++++ 4 files changed, 95 insertions(+), 3 deletions(-) diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index 81fb6a6a7e..c69746e66d 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -38,8 +38,8 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p - **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`. **No condition evaluation ever fails the step, data or reference alike**: a resolved value that is null is `met: null`, and a reference that read nothing is `met: null` + a `reason` (`source-step-not-reached` when no Get Data with that id ran on this path, `field-not-loaded` when one ran but the field is absent or errored), plus a `Warn`. Build-time validation cannot cover the second case (a Get Data step may let the AI pick its fields), so the run has to answer at runtime — and the way a decision step answers is by routing, unlike record steps (line above) which die on an unresolvable reference. What stops the fallback from passing for a decision the data took is the `reason`: the run view words it for the operator instead of showing a bare cross. "Never read" is not "read and empty", so `present`/`blank` get no answer out of it either. 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`. - - **Operator set = what a list view filter offers** (PRD-1147), 23 names in `CONDITION_OPERATORS`; nothing the front registry cannot render (so no `>=`, `<=`, `not_in`). 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`), whole numbers exactly as BigInt; 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); text operators are strings-only, case sensitive except `i_contains`, which folds case and keeps accents like the list filter's Postgres `ILIKE` path (other dialects' collations diverge; Postgres is the contract). - - **Relative dates read an injected `Clock`** (`{ now, timezone }`), never the machine's: the step executor builds one per step (so every condition of every option sees the same instant) from `ExecutionContext.timezone`, which the orchestrator sends as the **project's** zone (`AvailableStepExecution.timezone`, mapper falls back to `UTC` when unset, absent or not an IANA name). **`UTC` is the main path, not an edge case**: 0.7% of projects carry a timezone (635/85457, measured September 2026), so a Decision's "today" is usually UTC's day while the list filter shows the browser's. The machine's zone is never used: the fleet runs several instances, and the same run must route the same on each. Day windows (`today`, `yesterday`, `previous_x_days`, `previous_x_days_to_date`) are computed in that zone with the exact bounds of `datasource-toolkit`'s time transforms: end excluded, start included for a calendar date and excluded for a datetime (the toolkit emits `GreaterThanOrEqual` for Dateonly, `GreaterThan` otherwise), and a mid-day bound (`past`, `before_x_hours_ago`, the end of `previous_x_days_to_date`) read at the start of its day for a calendar date (the toolkit formats the bound with `toISODate` for a Dateonly column), so a record answers "today" the same way in a Decision and in a list filter; a **calendar date** (Dateonly) is read at midnight in that zone, otherwise Honolulu's own "today" would count as yesterday. An unknown zone name falls back to `UTC` in the mapper, like an absent one. The trace persists `evaluatedAt` + `timezone` alongside `evaluations`, since a relative condition cannot be explained later without the instant it was read at. A retry re-evaluates with a fresh clock and may route differently: accepted, by product decision. + - **Operator set = what a list view filter offers** (PRD-1147), 31 names in `CONDITION_OPERATORS`; nothing the front registry cannot render (so no `>=`, `<=`, `not_in`). 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`), whole numbers exactly as BigInt; 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); text operators are strings-only, case sensitive except `i_contains`, which folds case and keeps accents like the list filter's Postgres `ILIKE` path (other dialects' collations diverge; Postgres is the contract). + - **Relative dates read an injected `Clock`** (`{ now, timezone }`), never the machine's: the step executor builds one per step (so every condition of every option sees the same instant) from `ExecutionContext.timezone`, which the orchestrator sends as the **project's** zone (`AvailableStepExecution.timezone`, mapper falls back to `UTC` when unset, absent or not an IANA name). **`UTC` is the main path, not an edge case**: 0.7% of projects carry a timezone (635/85457, measured September 2026), so a Decision's "today" is usually UTC's day while the list filter shows the browser's. The machine's zone is never used: the fleet runs several instances, and the same run must route the same on each. Day and calendar-period windows (`today`, `yesterday`, `previous_x_days`, `previous_x_days_to_date`, and `previous_week`/`month`/`quarter`/`year` with their `_to_date` forms, which read the whole previous period or the current one up to the clock) are computed in that zone with the exact bounds of `datasource-toolkit`'s time transforms: end excluded, start included for a calendar date and excluded for a datetime (the toolkit emits `GreaterThanOrEqual` for Dateonly, `GreaterThan` otherwise), and a mid-day bound (`past`, `before_x_hours_ago`, the end of `previous_x_days_to_date`) read at the start of its day for a calendar date (the toolkit formats the bound with `toISODate` for a Dateonly column), so a record answers "today" the same way in a Decision and in a list filter; a **calendar date** (Dateonly) is read at midnight in that zone, otherwise Honolulu's own "today" would count as yesterday. An unknown zone name falls back to `UTC` in the mapper, like an absent one. The trace persists `evaluatedAt` + `timezone` alongside `evaluations`, since a relative condition cannot be explained later without the instant it was read at. A retry re-evaluates with a fresh clock and may route differently: accepted, by product decision. - **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. All four record steps pin the source by `selectedRecordStepId` — a **stable BPMN step id** (or the `WORKFLOW_START_STEP_ID` sentinel) resolved by `resolveSourceRecordRef`, chosen to survive the index shifts a revision causes; the editor writes it and treats it as a precondition for choosing fields. Presence, not truthiness, decides whether something is pinned -- `selectedRecordStepId`, `actionName` and `relationName` all check `!== undefined`. An empty value is a pin that lost its target, so it resolves to nothing and errors instead of falling back to the AI, which would silently pick a different record, run a different action, or follow a different relation than the step was configured to. Not reachable from the editor, which writes `stepId || undefined`. `selectedRecordStepIndex` (a runtime index, resolved in `resolveRecordRef`) survives only as a fallback on read-record/update-record, checked after the step id. Partial args supported. Unresolvable → `PinnedArgNotFoundError` when the name was pinned (`configuration`), `FieldNotFoundError`/`ActionNotFoundError`/`RelationNotFoundError` when the AI chose it (**unclassified** — a re-run may resolve differently, so nothing permanent can be asserted); bad shape / out-of-range index → `InvalidPreRecordedArgsError`. The split exists because the AI-facing messages tell the operator to rephrase the prompt — the right remedy for a name the AI chose, and unreachable for one the workflow fixed, where the step itself is the thing to edit. One class covers every pinned kind: the diagnosis differs per kind, the operator's remedy does not. Anything raised on a pinned value has to name the step. diff --git a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts index 7af7d4d4d9..ac35cbd00f 100644 --- a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts +++ b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts @@ -235,12 +235,43 @@ function days(value: unknown): number | null { return count !== null && Number.isInteger(count) && count > 0 ? count : null; } +// A calendar period: the whole previous one, or this one up to the clock. Luxon's week starts on +// Monday, like the toolkit's, so both agree on which week a Sunday belongs to. +const previousPeriod = + (unit: 'week' | 'month' | 'quarter' | 'year'): Window => + now => + [now.minus({ [unit]: 1 }).startOf(unit), now.startOf(unit)]; + +const periodToDate = + (unit: 'week' | 'month' | 'quarter' | 'year'): Window => + now => + [now.startOf(unit), now]; + const WINDOWS: Record< - 'today' | 'yesterday' | 'previous_x_days' | 'previous_x_days_to_date', + | 'today' + | 'yesterday' + | 'previous_x_days' + | 'previous_x_days_to_date' + | 'previous_week' + | 'previous_month' + | 'previous_quarter' + | 'previous_year' + | 'previous_week_to_date' + | 'previous_month_to_date' + | 'previous_quarter_to_date' + | 'previous_year_to_date', Window > = { today: now => [now.startOf('day'), now.plus({ days: 1 }).startOf('day')], yesterday: now => [now.minus({ days: 1 }).startOf('day'), now.startOf('day')], + previous_week: previousPeriod('week'), + previous_month: previousPeriod('month'), + previous_quarter: previousPeriod('quarter'), + previous_year: previousPeriod('year'), + previous_week_to_date: periodToDate('week'), + previous_month_to_date: periodToDate('month'), + previous_quarter_to_date: periodToDate('quarter'), + previous_year_to_date: periodToDate('year'), previous_x_days: (now, value) => { const count = days(value); @@ -323,6 +354,14 @@ const EVALUATORS: Record< yesterday: within('yesterday'), previous_x_days: within('previous_x_days'), previous_x_days_to_date: within('previous_x_days_to_date'), + previous_week: within('previous_week'), + previous_month: within('previous_month'), + previous_quarter: within('previous_quarter'), + previous_year: within('previous_year'), + previous_week_to_date: within('previous_week_to_date'), + previous_month_to_date: within('previous_month_to_date'), + previous_quarter_to_date: within('previous_quarter_to_date'), + previous_year_to_date: within('previous_year_to_date'), }; /** diff --git a/packages/workflow-executor/src/types/validated/step-definition.ts b/packages/workflow-executor/src/types/validated/step-definition.ts index 2e56665825..5fe930007a 100644 --- a/packages/workflow-executor/src/types/validated/step-definition.ts +++ b/packages/workflow-executor/src/types/validated/step-definition.ts @@ -68,6 +68,14 @@ export const CONDITION_OPERATORS = [ 'previous_x_days_to_date', 'before_x_hours_ago', 'after_x_hours_ago', + 'previous_week', + 'previous_month', + 'previous_quarter', + 'previous_year', + 'previous_week_to_date', + 'previous_month_to_date', + 'previous_quarter_to_date', + 'previous_year_to_date', ] as const; export type ConditionOperator = (typeof CONDITION_OPERATORS)[number]; @@ -78,6 +86,14 @@ const VALUE_LESS_OPERATORS: readonly ConditionOperator[] = [ 'future', 'today', 'yesterday', + 'previous_week', + 'previous_month', + 'previous_quarter', + 'previous_year', + 'previous_week_to_date', + 'previous_month_to_date', + 'previous_quarter_to_date', + 'previous_year_to_date', ]; const DeterministicConditionSchema = z 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 fbcd3a3155..7500579a69 100644 --- a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -501,6 +501,43 @@ describe('evaluateOperator', () => { expect(ev('previous_x_days', '2026-09-01T12:00:00Z', 'seven')).toBe(false); }); + // Whole calendar periods, and the current one up to the clock. The clock is Friday 4 September + // 2026, 12:30 in Paris: that week starts Monday 31 August, the quarter on 1 July. + it('reads the previous calendar period, end excluded', () => { + expect(ev('previous_week', '2026-08-28T12:00:00Z')).toBe(true); + expect(ev('previous_week', '2026-08-31T12:00:00Z')).toBe(false); + expect(ev('previous_month', '2026-08-15T12:00:00Z')).toBe(true); + expect(ev('previous_month', '2026-09-01T12:00:00Z')).toBe(false); + expect(ev('previous_quarter', '2026-05-15T12:00:00Z')).toBe(true); + expect(ev('previous_quarter', '2026-07-01T12:00:00Z')).toBe(false); + expect(ev('previous_year', '2025-06-15T12:00:00Z')).toBe(true); + expect(ev('previous_year', '2026-01-01T12:00:00Z')).toBe(false); + }); + + it('reads the current calendar period up to the clock, not past it', () => { + expect(ev('previous_week_to_date', '2026-08-31T12:00:00Z')).toBe(true); + expect(ev('previous_week_to_date', '2026-08-30T12:00:00Z')).toBe(false); + expect(ev('previous_week_to_date', '2026-09-04T11:00:00Z')).toBe(false); + expect(ev('previous_month_to_date', '2026-09-02T12:00:00Z')).toBe(true); + expect(ev('previous_month_to_date', '2026-08-31T12:00:00Z')).toBe(false); + expect(ev('previous_quarter_to_date', '2026-07-01T12:00:00Z')).toBe(true); + expect(ev('previous_quarter_to_date', '2026-06-30T12:00:00Z')).toBe(false); + expect(ev('previous_year_to_date', '2026-02-01T12:00:00Z')).toBe(true); + expect(ev('previous_year_to_date', '2025-12-31T12:00:00Z')).toBe(false); + }); + + // Monday, like the toolkit: a Sunday belongs to the week that started six days earlier. + it('starts a week on Monday', () => { + expect(ev('previous_week', '2026-08-30T12:00:00Z')).toBe(true); + expect(ev('previous_week_to_date', '2026-08-31T00:00:01Z')).toBe(true); + }); + + it('reads a calendar date in a period window too', () => { + expect(ev('previous_month', '2026-08-15')).toBe(true); + expect(ev('previous_month', '2026-09-01')).toBe(false); + expect(ev('previous_month_to_date', '2026-09-01')).toBe(true); + }); + // A calendar date has no instant: read as UTC midnight it would fall before Honolulu's day // even started, and the record's own "today" would be counted as yesterday. it('reads a calendar date in the clock timezone', () => { From 2144fcfa7017f8f432f492495bd72c0b41bf2ae6 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 7 Sep 2026 21:23:48 +0200 Subject: [PATCH 09/10] feat(workflow-executor): evaluate not_in on tri-state membership The Filters registry gained is not in (#9957), so the Decision offers it too. Membership answers true, false or not-comparable, so a value the members cannot be compared to does not satisfy the negated operator. --- packages/workflow-executor/CLAUDE.md | 2 +- .../deterministic-condition-evaluator.ts | 17 ++++++++++---- .../src/types/validated/step-definition.ts | 1 + .../deterministic-condition-evaluator.test.ts | 23 +++++++++++++++++++ .../test/types/step-definition.test.ts | 6 +++-- 5 files changed, 42 insertions(+), 7 deletions(-) diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index c69746e66d..6c33a56dc4 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -38,7 +38,7 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p - **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`. **No condition evaluation ever fails the step, data or reference alike**: a resolved value that is null is `met: null`, and a reference that read nothing is `met: null` + a `reason` (`source-step-not-reached` when no Get Data with that id ran on this path, `field-not-loaded` when one ran but the field is absent or errored), plus a `Warn`. Build-time validation cannot cover the second case (a Get Data step may let the AI pick its fields), so the run has to answer at runtime — and the way a decision step answers is by routing, unlike record steps (line above) which die on an unresolvable reference. What stops the fallback from passing for a decision the data took is the `reason`: the run view words it for the operator instead of showing a bare cross. "Never read" is not "read and empty", so `present`/`blank` get no answer out of it either. 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`. - - **Operator set = what a list view filter offers** (PRD-1147), 31 names in `CONDITION_OPERATORS`; nothing the front registry cannot render (so no `>=`, `<=`, `not_in`). 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`), whole numbers exactly as BigInt; 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); text operators are strings-only, case sensitive except `i_contains`, which folds case and keeps accents like the list filter's Postgres `ILIKE` path (other dialects' collations diverge; Postgres is the contract). + - **Operator set = what a list view filter offers** (PRD-1147), 32 names in `CONDITION_OPERATORS`; nothing the front registry cannot render (so no `>=`, `<=`). Membership is tri-state (`memberOf`), so `not_in` cannot claim "not a member" about a comparison it never managed to make. 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`), whole numbers exactly as BigInt; 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); text operators are strings-only, case sensitive except `i_contains`, which folds case and keeps accents like the list filter's Postgres `ILIKE` path (other dialects' collations diverge; Postgres is the contract). - **Relative dates read an injected `Clock`** (`{ now, timezone }`), never the machine's: the step executor builds one per step (so every condition of every option sees the same instant) from `ExecutionContext.timezone`, which the orchestrator sends as the **project's** zone (`AvailableStepExecution.timezone`, mapper falls back to `UTC` when unset, absent or not an IANA name). **`UTC` is the main path, not an edge case**: 0.7% of projects carry a timezone (635/85457, measured September 2026), so a Decision's "today" is usually UTC's day while the list filter shows the browser's. The machine's zone is never used: the fleet runs several instances, and the same run must route the same on each. Day and calendar-period windows (`today`, `yesterday`, `previous_x_days`, `previous_x_days_to_date`, and `previous_week`/`month`/`quarter`/`year` with their `_to_date` forms, which read the whole previous period or the current one up to the clock) are computed in that zone with the exact bounds of `datasource-toolkit`'s time transforms: end excluded, start included for a calendar date and excluded for a datetime (the toolkit emits `GreaterThanOrEqual` for Dateonly, `GreaterThan` otherwise), and a mid-day bound (`past`, `before_x_hours_ago`, the end of `previous_x_days_to_date`) read at the start of its day for a calendar date (the toolkit formats the bound with `toISODate` for a Dateonly column), so a record answers "today" the same way in a Decision and in a list filter; a **calendar date** (Dateonly) is read at midnight in that zone, otherwise Honolulu's own "today" would count as yesterday. An unknown zone name falls back to `UTC` in the mapper, like an absent one. The trace persists `evaluatedAt` + `timezone` alongside `evaluations`, since a relative condition cannot be explained later without the instant it was read at. A retry re-evaluates with a fresh clock and may route differently: accepted, by product decision. - **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. diff --git a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts index ac35cbd00f..ed69a06501 100644 --- a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts +++ b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts @@ -183,15 +183,23 @@ function isPresent(value: unknown): boolean { return true; } -function isMemberOf(list: unknown, candidate: unknown): boolean { - return Array.isArray(list) && list.some(item => scalarEqual(item, candidate) === true); +// Tri-state like scalarEqual: null = the candidate could not be compared to any member, so a +// negated membership test cannot claim "not a member" about a comparison it never managed to make. +// An empty list compares nothing and mismatches nothing, so it stays a plain false. +function memberOf(list: unknown, candidate: unknown): boolean | null { + if (!Array.isArray(list)) return null; + + const results = list.map(item => scalarEqual(item, candidate)); + if (results.includes(true)) return true; + + return results.includes(null) ? null : false; } function includesAll(actual: unknown, expected: unknown): boolean { if (!Array.isArray(actual)) return false; const wanted = Array.isArray(expected) ? expected : [expected]; - return wanted.length > 0 && wanted.every(item => isMemberOf(actual, item)); + return wanted.length > 0 && wanted.every(item => memberOf(actual, item) === true); } function stringTest(satisfies: (actual: string, expected: string) => boolean) { @@ -327,7 +335,8 @@ const EVALUATORS: Record< not_equal: (actual, expected) => isEqual(actual, expected) === false, greater_than: ordering(diff => diff > 0), less_than: ordering(diff => diff < 0), - in: (actual, expected) => isMemberOf(expected, actual), + in: (actual, expected) => memberOf(expected, actual) === true, + not_in: (actual, expected) => memberOf(expected, actual) === false, includes_all: includesAll, contains: stringTest((actual, expected) => actual.includes(expected)), not_contains: stringTest((actual, expected) => !actual.includes(expected)), diff --git a/packages/workflow-executor/src/types/validated/step-definition.ts b/packages/workflow-executor/src/types/validated/step-definition.ts index 5fe930007a..a9011ecc7f 100644 --- a/packages/workflow-executor/src/types/validated/step-definition.ts +++ b/packages/workflow-executor/src/types/validated/step-definition.ts @@ -52,6 +52,7 @@ export const CONDITION_OPERATORS = [ 'greater_than', 'less_than', 'in', + 'not_in', 'includes_all', 'contains', 'not_contains', 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 7500579a69..72925595c7 100644 --- a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -307,6 +307,29 @@ describe('evaluateOperator', () => { }); }); + describe('not_in', () => { + it('is met when the value is comparable to the members and absent from them', () => { + expect(ev('not_in', 'closed', ['active', 'pending'])).toBe(true); + expect(ev('not_in', 'active', ['active', 'pending'])).toBe(false); + expect(ev('not_in', 50, [150, 250])).toBe(true); + expect(ev('not_in', '150', [150, 250])).toBe(false); + }); + + // The whole reason membership is tri-state: "not a member" must not be claimed about a + // comparison that never happened, or a broken config would satisfy the negated operator. + it('is not met when the value cannot be compared to the members', () => { + expect(ev('not_in', 150, ['active', 'pending'])).toBe(false); + expect(ev('not_in', 'active', [150, 250])).toBe(false); + expect(ev('not_in', 'active', 'active')).toBe(false); + }); + + // An empty list compares nothing and mismatches nothing, so nothing is a member of it. + it('is met against an empty list', () => { + expect(ev('not_in', 'active', [])).toBe(true); + expect(ev('in', 'active', [])).toBe(false); + }); + }); + describe('contains', () => { it('matches a substring on strings', () => { expect(ev('contains', 'hello world', 'world')).toBe(true); diff --git a/packages/workflow-executor/test/types/step-definition.test.ts b/packages/workflow-executor/test/types/step-definition.test.ts index 7baf0bd844..028532ce3c 100644 --- a/packages/workflow-executor/test/types/step-definition.test.ts +++ b/packages/workflow-executor/test/types/step-definition.test.ts @@ -90,8 +90,9 @@ describe('ConditionStepDefinitionSchema deterministic conditions', () => { }); // value supplied so the failure is the operator enum, not the value-less-operator refinement. - // The last three were in the contract before PRD-1147 and are refused since. - it.each(['ilike', 'not_in', 'greater_than_or_equal', 'less_than_or_equal'])( + // The last two were in the contract before PRD-1147 and are refused since, having no Filters + // operator for the builder to render. + it.each(['ilike', 'greater_than_or_equal', 'less_than_or_equal'])( 'rejects the unknown operator "%s" at the schema boundary', operator => { const result = ConditionStepDefinitionSchema.safeParse({ @@ -218,6 +219,7 @@ describe('ConditionStepDefinitionSchema deterministic conditions', () => { 'greater_than', 'less_than', 'in', + 'not_in', 'includes_all', 'contains', 'not_contains', From a1a8fc69b4b08da60ebba69eb5f1ae624999e9df Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 7 Sep 2026 21:56:49 +0200 Subject: [PATCH 10/10] fix(workflow-executor): read a time of day as milliseconds, not as text Comparing the strings ordered 08:30:00.10 before 08:30:00.1, the same instant written twice, and left equality to a padding rule. One parse now backs equality and ordering, and a time nobody can be at is refused. --- .../deterministic-condition-evaluator.ts | 41 +++++++++++-------- .../deterministic-condition-evaluator.test.ts | 16 ++++++++ 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts index ed69a06501..67c7d0efbc 100644 --- a/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts +++ b/packages/workflow-executor/src/executors/deterministic-condition-evaluator.ts @@ -16,8 +16,10 @@ 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+)?$/; -// A Time column holds a zero-padded time of day, which nothing else in this file can order. -const TIME_OF_DAY = /^\d{2}:\d{2}(:\d{2}(\.\d+)?)?$/; +// A Time column holds a time of day, which nothing else in this file can read: not a number, not +// an instant. The seconds and their fraction are optional, and both sides may write them +// differently — Postgres returns "08:30:00", the builder's time widget writes "08:30". +const TIME_OF_DAY = /^(\d{2}):(\d{2})(?::(\d{2})(\.\d+)?)?$/; const INTEGER_STRING = /^-?\d+$/; // Date.parse rolls an out-of-range day over ("2026-02-30" becomes 2026-03-02), which would make a @@ -108,12 +110,21 @@ function compareIntegers(actual: unknown, expected: unknown): number | null { return actualInteger > expectedInteger ? 1 : -1; } -// Zero padding makes a time of day sort chronologically as text, so the seconds are filled in -// rather than parsed: "08:30" and "08:30:00" are the same instant of the day and must compare equal. -function toTimeOfDay(value: unknown): string | null { - if (typeof value !== 'string' || !TIME_OF_DAY.test(value)) return null; +// Milliseconds since midnight rather than text: comparing the strings would order "08:30:00.10" +// before "08:30:00.1", which is the same instant written twice. +function toMillisOfDay(value: unknown): number | null { + if (typeof value !== 'string') return null; - return value.length === 5 ? `${value}:00` : value; + const parts = TIME_OF_DAY.exec(value); + if (!parts) return null; + + const [, hours, minutes, seconds = '0', fraction = '.0'] = parts; + if (Number(hours) > 23 || Number(minutes) > 59 || Number(seconds) > 59) return null; + + return ( + ((Number(hours) * 60 + Number(minutes)) * 60 + Number(seconds)) * 1000 + + Math.round(Number(`0${fraction}`) * 1000) + ); } function scalarEqual(actual: unknown, expected: unknown): boolean | null { @@ -129,9 +140,8 @@ function scalarEqual(actual: unknown, expected: unknown): boolean | null { const expectedTs = toTimestamp(expected); if (actualTs !== null && expectedTs !== null) return actualTs === expectedTs; - // The builder's time widget writes "08:30" while the column holds "08:30:00". - const actualTime = toTimeOfDay(actual); - const expectedTime = toTimeOfDay(expected); + const actualTime = toMillisOfDay(actual); + const expectedTime = toMillisOfDay(expected); if (actualTime !== null && expectedTime !== null) return actualTime === expectedTime; return typeof actual === typeof expected ? false : null; @@ -163,14 +173,9 @@ function compare(actual: unknown, expected: unknown): number | null { // Without this, "is greater than" on a Time column can never be met: the builder offers the // operator, and the step would route to the fallback on every record without saying why. - const actualTime = toTimeOfDay(actual); - const expectedTime = toTimeOfDay(expected); - - if (actualTime !== null && expectedTime !== null) { - if (actualTime === expectedTime) return 0; - - return actualTime > expectedTime ? 1 : -1; - } + const actualTime = toMillisOfDay(actual); + const expectedTime = toMillisOfDay(expected); + if (actualTime !== null && expectedTime !== null) return actualTime - expectedTime; return null; } 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 72925595c7..1b327730d6 100644 --- a/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts +++ b/packages/workflow-executor/test/executors/deterministic-condition-evaluator.test.ts @@ -190,6 +190,22 @@ describe('evaluateOperator', () => { expect(ev('greater_than', '08:31', '08:30:59')).toBe(true); }); + // Text comparison would order "08:30:00.10" before "08:30:00.1", the same instant written twice. + it('reads the fraction of a second as a value, not as text', () => { + expect(ev('less_than', '08:30:00.1', '08:30:00.10')).toBe(false); + expect(ev('greater_than', '08:30:00.1', '08:30:00.10')).toBe(false); + expect(ev('equal', '08:30:00.1', '08:30:00.100')).toBe(true); + expect(ev('less_than', '08:30:00.09', '08:30:00.1')).toBe(true); + expect(ev('greater_than', '08:30:00.2', '08:30:00.19')).toBe(true); + }); + + it('is not met on a time nobody can be at', () => { + expect(ev('equal', '24:00:00', '24:00:00')).toBe(true); + expect(ev('greater_than', '25:00:00', '08:00:00')).toBe(false); + expect(ev('greater_than', '08:70:00', '08:00:00')).toBe(false); + expect(ev('greater_than', '08:30:70', '08:00:00')).toBe(false); + }); + it('is not met against something that is not a time of day', () => { expect(ev('greater_than', '08:30:00', 7)).toBe(false); expect(ev('greater_than', '08:30:00', 'morning')).toBe(false);