fix(workflow-executor): use last step from history instead of scanning for first non-done

[Scra3]

Summary

• Replace workflowHistory.find(s => !s.done && !s.cancelled && !s.context?.error) with workflowHistory.at(-1) in both run-to-available-step-mapper.ts and forest-server-workflow-port.ts
• The orchestrator (server) is the source of truth for which step to execute next — the executor should not second-guess it by filtering step states
• Removes tests that asserted the old filtering logic; updates remaining tests to reflect the new contract

Test plan

• yarn workspace @forestadmin/workflow-executor test passes
• Empty history still returns null
• Last step in history is picked regardless of its done/cancelled/error state

🤖 Generated with Claude Code

Note

Use last workflow history entry as current step instead of scanning for first non-done

• The run-to-available-step-mapper now picks the last entry in workflowHistory as the pending step, replacing the previous scan for the first non-done step. This is the core fix described in the PR title.
• Renames PendingStepExecution to AvailableStepExecution and introduces getAvailableRuns/getAvailableRun on WorkflowPort, replacing getPendingStepExecutions/getPendingStepExecutionsForRun.
• Adds idempotency tracking (executing/done phases) to mutating step executors (update-record, trigger-action, MCP) to prevent duplicate side effects on retry.
• Adds step execution timeouts (default 5 min), activity log emission per step, and auto-chaining of sequential steps returned by updateStepExecution.
• Introduces a Kolar integration (KYB/AML screening tools) and extends Zendesk ticket tools with requester, recipient, status, and search filter support.
• Risk: AvailableStepExecution replaces PendingStepExecution in the public API; callers importing the old type will need to update.

📊 Macroscope summarized bafb247. 81 files reviewed, 19 issues evaluated, 6 issues filtered, 5 comments posted

🗂️ Filtered Issues

packages/agent/src/routes/workflow/workflow-executor-proxy.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 24: The constructor accesses options.workflowExecutorUrl.replace(...) without checking for null. The type AgentOptionsWithDefaults is Readonly<Required<AgentOptions>>, but workflowExecutorUrl has type string | null, so Required only removes the optional marker while preserving the | null union. If the route is instantiated when workflowExecutorUrl is null, this line throws TypeError: Cannot read properties of null (reading 'replace'). [ Failed validation ]

packages/ai-proxy/src/integrations/zendesk/tools/update-ticket.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 17: The status enum removed 'hold' which is a valid Zendesk ticket status. According to Zendesk's API, valid ticket statuses are: new, open, pending, hold, solved, and closed. Removing 'hold' prevents users from setting tickets to the 'hold' status, which is a legitimate workflow state used to pause tickets awaiting external responses. [ Cross-file consolidated ]

packages/mcp-server/src/utils/parse-tool-list.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 9: The as ToolName[] cast is unsafe and will accept any string as a valid tool name without validation. If an environment variable contains a typo or invalid tool name (e.g., "lst" instead of "list"), it will be silently cast to ToolName[] and pass type checking, but the invalid tool name will either be silently ignored downstream or cause unexpected behavior. Consider validating each parsed string against the known ToolName values before returning. [ Out of scope (triage) ]

packages/workflow-executor/src/adapters/forestadmin-client-activity-log-port.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 86: The errorMessage parameter in markFailed is accepted but never sent to the server — it's only used in the catch block for local logging when the update operation fails (line 93). If the intent is to record why the step failed in the audit log, the error message should be included in the updateActivityLogStatus call (around line 86), not discarded. Currently, a failed activity log will be marked as status: 'failed' with no explanation of what went wrong. [ Out of scope (triage) ]

packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts — 1 comment posted, 2 evaluated, 1 filtered

• line 139: The function documentation states it "Returns null when the run has no available step (terminal state or all done/cancelled)" but the implementation only returns null when workflowHistory is empty. If the last step in history has done: true or cancelled: true, the function will incorrectly return it as an available step instead of returning null. The check on line 140 should also verify that pending is not already completed, e.g., if (!pending || pending.done || pending.cancelled) return null; [ Out of scope (triage) ]

