refactor(workflow-executor): workflow steps#1502
refactor(workflow-executor): workflow steps#1502Scra3 merged 56 commits intofeat/prd-214-setup-workflow-executor-packagefrom
Conversation
…on flow Implements PRD-219. Adds an executor for update-record steps with three execution branches: automatic completion, awaiting user confirmation, and re-entry after confirmation. Extracts shared record selection helpers from ReadRecordStepExecutor into BaseStepExecutor for reuse. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cordStepExecutor Match on type + stepIndex only, then guard on toolConfirmationInterruption presence separately. Removes redundant filter predicate. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…endingUpdate The field stores the proposed field update (from AI or user-edited) pending confirmation. "pendingUpdate" better describes the content than "toolConfirmationInterruption" which described the mechanism. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…f error outcome Missing pendingUpdate in Branch A is a bug (orchestrator/RunStore), not a business case — throw WorkflowExecutorError instead of returning a graceful error outcome. Also rename local variable interruption → execution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… casting
Add { skipped: true } to the executionResult union type, removing the
unsafe `as unknown as` cast when the user rejects the update.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… traceability Keep pendingUpdate in the saved execution data after both confirm and reject — useful for audit trail (what was proposed vs what happened). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ecutor
Pass userInput as parameter to handleConfirmation() where it is already
narrowed, removing the `as { confirmed: boolean }` cast. Also move
resolveFieldName inside try-catch blocks so WorkflowExecutorError is
properly caught and returned as a status: 'error' outcome.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…prove coverage - Extract resolveAndUpdate to deduplicate resolve+update+persist logic - Extract buildOutcomeResult/buildSuccessResult/buildErrorResult helpers - Move getCollectionSchema inside try-catch in confirmation flow - Rename executionParams.fieldName to fieldDisplayName for consistency - Add tests for resolveFieldName failure in both branches - Add test for relationship field exclusion from update tool schema Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dErrorResult wrappers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ment executionResult Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…StepExecutionData Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…field resolution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move findField to BaseStepExecutor with displayName priority over fieldName - Move buildOutcomeResult to BaseStepExecutor, use error !== undefined check - Fix resolveAndUpdate try-catch scope (saveStepExecution outside try block) - Fix ConditionStepExecutor catch-all to only catch WorkflowExecutorError - Add pendingUpdate visibility in step summary - Remove findField from types/record.ts and barrel export Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Condition steps handle BPMN routing, record-task steps operate on client data. The previous "ai-task" name was misleading since both step types use AI. Rename to "record-task" to reflect the actual distinction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Definition Drop recordSourceStepId and remoteToolsSourceId — both were declared but never read. allowedTools is kept as the orchestrator-provided tool filter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduces StepType.ToolTask ('tool-task') and ToolTaskStepDefinition for
steps where the AI freely selects and executes a tool from allowedTools.
Moves allowedTools out of RecordTaskStepDefinition where it didn't belong.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…on type Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…AndUpdate arity
- Extract UpdateTarget interface {selectedRecordRef, fieldDisplayName, value} so
resolveAndUpdate takes 2 params instead of 4 (addresses qlty reviewer comment)
- Collapse 3 uninitialized let declarations in handleFirstCall into one target object
- Fix missing runId in getAvailableRecordRefs and update-record saveStepExecution calls
- Strengthen test assertions to include runId as first arg
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ecutor Code changes: - Add JSDoc on buildOutcomeResult warning it is for record-task executors only - Add runtime guard on userInput.type in handleConfirmation (WorkflowExecutorError if an unexpected type is received, guarding against future UserInput union extension) New tests: - buildStepSummary: pendingUpdate branch coverage (update-record step with pendingUpdate but no executionParams emits Pending: in AI context) - handleConfirmation: stepIndex mismatch and missing pendingUpdate cases - stepOutcome shape: type/stepId/stepIndex asserted on happy path - unexpected userInput type: runtime guard verified - findField fieldName fallback: update succeeds when AI returns raw fieldName - schema caching: getCollectionSchema called once per collection in Branch B - RunStore error propagation: all four saveStepExecution/getStepExecutions call sites Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pletion→automaticExecution - StepType.ToolTask → StepType.McpTask (wire value: 'tool-task' → 'mcp-task') - ToolTaskStepDefinition → McpTaskStepDefinition - mcpServerId: string[] → string (single ID) - automaticCompletion → automaticExecution on both RecordTaskStepDefinition and McpTaskStepDefinition - Remove TODO rename comments - Update all tests accordingly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…record-1' Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the UserInput discriminated union with a plain boolean field userConfirmed on ExecutionContext and PendingStepExecution. Since the only user input type in practice is a boolean confirmation, the union wrapper adds complexity with no benefit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ion flow
Implements TriggerActionStepExecutor following the UpdateRecordStepExecutor
pattern (branches A/B/C, confirmation flow, automaticExecution).
- Add TriggerActionStepExecutionData type with executionParams (actionDisplayName
+ actionName), executionResult ({ success } | { skipped }), and pendingAction
- Add NoActionsError for collections with no actions
- Implement selectAction via AI tool with displayName enum and technical name hints
- resolveAndExecute stores the technical actionName in executionParams for
traceability; action result discarded per privacy constraint
- Fix buildStepSummary in BaseStepExecutor to include trigger-action pendingAction
in prior-step AI context (parity with update-record pendingUpdate)
- Export TriggerActionStepExecutor, TriggerActionStepExecutionData, NoActionsError
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t and pendingAction Resolve actionName once in handleFirstCall and store it in pendingAction, so resolveAndExecute receives it directly via ActionTarget without re-fetching the schema. Rename TriggerTarget → ActionTarget for consistency with English naming conventions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…f shapes to { name, displayName }
- Extract ActionRef { name, displayName } from inline types in TriggerActionStepExecutionData
- Extract FieldRef { name, displayName } replacing FieldReadBase in ReadRecord types
- UpdateRecordStepExecutionData executionParams/pendingUpdate now use FieldRef & { value }
- Rename actionDisplayName/actionName → displayName/name, fieldDisplayName/fieldName → displayName/name
- Move resolveFieldName to handleFirstCall (no re-resolution in resolveAndUpdate)
- Add missing tests: resolveActionName not-found path, saveStepExecution not-called assertions, trigger-action pendingAction in buildStepSummary
- Export ActionRef, FieldRef, NoActionsError from index; update CLAUDE.md
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…store display names in executionParams Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rams for consistency Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-executor Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ser facing messages Separates technical error messages (dev logs) from user-facing messages (Forest Admin UI). WorkflowExecutorError now carries a readonly `userMessage` field; base-step-executor uses it instead of `message` when building the step outcome error. Each subclass declares its own user-oriented message. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nject logger into execution context Introduces a Logger interface (port) and ConsoleLogger (default implementation). Adds logger to ExecutionContext, masks raw error messages from the HTTP API (security), and logs unhandled HTTP errors with context instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eLogger output Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ated-record executor, rename ids→id
- Add StepExecutionFormatters (static per-type formatters) and StepSummaryBuilder
(orchestrates step summary for AI context); decouple formatting from BaseStepExecutor
- Load-related-record executionResult is now self-contained: { relation: RelationRef; record: RecordRef }
- Rename ids → id in AgentPort and all callers (id is a composite key of one record, not multiple records)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ve summary to sub-folder - Replace QueryBase intersections with named query types (GetRecordQuery, UpdateRecordQuery, GetRelatedDataQuery, ExecuteActionQuery) for better DX and readability - Save executeAction return value in executionResult.actionResult instead of discarding it - Move StepSummaryBuilder and StepExecutionFormatters to executors/summary/ sub-folder Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…edData signature Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… into McpTaskStepExecutor remoteTools was a dependency of McpTaskStepExecutor only — 5 of 6 executors never used it. Inject it explicitly via the constructor so ExecutionContext stays focused on execution state, and remove the now-meaningless remoteTools: [] boilerplate from every other executor test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…utor Any WorkflowExecutorError with a cause is now logged automatically by the base executor using error.message as the log message. Removes the manual logger.error call from McpTaskStepExecutor and the StepPersistenceError- specific guard. Moves cause? up to the base class, removing duplicate declarations from StepPersistenceError and McpToolInvocationError. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add AgentPortError and SafeAgentPort to centralise infra error handling for all agentPort operations (getRecord, updateRecord, getRelatedData, executeAction). - add AgentPortError extending WorkflowExecutorError with a user-friendly message and structured cause logging - add SafeAgentPort implementing AgentPort: wraps infra errors in AgentPortError, passes through WorkflowExecutorError subclasses unchanged - expose this.agentPort (SafeAgentPort) as a protected property in BaseStepExecutor; all executors use it instead of this.context.agentPort - export AgentPortError from index.ts - add unit tests for SafeAgentPort and one integration test per executor verifying userMessage and logger.error cause on infra failures Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ask dependencies Add McpToolRef, McpToolCall and McpTaskStepExecutionData to step-execution-data, required by mcp-task-step-executor. Update package.json to depend on @forestadmin/ai-proxy and align @langchain/core version. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Export RemoteTool default, BaseChatModel, BaseMessage, HumanMessage, SystemMessage, StructuredToolInterface and DynamicStructuredTool so consumers of ai-proxy do not need a direct @langchain/core dependency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…restadmin/ai-proxy Replace all direct @langchain/core imports in src and tests with @forestadmin/ai-proxy re-exports. Add moduleNameMapper in jest.config.ts to resolve @anthropic-ai/sdk wildcard exports (Jest < 30 workaround, same pattern as ai-proxy). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@langchain/core is now consumed transitively via @forestadmin/ai-proxy. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Implement polling loop with configurable interval and auto-reschedule - Add getExecutor() factory dispatching all StepTypes to their executor - Add triggerPoll() for webhook-driven execution - Add in-flight step deduplication via inFlightSteps Set - Add MCP lazy tool loading with once() thunk - Replace McpConfiguration placeholder type with @forestadmin/ai-proxy type - Fix missing stepIndex field in step-scoped error logs - Fix missing cause field in unexpected error logs in BaseStepExecutor - Add comprehensive test coverage for Runner Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| return this.config.aiClient.loadRemoteTools(mergedConfig); | ||
| } | ||
|
|
||
| private async executeStep( |
There was a problem hiding this comment.
🟡 Medium src/runner.ts:182
inFlightSteps.delete(key) runs in the finally block before updateStepExecution is awaited, so a concurrent poll cycle can re-execute the step before the backend marks it complete. Move the delete to after updateStepExecution succeeds, or await the update inside the try block before cleanup.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/workflow-executor/src/runner.ts around line 182:
`inFlightSteps.delete(key)` runs in the `finally` block before `updateStepExecution` is awaited, so a concurrent poll cycle can re-execute the step before the backend marks it complete. Move the delete to after `updateStepExecution` succeeds, or await the update inside the try block before cleanup.
Evidence trail:
packages/workflow-executor/src/runner.ts lines 183-220 (REVIEWED_COMMIT): executeStep method shows `inFlightSteps.add(key)` at line 188, `finally { this.inFlightSteps.delete(key); }` at lines 210-212, and `await this.config.workflowPort.updateStepExecution(...)` at line 215 which is outside and after the try-catch-finally block.
packages/workflow-executor/src/runner.ts lines 153-166 (REVIEWED_COMMIT): runPollCycle method fetches pending steps from backend and filters using `!this.inFlightSteps.has(stepKey(s))`, showing the race window where a step could be re-fetched and re-executed.
After a MCP tool runs, trigger a second AI call that produces a concise human-readable summary stored as `formattedResponse` in `McpTaskStepExecutionData.executionResult`. The raw result is persisted first (safe state); the formatting step is non-blocking — failures are logged but never fail the step. Results longer than 20 000 chars are truncated before injection. `StepExecutionFormatters` now handles `mcp-task`: returns `Result: <summary>` when available, or a generic fallback line to avoid exposing raw `toolResult` in subsequent AI context messages. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…askStepExecutor Move handleConfirmationFlow() up to BaseStepExecutor so it is reusable by any executor. McpTaskStepExecutor now extends BaseStepExecutor directly instead of RecordTaskStepExecutor, and emits its own type 'mcp-task' outcome. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tic methods on Runner once, stepKey, stepOutcomeType, causeMessage are now private static methods on the Runner class instead of loose module-level functions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tory Move step dispatch logic from runner.ts into a dedicated StepExecutorFactory.create() static method in the executors/ folder, co-locating it with its peers. Runner is now solely responsible for orchestration; step type routing lives in the factory. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pExecutor - Add IStepExecutor interface to execution.ts - Add stepTypeToOutcomeType() helper to step-outcome.ts - Add ErrorStepExecutor: catches construction errors, returns typed error outcome - StepExecutorFactory absorbs buildContext, takes StepContextConfig, never throws - Runner simplified: removes buildContext, stepOutcomeType, StepOutcome knowledge - Extract causeMessage() to errors.ts (shared utility, removes DRY violation) - BaseStepExecutor explicitly implements IStepExecutor - Add error-step-executor.test.ts with full coverage (contract, logging, type mapping) - Strengthen runner.test.ts: FATAL path, loadTools rejection, non-Error throwable, stepTypeToOutcomeType unit tests, deduplication assertion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…andling in factory ErrorStepExecutor had no meaningful state once logging was moved out of it. The factory catch block now logs directly and returns an inline IStepExecutor. stepTypeToOutcomeType unit tests moved to test/types/step-outcome.test.ts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…iggerPoll - Add RunNotFoundError (extends Error, not WorkflowExecutorError) - Add getPendingStepExecutionsForRun(runId) to WorkflowPort (returns PendingStepExecution | null) - Implement in ForestServerWorkflowPort with runId query param + URL encoding - triggerPoll uses the new method, throws RunNotFoundError if step is null - handleTrigger returns 404 on RunNotFoundError, rethrows otherwise Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| ); | ||
| } | ||
|
|
||
| async getPendingStepExecutionsForRun(runId: string): Promise<PendingStepExecution | null> { |
There was a problem hiding this comment.
🟢 Low adapters/forest-server-workflow-port.ts:34
getPendingStepExecutionsForRun queries a list endpoint that returns an array, but declares return type PendingStepExecution | null. The server will return an array at runtime, causing callers to receive an array typed as a single object. Consider changing the return type to Promise<PendingStepExecution[]> to match the actual response shape.
| async getPendingStepExecutionsForRun(runId: string): Promise<PendingStepExecution | null> { | |
| async getPendingStepExecutionsForRun(runId: string): Promise<PendingStepExecution[]> { |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/workflow-executor/src/adapters/forest-server-workflow-port.ts around line 34:
`getPendingStepExecutionsForRun` queries a list endpoint that returns an array, but declares return type `PendingStepExecution | null`. The server will return an array at runtime, causing callers to receive an array typed as a single object. Consider changing the return type to `Promise<PendingStepExecution[]>` to match the actual response shape.
Evidence trail:
- packages/workflow-executor/src/adapters/forest-server-workflow-port.ts lines 10-16 (ROUTES definitions showing same base endpoint)
- packages/workflow-executor/src/adapters/forest-server-workflow-port.ts lines 25-39 (both methods using same endpoint with different return types)
- packages/forestadmin-client/src/utils/server.ts line 72 (ServerUtils.query returns response.body directly without transformation)
- packages/workflow-executor/src/runner.ts lines 91-96 (usage of result as single object)
- packages/workflow-executor/src/ports/workflow-port.ts lines 11-12 (interface definition)
…a — replace suggestedRecordId + selectedRecordId? with a single selectedRecordId The executor now writes the AI's pick directly into selectedRecordId. The future PATCH endpoint will overwrite it with the user's choice, removing the need to distinguish between "suggested" and "selected". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ontract doc Routes for the pending-data write endpoint are not yet defined (PRD-240). Replace the made-up PATCH route with explicit TODO markers throughout. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
39de72a
into
feat/prd-214-setup-workflow-executor-package

Summary
ActionRefandFieldRefshared interfaces ({ name, displayName }) from inline types inTriggerActionStepExecutionDataandUpdateRecordStepExecutionDataFieldReadBase(private) with exportedFieldRef—FieldReadSuccess/FieldReadErrornow extendFieldRefactionDisplayName/actionName→displayName/nameandfieldDisplayName/fieldName→displayName/namefor consistencyresolveFieldNametohandleFirstCallso the technical field name is stored inpendingUpdateat creation time (no re-resolution inresolveAndUpdate)resolveActionNamenot-found path,saveStepExecutionnot-called assertions onWorkflowExecutorError,trigger-actionpending action inbuildStepSummaryActionRef,FieldRef,NoActionsErrorfrom index; updateCLAUDE.mdTest plan
yarn workspace @forestadmin/workflow-executor test— 144 tests passyarn workspace @forestadmin/workflow-executor build— no errorsyarn workspace @forestadmin/workflow-executor lint— clean🤖 Generated with Claude Code
Note
Add new workflow step executors for record update, action trigger, related record loading, and MCP tasks
UpdateRecordStepExecutor,TriggerRecordActionStepExecutor,LoadRelatedRecordStepExecutor, andMcpTaskStepExecutor, each supporting automatic execution and user confirmation flows.BaseStepExecutorto provide a unifiedexecute()wrapper with structured error handling, confirmation flow helpers, tool invocation via@forestadmin/ai-proxy, schema caching, and previous-step summaries viaStepSummaryBuilder.StepExecutorFactoryto centralize executor instantiation; on factory errors, returns a fallback executor yielding a standardized error outcome.Runnerinternals: HTTP server startup, polling loop with in-flight deduplication, MCP tool lazy loading, and outcome reporting toWorkflowPort.errors.tswith ~15 new typed error classes, each carrying auserMessagefor frontend display.AiTaskStepDefinition/AiTaskStepOutcomeare replaced byRecordTaskStepDefinition/RecordTaskStepOutcome;isExecutedStepOnExecutoris no longer exported;AgentPortmethods now accept single query objects instead of positional parameters.Macroscope summarized a5d6249.