diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index 49e30e2fe3..3cefa25014 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -36,7 +36,7 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p `StepExecutionMode` (domain enum, mapped from the server contract in `step-definition-mapper.ts`): `Manual`, `AutomatedWithConfirmation`, `FullyAutomated`. - **Trigger Action** (`trigger-record-action-step-executor.ts`, `handleFirstCall`) — detects the form via `getActionForm` (full field list, not `getActionFormInfo`). Formless: `FullyAutomated` runs it *in the executor* via the audited agent; otherwise pauses. With a form: `Manual` pauses with the native form (no AI fill); `AutomatedWithConfirmation` AI-fills then pauses for the user to submit natively; `FullyAutomated` AI-fills (`fillFormWithAi`) and, if `filledForm.canExecute`, submits in the executor — falling back to `awaiting-input` (pause) when required fields are missing, or on `ActionFormValidationError`/`ActionRequiresApprovalError` (a human can finish those). `UnsupportedActionFormError` is declared/exported but **never thrown** in src. -- **Pre-recorded args** — record steps accept `preRecordedArgs` to skip AI. Technical names (`fieldName`/`fieldNames`/`actionName`/`relationName`) are matched exactly via `findFieldByTechnicalName` (no fuzz); `resolveAiFieldName` (exact-then-normalized) is reserved for AI-returned display names. The source record differs by step: read-record/update-record use `selectedRecordStepIndex` (a runtime index, resolved in `resolveRecordRef`); trigger-action/load-related-record use `selectedRecordStepId` — a **stable BPMN step id** (or `WORKFLOW_START_STEP_ID` sentinel) resolved by `resolveSourceRecordRef`, chosen to survive the index shifts a revision causes. Partial args supported. Unresolvable → `FieldNotFoundError`/`ActionNotFoundError`/`RelationNotFoundError`; bad shape / out-of-range index → `InvalidPreRecordedArgsError`. +- **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. `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 → `FieldNotFoundError`/`ActionNotFoundError`/`RelationNotFoundError`; bad shape / out-of-range index → `InvalidPreRecordedArgsError`. ## Invariants (read before changing executors) @@ -45,6 +45,9 @@ Front ◀──▶ Orchestrator ◀──pull/push──▶ Executor (this p - *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. + - `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 who has to act on a step. They are deliberately separate vocabularies — don't map one onto the other. - **`displayName` vs technical name** — AI tools/prompts use `displayName` (the admin-configured label end users write against), never `fieldName`. Map AI-returned display names back to technical names before any datasource op. - **Idempotency (mutating steps: update-record, trigger-action, mcp)** — write-ahead log in the RunStore: save `idempotencyPhase: 'executing'` before the side effect, `'done'` + `executionResult` after. On re-dispatch `(runId, stepIndex)`: `done` → rebuild success outcome without re-running or re-logging; `executing` → throw `StepStateError`. `checkIdempotency()` runs before `doExecute()`; the `executing` marker is set in the `beforeCall` thunk passed to `AgentWithLog` (after `createPending`) so a log-creation failure leaves no orphan marker. Non-mutating steps don't override it (replay is safe). - **Fetched steps must execute** — any step from `getAvailableRuns()` must run; silently dropping one breaks the orchestrator contract. The only allowed pre-filter is `inFlightRuns` dedup (keyed by `runId`, not step — a chain advances `stepId`). 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 36d0d0e4ae..942bd28899 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 @@ -27,7 +27,11 @@ import { type Step, type StepUser, } from '../types/validated/execution'; -import { stepTypeToOutcomeType } from '../types/validated/step-outcome'; +import { + ErrorKindSchema, + ErrorSourceStepIndexSchema, + stepTypeToOutcomeType, +} from '../types/validated/step-outcome'; function toRecordStatus(ctxStatus: unknown): RecordStepOutcome['status'] { if (ctxStatus === 'error') return 'error'; @@ -44,10 +48,17 @@ function toStepOutcome(s: ServerStepHistory): StepOutcome { const outcomeType = stepTypeToOutcomeType(stepDef.type); const ctx = (s.context ?? {}) as Record; + // A value the executor didn't write (legacy frontend, or a newer executor's vocabulary) is dropped + // rather than passed on: AvailableStepExecutionSchema.parse below would fail the whole run. + const parsedErrorKind = ErrorKindSchema.safeParse(ctx.errorKind); + const parsedSourceStepIndex = ErrorSourceStepIndexSchema.safeParse(ctx.errorSourceStepIndex); + const baseFromCtx = { stepId: s.stepName, stepIndex: s.stepIndex, error: typeof ctx.error === 'string' ? ctx.error : undefined, + ...(parsedErrorKind.success && { errorKind: parsedErrorKind.data }), + ...(parsedSourceStepIndex.success && { errorSourceStepIndex: parsedSourceStepIndex.data }), }; if (outcomeType === 'condition') { diff --git a/packages/workflow-executor/src/adapters/step-outcome-to-update-step-mapper.ts b/packages/workflow-executor/src/adapters/step-outcome-to-update-step-mapper.ts index 3965001a14..a672a0fb70 100644 --- a/packages/workflow-executor/src/adapters/step-outcome-to-update-step-mapper.ts +++ b/packages/workflow-executor/src/adapters/step-outcome-to-update-step-mapper.ts @@ -27,6 +27,12 @@ export default function toUpdateStepRequest( ): ServerUpdateStepRequest { const context: Record = { status: outcome.status }; if (outcome.error !== undefined) context.error = outcome.error; + if (outcome.errorKind !== undefined) context.errorKind = outcome.errorKind; + + // Index 0 is a real step, so this cannot be a truthiness check. + if (outcome.errorSourceStepIndex !== undefined) { + context.errorSourceStepIndex = outcome.errorSourceStepIndex; + } if (outcome.type === 'condition' && outcome.selectedOption !== undefined) { context.selectedOption = outcome.selectedOption; diff --git a/packages/workflow-executor/src/errors.ts b/packages/workflow-executor/src/errors.ts index 657afc5228..ebbced99c3 100644 --- a/packages/workflow-executor/src/errors.ts +++ b/packages/workflow-executor/src/errors.ts @@ -1,7 +1,7 @@ /* eslint-disable max-classes-per-file */ import type { MalformedRunInfo } from './ports/workflow-port'; import type { RecordId } from './types/validated/collection'; -import type { AwaitingInputReason } from './types/validated/step-outcome'; +import type { AwaitingInputReason, ErrorKind } from './types/validated/step-outcome'; import type { z } from 'zod'; export function causeMessage(error: unknown): string | undefined { @@ -30,10 +30,19 @@ export abstract class WorkflowExecutorError extends Error { readonly userMessage: string; cause?: unknown; + // The kind of failure, declared once by each family below via defaultErrorKind. The throw site + // overrides it only where the same error can be either kind depending on why it was raised. + errorKind?: ErrorKind; + static readonly defaultErrorKind?: ErrorKind; + + // Set when the error is about a different step than the one being executed. + errorSourceStepIndex?: number; + constructor(message: string, userMessage?: string) { super(message); this.name = this.constructor.name; this.userMessage = userMessage ?? message; + this.errorKind = (this.constructor as typeof WorkflowExecutorError).defaultErrorKind; } } @@ -46,6 +55,16 @@ export abstract class NotFoundError extends WorkflowExecutorError {} export abstract class AccessDeniedError extends WorkflowExecutorError {} export abstract class UnavailableError extends WorkflowExecutorError {} +// One abstract per classified kind: the family declares it once and a new member joins by extending +// it. An error extending neither stays unclassified, which is what preserves today's framing. +export abstract class WorkflowConfigurationError extends WorkflowExecutorError { + static override readonly defaultErrorKind: ErrorKind = 'configuration'; +} + +export abstract class WorkflowOperatorError extends WorkflowExecutorError { + static override readonly defaultErrorKind: ErrorKind = 'operator'; +} + export class MissingToolCallError extends WorkflowExecutorError { constructor() { super( @@ -67,7 +86,7 @@ export class MalformedToolCallError extends WorkflowExecutorError { } } -export class RecordNotFoundError extends WorkflowExecutorError { +export class RecordNotFoundError extends WorkflowOperatorError { constructor(collectionName: string, recordId: RecordId) { super( `Record not found: collection "${collectionName}", id "${recordId.join('|')}"`, @@ -76,13 +95,13 @@ export class RecordNotFoundError extends WorkflowExecutorError { } } -export class NoRecordsError extends WorkflowExecutorError { +export class NoRecordsError extends WorkflowOperatorError { constructor() { super('No records available'); } } -export class NoReadableFieldsError extends WorkflowExecutorError { +export class NoReadableFieldsError extends WorkflowConfigurationError { constructor(collectionName: string) { super( `No readable fields on record from collection "${collectionName}"`, @@ -100,7 +119,7 @@ export class NoResolvedFieldsError extends WorkflowExecutorError { } } -export class NoWritableFieldsError extends WorkflowExecutorError { +export class NoWritableFieldsError extends WorkflowConfigurationError { constructor(collectionName: string) { super( `No writable fields on record from collection "${collectionName}"`, @@ -109,7 +128,7 @@ export class NoWritableFieldsError extends WorkflowExecutorError { } } -export class NoActionsError extends WorkflowExecutorError { +export class NoActionsError extends WorkflowConfigurationError { constructor(collectionName: string) { super( `No actions available on collection "${collectionName}"`, @@ -130,7 +149,7 @@ export class UnsupportedActionFormError extends WorkflowExecutorError { // The action submission was rejected by the agent's server-side validation (bad/missing values), // NOT an infra failure. Full AI treats this as a fallback-to-AI-assisted reason // so a human can fix the values and resubmit. -export class ActionFormValidationError extends WorkflowExecutorError { +export class ActionFormValidationError extends WorkflowOperatorError { constructor(actionName: string, cause?: unknown) { super( `Action "${actionName}" rejected the submitted form values`, @@ -144,7 +163,7 @@ export class ActionFormValidationError extends WorkflowExecutorError { // CustomActionRequiresApprovalError. Distinct from a plain permission 403 — Full AI // falls back to AI-assisted so the native front handles the approval flow. The executor // MUST NOT self-sign an approval request. -export class ActionRequiresApprovalError extends WorkflowExecutorError { +export class ActionRequiresApprovalError extends WorkflowOperatorError { readonly roleIdsAllowedToApprove?: number[]; constructor(actionName: string, roleIdsAllowedToApprove?: number[]) { @@ -177,7 +196,7 @@ export class RunStorePortError extends UnavailableError { } } -export class NoRelationshipFieldsError extends WorkflowExecutorError { +export class NoRelationshipFieldsError extends WorkflowConfigurationError { constructor(collectionName: string) { super( `No relationship fields on record from collection "${collectionName}"`, @@ -186,7 +205,7 @@ export class NoRelationshipFieldsError extends WorkflowExecutorError { } } -export class RelatedRecordNotFoundError extends WorkflowExecutorError { +export class RelatedRecordNotFoundError extends WorkflowOperatorError { constructor(collectionName: string, relationName: string) { super( `No related record found for relation "${relationName}" on collection "${collectionName}"`, @@ -201,13 +220,13 @@ export class InvalidAIResponseError extends WorkflowExecutorError { } } -export class InvalidAiRequestError extends WorkflowExecutorError { +export class InvalidAiRequestError extends WorkflowConfigurationError { constructor(message: string) { super(message, 'Step configuration error — please contact your administrator.'); } } -export class RelationNotFoundError extends WorkflowExecutorError { +export class RelationNotFoundError extends WorkflowConfigurationError { constructor(name: string, collectionName: string) { super( `Relation "${name}" not found in collection "${collectionName}"`, @@ -216,7 +235,7 @@ export class RelationNotFoundError extends WorkflowExecutorError { } } -export class FieldNotFoundError extends WorkflowExecutorError { +export class FieldNotFoundError extends WorkflowConfigurationError { constructor(name: string, collectionName: string) { super( `Field "${name}" not found in collection "${collectionName}"`, @@ -225,7 +244,7 @@ export class FieldNotFoundError extends WorkflowExecutorError { } } -export class FieldTypeMissingError extends WorkflowExecutorError { +export class FieldTypeMissingError extends WorkflowConfigurationError { constructor(name: string, collectionName: string) { super( `Field "${name}" in collection "${collectionName}" has no column type`, @@ -235,7 +254,7 @@ export class FieldTypeMissingError extends WorkflowExecutorError { } } -export class ActionNotFoundError extends WorkflowExecutorError { +export class ActionNotFoundError extends WorkflowConfigurationError { constructor(name: string, collectionName: string) { super( `Action "${name}" not found in collection "${collectionName}"`, @@ -485,7 +504,7 @@ export class InvalidPendingDataError extends WorkflowExecutorError { } } -export class InvalidPreRecordedArgsError extends WorkflowExecutorError { +export class InvalidPreRecordedArgsError extends WorkflowConfigurationError { constructor(detail: string) { super(`Invalid pre-recorded args: ${detail}`, 'The pre-configured step parameters are invalid'); } @@ -494,13 +513,20 @@ export class InvalidPreRecordedArgsError extends WorkflowExecutorError { // A "Related to" / "On record" source step ran but loaded no record, so the step that uses it has // no source to act on ("no source record"). Distinct from a bad config — the // user can continue without. Wording is step-type-neutral (shared by load-related and trigger-action). +// The kind comes from the throw site: only there is it known whether the operator had a record to +// pick, which is what decides who can act on it. export class SourceRecordMissingError extends WorkflowExecutorError { - constructor(sourceTitle?: string) { + constructor( + sourceTitle?: string, + options: { errorKind?: ErrorKind; errorSourceStepIndex?: number } = {}, + ) { const from = sourceTitle ? `"${sourceTitle}"` : 'its source step'; super( `Source step ${from} loaded no record`, `This step uses ${from} as its source, but that step didn't load any record.`, ); + this.errorKind = options.errorKind ?? this.errorKind; + this.errorSourceStepIndex = options.errorSourceStepIndex; } } diff --git a/packages/workflow-executor/src/executors/base-step-executor.ts b/packages/workflow-executor/src/executors/base-step-executor.ts index 60021c0c90..97657f496b 100644 --- a/packages/workflow-executor/src/executors/base-step-executor.ts +++ b/packages/workflow-executor/src/executors/base-step-executor.ts @@ -6,7 +6,7 @@ import type { import type { ConfirmableStepExecutionData, StepExecutionData } from '../types/step-execution-data'; import type { Step } from '../types/validated/execution'; import type { StepDefinition } from '../types/validated/step-definition'; -import type { StepStatus } from '../types/validated/step-outcome'; +import type { ErrorKind, StepStatus } from '../types/validated/step-outcome'; import type { BaseMessage, DynamicStructuredTool, @@ -75,7 +75,7 @@ export default abstract class BaseStepExecutor; protected checkIdempotency(): Promise { @@ -146,6 +159,8 @@ export default abstract class BaseStepExecutor( diff --git a/packages/workflow-executor/src/executors/condition-step-executor.ts b/packages/workflow-executor/src/executors/condition-step-executor.ts index 36580d0956..a18780d614 100644 --- a/packages/workflow-executor/src/executors/condition-step-executor.ts +++ b/packages/workflow-executor/src/executors/condition-step-executor.ts @@ -1,6 +1,6 @@ import type { StepExecutionResult } from '../types/execution-context'; import type { ConditionStepDefinition } from '../types/validated/step-definition'; -import type { ConditionStepOutcome } from '../types/validated/step-outcome'; +import type { ConditionStepOutcome, ErrorKind } from '../types/validated/step-outcome'; import { DynamicStructuredTool, HumanMessage, SystemMessage } from '@forestadmin/ai-proxy'; import { z } from 'zod'; @@ -47,6 +47,8 @@ export default class ConditionStepExecutor extends BaseStepExecutor protected buildOutcomeResult(outcome: { status: RecordStepStatus; error?: string; + errorKind?: ErrorKind; + errorSourceStepIndex?: number; awaitingInputReason?: AwaitingInputReason; }): StepExecutionResult { return { diff --git a/packages/workflow-executor/src/executors/record-step-executor.ts b/packages/workflow-executor/src/executors/record-step-executor.ts index 5bc15ab656..a883c7885b 100644 --- a/packages/workflow-executor/src/executors/record-step-executor.ts +++ b/packages/workflow-executor/src/executors/record-step-executor.ts @@ -1,7 +1,8 @@ import type { StepExecutionResult } from '../types/execution-context'; +import type { StepExecutionData } from '../types/step-execution-data'; import type { CollectionSchema, FieldSchema, RecordRef } from '../types/validated/collection'; import type { StepDefinition } from '../types/validated/step-definition'; -import type { RecordStepStatus } from '../types/validated/step-outcome'; +import type { ErrorKind, RecordStepStatus } from '../types/validated/step-outcome'; import { DynamicStructuredTool, HumanMessage, SystemMessage } from '@forestadmin/ai-proxy'; import { z } from 'zod'; @@ -15,12 +16,31 @@ import { import BaseStepExecutor from './base-step-executor'; import { StepType, WORKFLOW_START_STEP_ID } from '../types/validated/step-definition'; +// A source step that offered a candidate and was passed over is an operator situation; one that had +// nothing to offer is a configuration one. An execution the guard cannot read stays unclassified. +function classifyMissingSourceRecord(execution?: StepExecutionData): ErrorKind | undefined { + if (execution?.type !== 'load-related-record') return undefined; + + const { pendingData, executionResult } = execution; + + // A result outside the declared shape signals an executor defect, not a decision the run made. + if (executionResult !== undefined && !('skipped' in executionResult)) return undefined; + + // Nothing was ever offered: only Full AI continues without pausing, so there was no choice to make. + if (!pendingData) return executionResult !== undefined ? 'configuration' : undefined; + + // Whether it paused or recorded a decline, the candidate list says whether there was a choice. + return pendingData.availableRecordIds.length > 0 ? 'operator' : 'configuration'; +} + export default abstract class RecordStepExecutor< TStep extends StepDefinition = StepDefinition, > extends BaseStepExecutor { protected buildOutcomeResult(outcome: { status: RecordStepStatus; error?: string; + errorKind?: ErrorKind; + errorSourceStepIndex?: number; }): StepExecutionResult { return { stepOutcome: { @@ -84,7 +104,10 @@ export default abstract class RecordStepExecutor< // The source step exists but loaded nothing → clear "no source record" message, // distinct from a config pointing at a non-existent step. - throw new SourceRecordMissingError(sourceStep.stepDefinition.title); + throw new SourceRecordMissingError(sourceStep.stepDefinition.title, { + errorKind: classifyMissingSourceRecord(execution), + errorSourceStepIndex: sourceStep.stepOutcome.stepIndex, + }); } throw new InvalidPreRecordedArgsError(`No source record found for step "${stepId}"`); diff --git a/packages/workflow-executor/src/executors/summary/step-summary-builder.ts b/packages/workflow-executor/src/executors/summary/step-summary-builder.ts index aafbbb6013..adee90133c 100644 --- a/packages/workflow-executor/src/executors/summary/step-summary-builder.ts +++ b/packages/workflow-executor/src/executors/summary/step-summary-builder.ts @@ -52,7 +52,10 @@ export default class StepSummaryBuilder { } } } else { - const { stepId, stepIndex, type, ...historyDetails } = stepOutcome; + // The classification addresses the operator and the UI, not the model: `error` already carries + // the only fact that constrains a later step, and naming a culprit cannot change what it writes. + const { stepId, stepIndex, type, errorKind, errorSourceStepIndex, ...historyDetails } = + stepOutcome; lines.push(` History: ${JSON.stringify(historyDetails)}`); } diff --git a/packages/workflow-executor/src/executors/trigger-record-action-step-executor.ts b/packages/workflow-executor/src/executors/trigger-record-action-step-executor.ts index 3fe536b0f3..a35091b3c4 100644 --- a/packages/workflow-executor/src/executors/trigger-record-action-step-executor.ts +++ b/packages/workflow-executor/src/executors/trigger-record-action-step-executor.ts @@ -7,7 +7,7 @@ import type { } from '../types/step-execution-data'; import type { ActionSchema, CollectionSchema, RecordRef } from '../types/validated/collection'; import type { TriggerActionStepDefinition } from '../types/validated/step-definition'; -import type { RecordStepStatus } from '../types/validated/step-outcome'; +import type { ErrorKind, RecordStepStatus } from '../types/validated/step-outcome'; import { DynamicStructuredTool, HumanMessage, SystemMessage } from '@forestadmin/ai-proxy'; import { z } from 'zod'; @@ -51,6 +51,8 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< protected override buildOutcomeResult(outcome: { status: RecordStepStatus; error?: string; + errorKind?: ErrorKind; + errorSourceStepIndex?: number; approvalRequest?: { id: string }; }): StepExecutionResult { return super.buildOutcomeResult(outcome); diff --git a/packages/workflow-executor/src/types/validated/step-outcome.ts b/packages/workflow-executor/src/types/validated/step-outcome.ts index 35d086d2a0..b7a774a710 100644 --- a/packages/workflow-executor/src/types/validated/step-outcome.ts +++ b/packages/workflow-executor/src/types/validated/step-outcome.ts @@ -14,6 +14,15 @@ export type RecordStepStatus = z.infer; export const AwaitingInputReasonSchema = z.enum(['needs-oauth-reauth']); export type AwaitingInputReason = z.infer; +// What kind of failure a step error is. All three cross the wire even though only 'operator' drives +// a UI branch today: widening an enum is cheap, changing a cross-service contract is not. +export const ErrorKindSchema = z.enum(['operator', 'configuration', 'system']); +export type ErrorKind = z.infer; + +// Identifies the step an error is about by index rather than by step id: a LinkTo loop repeats ids, +// so only the index says which iteration the error came from. +export const ErrorSourceStepIndexSchema = z.number().int().nonnegative(); + export type StepStatus = BaseStepStatus | RecordStepStatus; /** @@ -26,6 +35,10 @@ const baseOutcomeFields = { stepIndex: z.number().int().nonnegative(), /** Present when status is 'error'. */ error: z.string().optional(), + /** Present when the error has been classified. Absent leaves the error framed as it is today. */ + errorKind: ErrorKindSchema.optional(), + /** Present when the error is about another step, e.g. a source step that loaded no record. */ + errorSourceStepIndex: ErrorSourceStepIndexSchema.optional(), }; export const ConditionStepOutcomeSchema = z diff --git a/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts b/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts index 97fb52c6b3..a8743fbc8b 100644 --- a/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts +++ b/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts @@ -458,6 +458,44 @@ describe('ForestServerWorkflowPort', () => { ); }); + it('posts the classification and the source step alongside the error', async () => { + mockQuery.mockResolvedValue(undefined); + const stepOutcome: StepOutcome = { + type: 'record', + stepId: 'step-1', + stepIndex: 1, + status: 'error', + error: 'boom', + errorKind: 'operator', + errorSourceStepIndex: 0, + }; + + await port.updateStepExecution('42', stepOutcome); + + expect(mockQuery).toHaveBeenCalledWith( + options, + 'post', + '/api/workflow-orchestrator/update-step', + {}, + { + runId: 42, + stepUpdate: { + stepIndex: 1, + attributes: { + done: true, + context: { + status: 'error', + error: 'boom', + errorKind: 'operator', + errorSourceStepIndex: 0, + }, + }, + }, + executionStatus: { type: 'error', message: 'boom' }, + }, + ); + }); + it('posts the mapped body for an awaiting-input outcome', async () => { mockQuery.mockResolvedValue(undefined); const stepOutcome: StepOutcome = { 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 4397c00e91..fd7c30996b 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 @@ -5,6 +5,7 @@ import type { ServerWorkflowCondition, ServerWorkflowTask, } from '../../src/adapters/server-types'; +import type { StepOutcome } from '../../src/types/validated/step-outcome'; import { z } from 'zod'; @@ -15,6 +16,7 @@ import { ServerTaskTypeEnum, ServerWorkflowTriggerType, } from '../../src/adapters/server-types'; +import toUpdateStepRequest from '../../src/adapters/step-outcome-to-update-step-mapper'; import { DomainValidationError, InvalidStepDefinitionError } from '../../src/errors'; import { TriggerType } from '../../src/types/validated/execution'; import { StepType } from '../../src/types/validated/step-definition'; @@ -519,6 +521,119 @@ describe('toAvailableStepExecution', () => { expect(() => toAvailableStepExecution(run)).toThrow(InvalidStepDefinitionError); }); + + describe('errorKind', () => { + it('should read errorKind back from the step context', () => { + const run = makeRun({ + workflowHistory: [ + makeStepHistory({ + stepName: 's0', + stepIndex: 0, + done: true, + context: { status: 'error', error: 'No records available', errorKind: 'operator' }, + }), + makeStepHistory({ stepName: 's1', stepIndex: 1, done: false }), + ], + }); + + const result = toAvailableStepExecution(run); + + expect(result?.previousSteps[0].stepOutcome).toEqual({ + type: 'record', + stepId: 's0', + stepIndex: 0, + status: 'error', + error: 'No records available', + errorKind: 'operator', + }); + }); + + // `context` is free-form on the wire and this mapper zod-parses what it builds, so an + // off-vocabulary kind must be dropped rather than fail the whole run. + it.each(['user', 42, null])( + 'should drop the errorKind %p instead of failing the run', + badKind => { + const run = makeRun({ + workflowHistory: [ + makeStepHistory({ + stepName: 's0', + stepIndex: 0, + done: true, + context: { status: 'error', error: 'No records available', errorKind: badKind }, + }), + makeStepHistory({ stepName: 's1', stepIndex: 1, done: false }), + ], + }); + + const result = toAvailableStepExecution(run); + + expect(result?.previousSteps[0].stepOutcome).not.toHaveProperty('errorKind'); + expect(result?.previousSteps[0].stepOutcome.status).toBe('error'); + }, + ); + + it.each(['2', -1, 1.5, null])( + 'should drop the errorSourceStepIndex %p instead of failing the run', + badIndex => { + const run = makeRun({ + workflowHistory: [ + makeStepHistory({ + stepName: 's0', + stepIndex: 0, + done: true, + context: { + status: 'error', + error: 'No records available', + errorSourceStepIndex: badIndex, + }, + }), + makeStepHistory({ stepName: 's1', stepIndex: 1, done: false }), + ], + }); + + const result = toAvailableStepExecution(run); + + expect(result?.previousSteps[0].stepOutcome).not.toHaveProperty('errorSourceStepIndex'); + expect(result?.previousSteps[0].stepOutcome.status).toBe('error'); + }, + ); + + // The two mappers are each other's inverse over `context`, and source index 0 is the case a + // falsy-value check on either side would silently drop. + it('should round trip errorKind and errorSourceStepIndex written by the forward mapper', () => { + const reported: StepOutcome = { + type: 'record', + stepId: 's1', + stepIndex: 1, + status: 'error', + error: 'The record no longer exists. It may have been deleted.', + errorKind: 'operator', + errorSourceStepIndex: 0, + }; + const { stepUpdate } = toUpdateStepRequest('42', reported); + const run = makeRun({ + workflowHistory: [ + makeStepHistory({ + stepName: 's0', + stepIndex: 0, + done: true, + context: { status: 'success' }, + }), + makeStepHistory({ + stepName: 's1', + stepIndex: 1, + done: true, + context: stepUpdate.attributes.context, + }), + makeStepHistory({ stepName: 's2', stepIndex: 2, done: false }), + ], + }); + + const result = toAvailableStepExecution(run); + + expect(result?.previousSteps[1].stepOutcome).toEqual(reported); + }); + }); }); describe('revision handling', () => { diff --git a/packages/workflow-executor/test/adapters/step-outcome-to-update-step-mapper.test.ts b/packages/workflow-executor/test/adapters/step-outcome-to-update-step-mapper.test.ts index 620cf111d0..6320c943f0 100644 --- a/packages/workflow-executor/test/adapters/step-outcome-to-update-step-mapper.test.ts +++ b/packages/workflow-executor/test/adapters/step-outcome-to-update-step-mapper.test.ts @@ -255,4 +255,116 @@ describe('toUpdateStepRequest', () => { expect(body.stepUpdate.attributes.context).toEqual({ status: 'success' }); }); }); + + describe('errorKind propagation', () => { + it('writes errorKind beside error in the update-step context', () => { + const outcome: StepOutcome = { + type: 'record', + stepId: 'step-1', + stepIndex: 2, + status: 'error', + error: 'The record no longer exists. It may have been deleted.', + errorKind: 'operator', + }; + + const body = toUpdateStepRequest('42', outcome); + + expect(body.stepUpdate.attributes).toEqual({ + done: true, + context: { + status: 'error', + error: 'The record no longer exists. It may have been deleted.', + errorKind: 'operator', + }, + }); + expect(body.executionStatus).toEqual({ + type: 'error', + message: 'The record no longer exists. It may have been deleted.', + }); + }); + + // Only 'operator' drives a UI branch today, but all three cross the wire — widening the enum + // later must not require another cross-service change. + it.each(['operator', 'configuration', 'system'] as const)('forwards the %s kind', kind => { + const outcome: StepOutcome = { + type: 'mcp', + stepId: 'step-1', + stepIndex: 0, + status: 'error', + error: 'The tool failed to execute.', + errorKind: kind, + }; + + const body = toUpdateStepRequest('42', outcome); + + expect(body.stepUpdate.attributes.context).toEqual({ + status: 'error', + error: 'The tool failed to execute.', + errorKind: kind, + }); + }); + + it('writes errorSourceStepIndex alongside the kind', () => { + const outcome: StepOutcome = { + type: 'record', + stepId: 'step-3', + stepIndex: 3, + status: 'error', + error: + 'This step uses "Load the order" as its source, but that step didn\'t load any record.', + errorKind: 'operator', + errorSourceStepIndex: 2, + }; + + const body = toUpdateStepRequest('42', outcome); + + expect(body.stepUpdate.attributes.context).toEqual({ + status: 'error', + error: + 'This step uses "Load the order" as its source, but that step didn\'t load any record.', + errorKind: 'operator', + errorSourceStepIndex: 2, + }); + }); + + // Index 0 is a real step, so a falsy-value check in the mapper would drop the first step of a run. + it('writes errorSourceStepIndex 0', () => { + const outcome: StepOutcome = { + type: 'record', + stepId: 'step-1', + stepIndex: 1, + status: 'error', + error: 'boom', + errorKind: 'operator', + errorSourceStepIndex: 0, + }; + + const body = toUpdateStepRequest('42', outcome); + + expect(body.stepUpdate.attributes.context).toEqual({ + status: 'error', + error: 'boom', + errorKind: 'operator', + errorSourceStepIndex: 0, + }); + }); + + it('sends the payload unchanged for an unclassified error', () => { + const outcome: StepOutcome = { + type: 'condition', + stepId: 'step-1', + stepIndex: 0, + status: 'error', + error: 'AI gateway unreachable', + }; + + const body = toUpdateStepRequest('7', outcome); + + expect(body.stepUpdate.attributes.context).toEqual({ + status: 'error', + error: 'AI gateway unreachable', + }); + expect(body.stepUpdate.attributes.context).not.toHaveProperty('errorKind'); + }); + }); }); diff --git a/packages/workflow-executor/test/errors.test.ts b/packages/workflow-executor/test/errors.test.ts index bdc98ef8a7..f3302bc5fc 100644 --- a/packages/workflow-executor/test/errors.test.ts +++ b/packages/workflow-executor/test/errors.test.ts @@ -1,11 +1,33 @@ +import type { WorkflowExecutorError } from '../src/errors'; + import { + ActionFormValidationError, + ActionNotFoundError, + ActionRequiresApprovalError, + AgentPortError, AiModelPortError, + FieldNotFoundError, + FieldTypeMissingError, + InvalidAiRequestError, InvalidPendingDataError, + InvalidPreRecordedArgsError, + MissingToolCallError, + NoActionsError, NoMcpToolsError, + NoReadableFieldsError, + NoRecordsError, + NoRelationshipFieldsError, + NoWritableFieldsError, OAuthInvalidGrantError, OAuthReauthRequiredError, OAuthRefreshError, PendingDataNotFoundError, + RecordNotFoundError, + RelatedRecordNotFoundError, + RelationNotFoundError, + SourceRecordMissingError, + StepStateError, + StepTimeoutError, causeMessage, extractErrorMessage, } from '../src/errors'; @@ -199,3 +221,74 @@ describe('OAuthInvalidGrantError', () => { expect(new OAuthInvalidGrantError('token expired').message).toMatch(/token expired/); }); }); + +describe('errorKind classification', () => { + // The record or the submitted input is the problem — nothing is broken in the workflow or the agent. + it.each<[string, WorkflowExecutorError]>([ + ['NoRecordsError', new NoRecordsError()], + ['RecordNotFoundError', new RecordNotFoundError('customers', [42])], + ['RelatedRecordNotFoundError', new RelatedRecordNotFoundError('customers', 'orders')], + ['ActionFormValidationError', new ActionFormValidationError('send-welcome-email')], + ['ActionRequiresApprovalError', new ActionRequiresApprovalError('send-welcome-email')], + ])('classifies %s as operator', (_, error) => { + expect(error.errorKind).toBe('operator'); + }); + + // The step cannot succeed as configured, whatever the run does. Every member's own userMessage + // already says so; the kind only makes it machine-readable. + it.each<[string, WorkflowExecutorError]>([ + ['FieldNotFoundError', new FieldNotFoundError('emailz', 'customers')], + ['ActionNotFoundError', new ActionNotFoundError('sned-email', 'customers')], + ['RelationNotFoundError', new RelationNotFoundError('orderz', 'customers')], + ['NoActionsError', new NoActionsError('customers')], + ['NoWritableFieldsError', new NoWritableFieldsError('customers')], + ['NoReadableFieldsError', new NoReadableFieldsError('customers')], + ['NoRelationshipFieldsError', new NoRelationshipFieldsError('customers')], + ['FieldTypeMissingError', new FieldTypeMissingError('status', 'customers')], + ['InvalidAiRequestError', new InvalidAiRequestError('SystemMessage at position 3')], + ['InvalidPreRecordedArgsError', new InvalidPreRecordedArgsError('no record at step index 4')], + ])('classifies %s as configuration', (_, error) => { + expect(error.errorKind).toBe('configuration'); + }); + + // Unclassified is the starting default: an error nobody has triaged keeps today's framing. + it.each<[string, WorkflowExecutorError]>([ + ['StepTimeoutError', new StepTimeoutError(30)], + ['AgentPortError', new AgentPortError('getRecord', new Error('ECONNREFUSED'))], + ['AiModelPortError', new AiModelPortError('invoke', new Error('timeout'))], + ['MissingToolCallError', new MissingToolCallError()], + ['StepStateError', new StepStateError('Step at index 0 has no pending data')], + ])('leaves %s unclassified', (_, error) => { + expect(error.errorKind).toBeUndefined(); + }); + + describe('SourceRecordMissingError', () => { + it('is unclassified and names no source step by default', () => { + const error = new SourceRecordMissingError('Load the order'); + + expect(error.errorKind).toBeUndefined(); + expect(error.errorSourceStepIndex).toBeUndefined(); + }); + + // The only error whose kind depends on why it was thrown: the throw site is the one place that + // knows whether a candidate was offered. + it.each(['operator', 'configuration'] as const)( + 'takes the %s kind from the throw site', + kind => { + const error = new SourceRecordMissingError('Load the order', { errorKind: kind }); + + expect(error.errorKind).toBe(kind); + expect(error.userMessage).toContain("didn't load any record"); + }, + ); + + it('carries the index of the source step the guard resolved', () => { + const error = new SourceRecordMissingError('Load the order', { + errorKind: 'operator', + errorSourceStepIndex: 4, + }); + + expect(error.errorSourceStepIndex).toBe(4); + }); + }); +}); 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 b2b17fd97e..45140117a0 100644 --- a/packages/workflow-executor/test/executors/base-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/base-step-executor.test.ts @@ -9,7 +9,11 @@ import type { StepExecutionData } from '../../src/types/step-execution-data'; import type { RecordRef } from '../../src/types/validated/collection'; import type { Step } from '../../src/types/validated/execution'; import type { StepDefinition } from '../../src/types/validated/step-definition'; -import type { BaseStepStatus, StepOutcome } from '../../src/types/validated/step-outcome'; +import type { + BaseStepStatus, + ErrorKind, + StepOutcome, +} from '../../src/types/validated/step-outcome'; import type { BaseMessage, DynamicStructuredTool } from '@forestadmin/ai-proxy'; import { HumanMessage, SystemMessage } from '@forestadmin/ai-proxy'; @@ -45,6 +49,7 @@ class TestableExecutor extends BaseStepExecutor { protected buildOutcomeResult(outcome: { status: BaseStepStatus; error?: string; + errorKind?: ErrorKind; }): StepExecutionResult { return { stepOutcome: { @@ -53,6 +58,7 @@ class TestableExecutor extends BaseStepExecutor { stepIndex: this.context.stepIndex, status: outcome.status, ...(outcome.error !== undefined && { error: outcome.error }), + ...(outcome.errorKind !== undefined && { errorKind: outcome.errorKind }), }, }; } @@ -407,21 +413,36 @@ describe('BaseStepExecutor', () => { }); describe('execute error handling', () => { - it('converts NoRecordsError to error outcome', async () => { + it('converts NoRecordsError to an operator-classified error outcome', async () => { const executor = new TestableExecutor(makeContext(), new NoRecordsError()); const result = await executor.execute(); expect(result.stepOutcome.status).toBe('error'); expect(result.stepOutcome.error).toBe('No records available'); + expect(result.stepOutcome.errorKind).toBe('operator'); + }); + + it('reports an unclassified error without an errorKind', async () => { + const executor = new TestableExecutor( + makeContext(), + new StepStateError('Step at index 0 has no pending data'), + ); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome).not.toHaveProperty('errorKind'); }); describe('unexpected error handling', () => { + // A thrown non-WorkflowExecutorError has no kind to carry — it keeps today's framing. it('returns error outcome instead of rethrowing', async () => { const executor = new TestableExecutor(makeContext(), new Error('db connection refused')); const result = await executor.execute(); expect(result.stepOutcome.status).toBe('error'); expect(result.stepOutcome.error).toBe('Unexpected error during step execution'); + expect(result.stepOutcome).not.toHaveProperty('errorKind'); }); it('logs the full error context when logger is provided', async () => { diff --git a/packages/workflow-executor/test/executors/step-summary-builder.test.ts b/packages/workflow-executor/test/executors/step-summary-builder.test.ts index 371111da20..156ea9d587 100644 --- a/packages/workflow-executor/test/executors/step-summary-builder.test.ts +++ b/packages/workflow-executor/test/executors/step-summary-builder.test.ts @@ -80,6 +80,26 @@ describe('StepSummaryBuilder', () => { expect(result).not.toContain('"type"'); }); + // A step that errored and was then completed manually reaches this branch, so the classification + // would otherwise land in the model's context. `error` already says the record is absent. + it('keeps the error classification out of History', () => { + const step = makeConditionStep('Pick one'); + const outcome = makeConditionOutcome('cond-1', 0, { + status: 'error', + error: 'that step did not load any record', + errorKind: 'operator', + errorSourceStepIndex: 2, + }); + + const result = StepSummaryBuilder.build(step, outcome, undefined); + + expect(result).toContain( + 'History: {"status":"error","error":"that step did not load any record"}', + ); + expect(result).not.toContain('errorKind'); + expect(result).not.toContain('errorSourceStepIndex'); + }); + it('includes selectedOption in History for condition steps', () => { const step = makeConditionStep('Approved?'); const outcome = makeConditionOutcome('cond-approval', 0, { selectedOption: 'Yes' }); 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 fdef521cd8..c9e965762d 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 @@ -3,7 +3,10 @@ import type { AgentPort } from '../../src/ports/agent-port'; import type { RunStore } from '../../src/ports/run-store'; import type { WorkflowPort } from '../../src/ports/workflow-port'; import type { ExecutionContext } from '../../src/types/execution-context'; -import type { TriggerRecordActionStepExecutionData } from '../../src/types/step-execution-data'; +import type { + LoadRelatedRecordStepExecutionData, + TriggerRecordActionStepExecutionData, +} from '../../src/types/step-execution-data'; import type { CollectionSchema, RecordRef } from '../../src/types/validated/collection'; import type { Step } from '../../src/types/validated/execution'; import type { TriggerActionStepDefinition } from '../../src/types/validated/step-definition'; @@ -1989,27 +1992,209 @@ describe('TriggerRecordActionStepExecutor', () => { ); }); - it('errors when the pinned source step (a Load Related Record) loaded no record', async () => { - const agentPort = makeMockAgentPort(); - // The source Load Related Record step is on the live path but has no execution record stored - // (it loaded nothing) → SourceRecordMissingError, no action triggered. - const runStore = makeMockRunStore({ getStepExecutions: jest.fn().mockResolvedValue([]) }); - const context = makeContext({ - agentPort, - runStore, - previousSteps: [makeLoadRelatedPreviousStep(2)], - stepDefinition: makeStep({ - executionType: StepExecutionMode.FullyAutomated, - preRecordedArgs: { selectedRecordStepId: 'load-2', actionName: 'send-welcome-email' }, - }), + describe('a source step that loaded no record', () => { + const relation = { name: 'orders', displayName: 'Orders' }; + const oneCandidate = [{ recordId: [99], referenceFieldValue: 'Order #99' }]; + + // Every case here pins the action to the same Load Related Record source (step id 'load-2' at + // index 2) and varies only what that step left behind in the run store. + async function runPinnedToSource({ + executions = [], + executionType = StepExecutionMode.FullyAutomated, + selectedRecordStepId = 'load-2', + }: { + executions?: LoadRelatedRecordStepExecutionData[]; + executionType?: StepExecutionMode; + selectedRecordStepId?: string; + } = {}) { + const agentPort = makeMockAgentPort(); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue(executions), + }); + const context = makeContext({ + agentPort, + runStore, + previousSteps: [makeLoadRelatedPreviousStep(2)], + stepDefinition: makeStep({ + executionType, + preRecordedArgs: { selectedRecordStepId, actionName: 'send-welcome-email' }, + }), + }); + + const { stepOutcome } = await new TriggerRecordActionStepExecutor(context).execute(); + + return { stepOutcome, agentPort }; + } + + it('errors without triggering the action when no execution record was stored', async () => { + const { stepOutcome, agentPort } = await runPinnedToSource(); + + expect(stepOutcome.status).toBe('error'); + expect(stepOutcome.error).toContain("didn't load any record"); + expect(agentPort.executeAction).not.toHaveBeenCalled(); + // As likely our own missing run-store entry as anything the operator did, so it names nobody + // — but which step is implicated is known regardless. + expect(stepOutcome).not.toHaveProperty('errorKind'); + expect(stepOutcome.errorSourceStepIndex).toBe(2); }); - const executor = new TriggerRecordActionStepExecutor(context); - const result = await executor.execute(); + it('classifies a manually completed source as an operator error', async () => { + // Paused (pendingData saved, no executionResult), then completed out of band, which never + // comes back through the executor. A candidate was on the table and they passed on it. + const { stepOutcome, agentPort } = await runPinnedToSource({ + executions: [ + { + type: 'load-related-record', + stepIndex: 2, + pendingData: { + availableFields: [relation], + suggestedField: relation, + availableRecordIds: oneCandidate, + suggestNoRecord: true, + }, + }, + ], + }); - expect(result.stepOutcome.status).toBe('error'); - expect(result.stepOutcome.error).toContain("didn't load any record"); - expect(agentPort.executeAction).not.toHaveBeenCalled(); + expect(stepOutcome.status).toBe('error'); + expect(stepOutcome.errorKind).toBe('operator'); + // A LinkTo loop repeats step ids, so the index is the only thing that identifies which + // iteration lost its record. + expect(stepOutcome.errorSourceStepIndex).toBe(2); + expect(agentPort.executeAction).not.toHaveBeenCalled(); + }); + + it('classifies a source that offered candidates with no AI suggestion as operator', async () => { + // A Manual source pause populates the candidate list and never sets suggestNoRecord, so the + // list is what says the operator had a choice — reading the flag would miss this. + const { stepOutcome } = await runPinnedToSource({ + executionType: StepExecutionMode.Manual, + executions: [ + { + type: 'load-related-record', + stepIndex: 2, + pendingData: { + availableFields: [relation], + suggestedField: relation, + availableRecordIds: oneCandidate, + }, + }, + ], + }); + + expect(stepOutcome.status).toBe('error'); + expect(stepOutcome.errorKind).toBe('operator'); + expect(stepOutcome.errorSourceStepIndex).toBe(2); + }); + + it('classifies a declined confirmation with candidates as an operator error', async () => { + // The confirmation flow records a decline as a skipped result while keeping the candidate + // list, so the result shape alone would read this as nobody's choice. + const { stepOutcome } = await runPinnedToSource({ + executions: [ + { + type: 'load-related-record', + stepIndex: 2, + pendingData: { + availableFields: [relation], + suggestedField: relation, + availableRecordIds: oneCandidate, + }, + userConfirmation: { userConfirmed: false }, + executionResult: { skipped: true }, + }, + ], + }); + + expect(stepOutcome.status).toBe('error'); + expect(stepOutcome.errorKind).toBe('operator'); + expect(stepOutcome.errorSourceStepIndex).toBe(2); + }); + + it('classifies a source step id that matches no step as a configuration error', async () => { + const { stepOutcome, agentPort } = await runPinnedToSource({ + selectedRecordStepId: 'load-9', + }); + + expect(stepOutcome.status).toBe('error'); + expect(stepOutcome.errorKind).toBe('configuration'); + // Nothing resolved, so there is no history entry to name. + expect(stepOutcome).not.toHaveProperty('errorSourceStepIndex'); + expect(agentPort.executeAction).not.toHaveBeenCalled(); + }); + + // The next two are the same empty relation seen from the two execution modes that reach it. + // They must agree: who has to act does not depend on which mode ran the source step. + it('classifies a relation the executor skipped with no candidates as configuration', async () => { + // Full AI found nothing to offer and continued on its own judgment (persistSkip). Nobody was + // there to decide, and the workflow routes a record-consuming step off an emptiable relation. + const { stepOutcome } = await runPinnedToSource({ + executions: [ + { + type: 'load-related-record', + stepIndex: 2, + executionParams: relation, + executionResult: { skipped: true }, + }, + ], + }); + + expect(stepOutcome.status).toBe('error'); + expect(stepOutcome.error).toContain("didn't load any record"); + expect(stepOutcome.errorKind).toBe('configuration'); + }); + + it('classifies an acknowledged empty relation as configuration', async () => { + // Same empty relation, AI-assisted: the step paused with nothing to offer and the operator + // acknowledged it. They decided, but never had an alternative to decide between. + const { stepOutcome } = await runPinnedToSource({ + executionType: StepExecutionMode.AutomatedWithConfirmation, + executions: [ + { + type: 'load-related-record', + stepIndex: 2, + pendingData: { + availableFields: [relation], + suggestedField: relation, + availableRecordIds: [], + suggestNoRecord: true, + }, + }, + ], + }); + + expect(stepOutcome.status).toBe('error'); + expect(stepOutcome.errorKind).toBe('configuration'); + expect(stepOutcome.errorSourceStepIndex).toBe(2); + }); + + it('leaves a source with neither a result nor a candidate list unclassified', async () => { + // Nothing to read the situation from: it neither finished nor recorded what it offered. + const { stepOutcome } = await runPinnedToSource({ + executions: [{ type: 'load-related-record', stepIndex: 2 }], + }); + + expect(stepOutcome.status).toBe('error'); + expect(stepOutcome).not.toHaveProperty('errorKind'); + expect(stepOutcome.errorSourceStepIndex).toBe(2); + }); + + it('leaves a source with an unreadable result shape unclassified', async () => { + // A result the guard cannot read is as likely our own shape mismatch as anything that + // happened in the run, so it must not name a culprit. + const { stepOutcome } = await runPinnedToSource({ + executions: [ + { + type: 'load-related-record', + stepIndex: 2, + executionResult: { relation }, + } as unknown as LoadRelatedRecordStepExecutionData, + ], + }); + + expect(stepOutcome.status).toBe('error'); + expect(stepOutcome).not.toHaveProperty('errorKind'); + }); }); }); diff --git a/packages/workflow-executor/test/types/step-outcome.test.ts b/packages/workflow-executor/test/types/step-outcome.test.ts index 419077e64b..0709dbe981 100644 --- a/packages/workflow-executor/test/types/step-outcome.test.ts +++ b/packages/workflow-executor/test/types/step-outcome.test.ts @@ -1,6 +1,10 @@ import { StepType } from '../../src/types/validated/step-definition'; import { + ConditionStepOutcomeSchema, + ErrorKindSchema, + GuidanceStepOutcomeSchema, McpStepOutcomeSchema, + RecordStepOutcomeSchema, stepTypeToOutcomeType, } from '../../src/types/validated/step-outcome'; @@ -77,3 +81,96 @@ describe('McpStepOutcomeSchema — awaitingInputReason', () => { ).toThrow(); }); }); + +describe('ErrorKindSchema', () => { + // All three kinds cross the wire even though only 'operator' drives a UI branch — the vocabulary + // is a cross-service contract, so it is pinned here rather than by server-side validation. + it('accepts the three kinds of the classification vocabulary', () => { + expect(ErrorKindSchema.parse('operator')).toBe('operator'); + expect(ErrorKindSchema.parse('configuration')).toBe('configuration'); + expect(ErrorKindSchema.parse('system')).toBe('system'); + }); + + it('rejects a kind outside the vocabulary', () => { + expect(() => ErrorKindSchema.parse('catastrophic')).toThrow(); + }); +}); + +describe('errorKind on step outcomes', () => { + const errored = { stepId: 'step-1', stepIndex: 0, status: 'error' as const, error: 'boom' }; + + // errorKind joins baseOutcomeFields, so a consumer reads the same key whatever the step type was. + it('is accepted on every outcome type', () => { + expect( + RecordStepOutcomeSchema.parse({ ...errored, type: 'record', errorKind: 'operator' }) + .errorKind, + ).toBe('operator'); + expect( + ConditionStepOutcomeSchema.parse({ + ...errored, + type: 'condition', + errorKind: 'configuration', + }).errorKind, + ).toBe('configuration'); + expect( + McpStepOutcomeSchema.parse({ ...errored, type: 'mcp', errorKind: 'system' }).errorKind, + ).toBe('system'); + expect( + GuidanceStepOutcomeSchema.parse({ ...errored, type: 'guidance', errorKind: 'operator' }) + .errorKind, + ).toBe('operator'); + }); + + it('is absent from an unclassified error outcome', () => { + const parsed = RecordStepOutcomeSchema.parse({ ...errored, type: 'record' }); + + expect(parsed.errorKind).toBeUndefined(); + }); + + it('rejects an unknown kind on an outcome', () => { + expect(() => + RecordStepOutcomeSchema.parse({ ...errored, type: 'record', errorKind: 'user' }), + ).toThrow(); + }); +}); + +describe('errorSourceStepIndex on step outcomes', () => { + const errored = { stepId: 'step-1', stepIndex: 3, status: 'error' as const, error: 'boom' }; + + it('carries the index of the step the error is about', () => { + const parsed = RecordStepOutcomeSchema.parse({ + ...errored, + type: 'record', + errorSourceStepIndex: 1, + }); + + expect(parsed.errorSourceStepIndex).toBe(1); + }); + + // The first step of a run is index 0, so the floor has to be inclusive. + it('accepts the first step of a run', () => { + const parsed = RecordStepOutcomeSchema.parse({ + ...errored, + type: 'record', + errorSourceStepIndex: 0, + }); + + expect(parsed.errorSourceStepIndex).toBe(0); + }); + + it('is absent when the error names no source step', () => { + const parsed = RecordStepOutcomeSchema.parse({ ...errored, type: 'record' }); + + expect(parsed.errorSourceStepIndex).toBeUndefined(); + }); + + it.each([-1, 1.5, '2'])('rejects %p as a step index', badIndex => { + expect(() => + RecordStepOutcomeSchema.parse({ + ...errored, + type: 'record', + errorSourceStepIndex: badIndex, + }), + ).toThrow(); + }); +});