packages/workflow-executor/src/executors/update-record-step-executor.ts — 1 comment posted, 2 evaluated, 1 filtered

• line 228: If saveStepExecution fails after updateRecord succeeds at line 228-236, the record is updated in the database but the idempotency phase remains 'executing'. On retry, checkIdempotency() at lines 118-119 sees 'executing' and throws StepStateError, telling the user to retry manually. However, the update already succeeded, so a manual retry would update the record again, causing duplicate updates. The previous code wrapped this in a try-catch with StepPersistenceError that explicitly stated 'Record update persisted but step state could not be saved', giving context that the update succeeded despite the save failure. [ Cross-file consolidated ]

[linear]

PRD-214

[qltysh]

22 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with many returns (count = 4): toStepOutcome	10
qlty	Structure	Function with high complexity (count = 11): toStepOutcome	8
qlty	Structure	Function with many parameters (count = 4): forwardRequest	4

[Scra3]

Closing duplicate — commit already covered by #1564

[macroscopeapp]

🟢 Low

agent-nodejs/packages/workflow-executor/src/executors/load-related-record-step-executor.ts

Line 132 in bafb247

private async resolveAndLoadAutomatic(target: RelationTarget): Promise<StepExecutionResult> {

Adding 'BelongsToMany' to RelationTarget.relationType without updating resolveAndLoadAutomatic causes BelongsToMany relations to fall through to fetchFirstCandidate, which returns an arbitrary first record. Since BelongsToMany is a many-to-many relationship with multiple candidates, this bypasses the AI-based selection that HasMany receives via selectBestRelatedRecord.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/executors/load-related-record-step-executor.ts around line 132:

Adding `'BelongsToMany'` to `RelationTarget.relationType` without updating `resolveAndLoadAutomatic` causes `BelongsToMany` relations to fall through to `fetchFirstCandidate`, which returns an arbitrary first record. Since `BelongsToMany` is a many-to-many relationship with multiple candidates, this bypasses the AI-based selection that `HasMany` receives via `selectBestRelatedRecord`.

Evidence trail:
packages/workflow-executor/src/executors/load-related-record-step-executor.ts:36 (RelationTarget type includes BelongsToMany), lines 132-136 (resolveAndLoadAutomatic only checks HasMany), lines 239-241 (fetchFirstCandidate fetches limit=1), lines 232-235 (selectBestRelatedRecord uses AI with 50 records). PR diff confirms BelongsToMany was added in this PR to line 36 but resolveAndLoadAutomatic was not updated. packages/workflow-executor/src/types/validated/collection.ts:42 confirms BelongsToMany is a valid schema relationType.

[macroscopeapp]

🟢 Low executors/update-record-step-executor.ts:76

In buildZodSchemaForField, when type is an array of JSON object structures like [{ field: 'String' }], the check at line 81 only handles nested arrays, not nested objects. Array.isArray(type[0]) is false for objects, so line 83 passes the object to buildZodSchemaForPrimitive as a string, causing it to fall through to the default case and return z.string(). This validates arrays of structured JSON as arrays of plain strings instead of treating them as opaque JSON.

-function buildZodSchemaForField(field: FieldSchema): z.ZodTypeAny {
-  const { type, enumValues } = field;
-
-  if (Array.isArray(type)) {
-    // Nested array (e.g. [['String']]) → treat as opaque JSON.
-    if (Array.isArray(type[0])) return z.array(jsonStringSchema);
-
-    return z.array(buildZodSchemaForPrimitive(type[0] as string, enumValues));
-  }
-
-  if (typeof type === 'object' && type !== null) {
-    return jsonStringSchema;
-  }
-
-  return buildZodSchemaForPrimitive(type as string, enumValues);
-}

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/executors/update-record-step-executor.ts around lines 76-91:

In `buildZodSchemaForField`, when `type` is an array of JSON object structures like `[{ field: 'String' }]`, the check at line 81 only handles nested arrays, not nested objects. `Array.isArray(type[0])` is false for objects, so line 83 passes the object to `buildZodSchemaForPrimitive` as a string, causing it to fall through to the default case and return `z.string()`. This validates arrays of structured JSON as arrays of plain strings instead of treating them as opaque JSON.

Evidence trail:
packages/workflow-executor/src/types/validated/collection.ts lines 26-33 (ColumnType definition showing `[{field:'String'}]` is valid), packages/workflow-executor/src/executors/update-record-step-executor.ts lines 76-91 (`buildZodSchemaForField` missing object-in-array handling), lines 42-74 (`buildZodSchemaForPrimitive` switch defaulting to `z.string()` for non-string input)

[macroscopeapp]

🟠 High src/cli-core.ts:153

maxChainDepth is parsed with parsePositiveIntEnv, which uses z.coerce.number().int().positive() and rejects 0. According to the interface comment in ExecutorOptions, 0 is a valid value that disables chaining, so users setting MAX_CHAIN_DEPTH=0 receive a validation error instead of disabling chaining. Consider using a non-negative integer parser for this field.

-    maxChainDepth: parsePositiveIntEnv('MAX_CHAIN_DEPTH', env.MAX_CHAIN_DEPTH),
+    maxChainDepth: parseNonNegativeIntEnv('MAX_CHAIN_DEPTH', env.MAX_CHAIN_DEPTH),

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/cli-core.ts around line 153:

`maxChainDepth` is parsed with `parsePositiveIntEnv`, which uses `z.coerce.number().int().positive()` and rejects `0`. According to the interface comment in `ExecutorOptions`, `0` is a valid value that disables chaining, so users setting `MAX_CHAIN_DEPTH=0` receive a validation error instead of disabling chaining. Consider using a non-negative integer parser for this field.

Evidence trail:
packages/workflow-executor/src/cli-core.ts:17 - `const POSITIVE_INT = z.coerce.number().int().positive();`
packages/workflow-executor/src/cli-core.ts:19-29 - `parsePositiveIntEnv` function using POSITIVE_INT
packages/workflow-executor/src/cli-core.ts:153 - `maxChainDepth: parsePositiveIntEnv('MAX_CHAIN_DEPTH', env.MAX_CHAIN_DEPTH)`
packages/workflow-executor/src/build-workflow-executor.ts:43-44 - `// Max auto-chained steps per entry (see RunnerConfig.maxChainDepth). 0 disables chaining.` and `maxChainDepth?: number;`
packages/workflow-executor/test/runner.test.ts:897-910 - test 'disables chaining when maxChainDepth is 0' using `maxChainDepth: 0`

[macroscopeapp]

🟢 Low executors/trigger-record-action-step-executor.ts:182

saveFrontendResult persists the execution result but never sets idempotencyPhase: 'done'. When checkIdempotency runs on a frontend-completed step, existing?.idempotencyPhase === 'done' is false, so the check returns null instead of short-circuiting with success. This allows the step to be re-executed on retry, unlike executeOnExecutor which correctly sets the phase.

Also found in 1 other location(s)

packages/workflow-executor/src/executors/update-record-step-executor.ts:228

If saveStepExecution fails after updateRecord succeeds at line 228-236, the record is updated in the database but the idempotency phase remains 'executing'. On retry, checkIdempotency() at lines 118-119 sees 'executing' and throws StepStateError, telling the user to retry manually. However, the update already succeeded, so a manual retry would update the record again, causing duplicate updates. The previous code wrapped this in a try-catch with StepPersistenceError that explicitly stated 'Record update persisted but step state could not be saved', giving context that the update succeeded despite the save failure.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/executors/trigger-record-action-step-executor.ts around line 182:

`saveFrontendResult` persists the execution result but never sets `idempotencyPhase: 'done'`. When `checkIdempotency` runs on a frontend-completed step, `existing?.idempotencyPhase === 'done'` is false, so the check returns `null` instead of short-circuiting with success. This allows the step to be re-executed on retry, unlike `executeOnExecutor` which correctly sets the phase.

Evidence trail:
packages/workflow-executor/src/executors/trigger-record-action-step-executor.ts lines 45-59 (checkIdempotency), lines 132-137 (Branch C initial save, no idempotencyPhase), lines 143-172 (executeOnExecutor sets idempotencyPhase: 'done' at line 168), lines 175-192 (saveFrontendResult spreads existingExecution but never adds idempotencyPhase: 'done'). packages/workflow-executor/src/executors/base-step-executor.ts lines 248-272 (handleConfirmationFlow does not set idempotencyPhase). packages/workflow-executor/src/types/step-execution-data.ts lines 13-14 (MutatingStepExecutionData defines optional idempotencyPhase).

Also found in 1 other location(s):
- packages/workflow-executor/src/executors/update-record-step-executor.ts:228 -- If `saveStepExecution` fails after `updateRecord` succeeds at line 228-236, the record is updated in the database but the idempotency phase remains 'executing'. On retry, `checkIdempotency()` at lines 118-119 sees 'executing' and throws `StepStateError`, telling the user to retry manually. However, the update already succeeded, so a manual retry would update the record again, causing duplicate updates. The previous code wrapped this in a try-catch with `StepPersistenceError` that explicitly stated 'Record update persisted but step state could not be saved', giving context that the update succeeded despite the save failure.

[macroscopeapp]

🟢 Low adapters/run-to-available-step-mapper.ts:63

The guidance branch at line 64 only checks for 'error' status, mapping any other value (including 'awaiting-input') to 'success'. This drops the 'awaiting-input' state that GuidanceStepOutcomeSchema supports via RecordStepStatusSchema. Unlike the mcp and record branches which correctly preserve 'awaiting-input' through toRecordStatus(), a guidance step awaiting user input will incorrectly report as successful.

  if (outcomeType === 'guidance') {
-    const status: GuidanceStepOutcome['status'] = ctx.status === 'error' ? 'error' : 'success';
-
-    return { type: 'guidance', ...baseFromCtx, status };
+    const status = toRecordStatus(ctx.status);
+
+    return { type: 'guidance', ...baseFromCtx, status } satisfies GuidanceStepOutcome;
  }

Also found in 1 other location(s)

packages/ai-proxy/src/integrations/zendesk/tools/update-ticket.ts:17

The status enum removed 'hold' which is a valid Zendesk ticket status. According to Zendesk's API, valid ticket statuses are: new, open, pending, hold, solved, and closed. Removing 'hold' prevents users from setting tickets to the 'hold' status, which is a legitimate workflow state used to pause tickets awaiting external responses.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts around lines 63-67:

The `guidance` branch at line 64 only checks for `'error'` status, mapping any other value (including `'awaiting-input'`) to `'success'`. This drops the `'awaiting-input'` state that `GuidanceStepOutcomeSchema` supports via `RecordStepStatusSchema`. Unlike the `mcp` and `record` branches which correctly preserve `'awaiting-input'` through `toRecordStatus()`, a guidance step awaiting user input will incorrectly report as successful.

Evidence trail:
packages/workflow-executor/src/adapters/run-to-available-step-mapper.ts lines 30-35 (toRecordStatus), lines 63-66 (guidance branch), lines 69-75 (mcp/record branches); packages/workflow-executor/src/types/validated/step-outcome.ts lines 11 (RecordStepStatusSchema), lines 57-64 (GuidanceStepOutcomeSchema using RecordStepStatusSchema)

Also found in 1 other location(s):
- packages/ai-proxy/src/integrations/zendesk/tools/update-ticket.ts:17 -- The `status` enum removed `'hold'` which is a valid Zendesk ticket status. According to Zendesk's API, valid ticket statuses are: new, open, pending, hold, solved, and closed. Removing `'hold'` prevents users from setting tickets to the 'hold' status, which is a legitimate workflow state used to pause tickets awaiting external responses.