diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index 9a3dabe201..a05a821658 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -91,7 +91,7 @@ src/ - **Idempotency in mutating executors** — `update-record`, `trigger-action`, and `mcp` executors protect against duplicate side effects via a write-ahead log in the `RunStore`. Before the side effect fires, the executor saves `idempotencyPhase: 'executing'`. After, it saves `idempotencyPhase: 'done'` alongside the normal `executionResult`. On re-dispatch (same `runId + stepIndex`): `done` → reconstruct success outcome via `buildOutcomeResult` without re-executing or emitting an activity log; `executing` → throw `StepStateError` (user retries manually, also no activity log). The `checkIdempotency()` hook in `BaseStepExecutor` is called before `runWithActivityLog()` so neither cache hits nor uncertain-state errors emit activity log entries. Non-mutating executors (`condition`, `read-record`, `guidance`, `load-related-record`) do not override `checkIdempotency()` — replaying them is safe. - **Fetched steps must be executed** — Any step retrieved from the orchestrator via `getAvailableRuns()` must be executed. Silently discarding a fetched step (e.g. filtering it out by `runId` after fetching) violates the executor contract: the orchestrator assumes execution is guaranteed once the step is dispatched. The only valid filter before executing is deduplication via `inFlightRuns` (keyed by `runId`, to avoid running the same run twice concurrently; the key is the run, not the step, because a chain advances the `stepId` between iterations). - **Auto-chain from `/update-step` response** — `WorkflowPort.updateStepExecution` returns `AvailableRunDispatch | null`: when non-null, the `Runner` executes the next step inline instead of waiting for the next poll. The chain exits on `null` (awaiting-input / finished / error), on a non-progressing `stepIndex` (server bug defense), at `maxChainDepth` (config, default 50), or when `stop()` is called. Each chained step uses the `forestServerToken` from its own dispatch — token freshness is preserved across the chain. The port retries `POST /update-step` on transient failures (network, 5xx) — this relies on server-side idempotency: the orchestrator MUST deduplicate identical outcomes for a given `(runId, stepIndex)` to prevent double side-effects on retry. -- **Pre-recorded AI decisions** — Record step executors support `preRecordedArgs` in the step definition to bypass AI calls. When provided, executors use the pre-recorded values (display names) directly instead of invoking the AI. Each record step type has its own typed `preRecordedArgs` shape. Validation happens via schema resolution — invalid display names throw `InvalidPreRecordedArgsError`. Partial args are supported: only the provided fields skip AI, the rest still use AI. +- **Pre-recorded AI decisions** — Record step executors support `preRecordedArgs` in the step definition to bypass AI calls. When provided, executors use the pre-recorded **technical names** (`fieldName`/`fieldNames`/`actionName`/`relationName`) directly instead of invoking the AI — the orchestrator→executor wire references fields/relations/actions by their stable technical name, never by the mutable, non-unique `displayName`. The `displayName` persisted in the RunStore is always resolved from the live schema at execution time (still persisted for the AI and for the front — see "displayName in AI tools"). Technical names are matched exactly against the schema (`findFieldByTechnicalName` / the exact action lookup) — the displayName + fuzzy tolerances of `findField` are reserved for AI-returned names, so a technical name can't resolve to a different field whose displayName collides. Each record step type has its own typed `preRecordedArgs` shape. An unresolvable name throws `FieldNotFoundError` / `ActionNotFoundError` / `RelationNotFoundError` (read-record instead throws `NoResolvedFieldsError`, only when *no* field resolves — individual misses are surfaced per-field). Malformed arg shapes — e.g. `fieldName` without `value`, or an out-of-range `selectedRecordStepIndex` — throw `InvalidPreRecordedArgsError`. Partial args are supported: only the provided fields skip AI, the rest still use AI. - **Graceful shutdown** — `stop()` drains in-flight steps before closing resources. The `state` getter exposes the lifecycle: `idle → running → draining → stopped`. `stopTimeoutMs` (default 30s) prevents `stop()` from hanging forever if a step is stuck. The HTTP server stays up during drain so the frontend can still query run status. Signal handling (`SIGTERM`/`SIGINT`) is the consumer's responsibility — the Runner is a library class. - **Structured log context** — `BaseStepExecutor.execute()` stamps every log line with a shared `logCtx` (`runId`, `stepId`, `stepIndex`, `stepType`). Executors with type-specific identifiers add them via the `getExtraLogContext()` hook (default `{}`), keeping the base class free of step-specific knowledge — e.g. `McpStepExecutor` returns `{ mcpServerId, mcpServerName }` so MCP step logs unambiguously identify the targeted server (`mcpServerId` is canonical; `mcpServerName` is the human-readable Record key, not guaranteed unique at the DB level). `mcpServerName` is resolved by `RemoteToolFetcher.fetch()` from the scoped config Record key and forwarded to the executor constructor. - **Boundary validation** — Types that cross a trust boundary (wire from the orchestrator, or mapper output) live under `src/types/validated/` as zod schemas with TS types inferred via `z.infer<>`. Strictness depends on origin: schemas the executor **produces** (mapper output) and **frontend** HTTP bodies use `.strict()` (catch our own bugs / input hygiene); the **orchestrator collection schema** instead **strips** unknown keys and requires only structural fields, with step-specific props optional and asserted at use-time by the consuming executor. This keeps the executor resilient to independent orchestrator drift — we fail at step execution, only when a step genuinely lacks what it needs, never in bulk up front for an unrelated add/remove. Validation runs where data enters (`forest-server-workflow-port.getCollectionSchema`, `run-to-available-step-mapper.toAvailableStepExecution`). On parse failure: throw `DomainValidationError` (extends `WorkflowExecutorError`) → bucketized as malformed (dispatch) or surfaced as a step error (execution). Types outside `validated/` are internal runtime state and not zod-validated. Note: `StepOutcome` is validated when it arrives as input via `previousSteps`; executor outputs are trusted by construction. diff --git a/packages/workflow-executor/src/executors/load-related-record-step-executor.ts b/packages/workflow-executor/src/executors/load-related-record-step-executor.ts index 232e359f38..540b6b6d52 100644 --- a/packages/workflow-executor/src/executors/load-related-record-step-executor.ts +++ b/packages/workflow-executor/src/executors/load-related-record-step-executor.ts @@ -124,10 +124,10 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { + ): Promise<{ relationName: string }> { const tool = this.buildSelectRelationTool(schema); const messages = [ this.buildContextMessage(), @@ -481,7 +481,12 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor(messages, tool); + const { relationName } = await this.invokeWithTool<{ relationName: string; reasoning: string }>( + messages, + tool, + ); + + return { relationName: this.resolveAiFieldName(schema, relationName) }; } private buildSelectRelationTool(schema: CollectionSchema): DynamicStructuredTool { diff --git a/packages/workflow-executor/src/executors/read-record-step-executor.ts b/packages/workflow-executor/src/executors/read-record-step-executor.ts index b6c761632d..2e0f0f060c 100644 --- a/packages/workflow-executor/src/executors/read-record-step-executor.ts +++ b/packages/workflow-executor/src/executors/read-record-step-executor.ts @@ -1,7 +1,7 @@ import type { CreateActivityLogArgs } from '../ports/activity-log-port'; import type { StepExecutionResult } from '../types/execution-context'; import type { FieldReadResult } from '../types/step-execution-data'; -import type { CollectionSchema } from '../types/validated/collection'; +import type { CollectionSchema, FieldSchema } from '../types/validated/collection'; import type { ReadRecordStepDefinition } from '../types/validated/step-definition'; import { DynamicStructuredTool, HumanMessage, SystemMessage } from '@forestadmin/ai-proxy'; @@ -40,14 +40,19 @@ export default class ReadRecordStepExecutor extends RecordStepExecutor this.findField(schema, name)?.fieldName) + const fieldNames = + preRecordedArgs?.fieldNames ?? (await this.selectFields(schema, step.prompt)); + const selectedFields = fieldNames.map(requested => ({ + requested, + field: this.findFieldByTechnicalName(schema, requested), + })); + + const resolvedFieldNames = selectedFields + .map(s => s.field?.fieldName) .filter((name): name is string => name !== undefined); if (resolvedFieldNames.length === 0) { - throw new NoResolvedFieldsError(selectedDisplayNames); + throw new NoResolvedFieldsError(selectedFields.map(s => s.requested)); } const recordData = await this.agentPort.getRecord( @@ -58,7 +63,7 @@ export default class ReadRecordStepExecutor extends RecordStepExecutor(messages, tool); + const { fieldNames } = await this.invokeWithTool<{ fieldNames: string[] }>(messages, tool); - return args.fieldNames; + return fieldNames.map(name => this.resolveAiFieldName(schema, name)); } private buildReadFieldTool(schema: CollectionSchema): DynamicStructuredTool { @@ -123,13 +128,12 @@ export default class ReadRecordStepExecutor extends RecordStepExecutor, - schema: CollectionSchema, - fieldDisplayNames: string[], + selected: Array<{ requested: string; field: FieldSchema | undefined }>, ): FieldReadResult[] { - return fieldDisplayNames.map(name => { - const field = this.findField(schema, name); - - if (!field) return { error: `Field not found: ${name}`, name, displayName: name }; + return selected.map(({ requested, field }) => { + if (!field) { + return { error: `Field not found: ${requested}`, name: requested, displayName: requested }; + } return { value: values[field.fieldName], diff --git a/packages/workflow-executor/src/executors/record-step-executor.ts b/packages/workflow-executor/src/executors/record-step-executor.ts index 4430dfacc4..39d91a0322 100644 --- a/packages/workflow-executor/src/executors/record-step-executor.ts +++ b/packages/workflow-executor/src/executors/record-step-executor.ts @@ -76,26 +76,32 @@ export default abstract class RecordStepExecutor< return schema; } - protected findField(schema: CollectionSchema, name: string): FieldSchema | undefined { - // LLMs occasionally return formatting variants of field names (e.g. "first_name" for - // "firstname", "full-name" for "Full Name") even though the tool schema declares them - // as literals. Fall back to a normalized comparison so a cosmetic variation doesn't - // fail an otherwise correct step. - const normalizeFieldName = (s: string) => s.toLowerCase().replace(/[\s_-]/g, ''); - const normalized = normalizeFieldName(name); + protected findFieldByTechnicalName( + schema: CollectionSchema, + name: string | undefined, + ): FieldSchema | undefined { + if (name === undefined) return undefined; + return schema.fields.find(f => f.fieldName === name); + } + + // Map an AI-returned displayName back to its technical fieldName. LLMs occasionally return + // formatting variants (e.g. "first_name" for "firstname", "full-name" for "Full Name"), so fall + // back to a normalized comparison. On a miss, returns the raw name — the exact lookup downstream + // turns it into a loud error. + protected resolveAiFieldName(schema: CollectionSchema, name: string): string { const exact = schema.fields.find(f => f.displayName === name) ?? schema.fields.find(f => f.fieldName === name); - if (exact) return exact; + if (exact) return exact.fieldName; + const normalize = (s: string) => s.toLowerCase().replace(/[\s_-]/g, ''); + const normalized = normalize(name); const fuzzy = schema.fields.filter( - f => - normalizeFieldName(f.displayName) === normalized || - normalizeFieldName(f.fieldName) === normalized, + f => normalize(f.displayName) === normalized || normalize(f.fieldName) === normalized, ); - return fuzzy.length === 1 ? fuzzy[0] : undefined; + return fuzzy.length === 1 ? fuzzy[0].fieldName : name; } private async toRecordIdentifier(record: RecordRef): Promise { 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 3f8d09e504..54dfb58564 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 @@ -1,7 +1,7 @@ import type { CreateActivityLogArgs } from '../ports/activity-log-port'; import type { StepExecutionResult } from '../types/execution-context'; import type { ActionRef, TriggerRecordActionStepExecutionData } from '../types/step-execution-data'; -import type { CollectionSchema, RecordRef } from '../types/validated/collection'; +import type { ActionSchema, CollectionSchema, RecordRef } from '../types/validated/collection'; import type { TriggerActionStepDefinition } from '../types/validated/step-definition'; import { DynamicStructuredTool, HumanMessage, SystemMessage } from '@forestadmin/ai-proxy'; @@ -108,11 +108,19 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< preRecordedArgs?.selectedRecordStepIndex, ); const schema = await this.getCollectionSchema(selectedRecordRef.collectionName); - const args = preRecordedArgs?.actionDisplayName - ? { actionName: preRecordedArgs.actionDisplayName } - : await this.selectAction(schema, step.prompt); - const name = this.resolveActionName(schema, args.actionName); - const target: ActionTarget = { selectedRecordRef, displayName: args.actionName, name }; + const recordedAction = preRecordedArgs?.actionName; + const actionName = recordedAction ?? (await this.selectAction(schema, step.prompt)).actionName; + const action = this.findActionByTechnicalName(schema, actionName); + + if (!action) { + throw new ActionNotFoundError(actionName, schema.collectionName); + } + + const target: ActionTarget = { + selectedRecordRef, + displayName: action.displayName, + name: action.name, + }; // Branch B -- fully automated: executor runs the action itself, so it cannot // handle forms (no UI to fill them). Reject form-bearing actions here. When the @@ -121,7 +129,7 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< const { hasForm } = await this.agentPort.getActionFormInfo( { collection: selectedRecordRef.collectionName, - action: name, + action: target.name, id: selectedRecordRef.recordId, }, this.context.user, @@ -197,7 +205,7 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< private async selectAction( schema: CollectionSchema, prompt: string | undefined, - ): Promise<{ actionName: string; reasoning: string }> { + ): Promise<{ actionName: string }> { const tool = this.buildSelectActionTool(schema); const messages = [ this.buildContextMessage(), @@ -209,7 +217,12 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< new HumanMessage(`**Request**: ${prompt ?? 'Trigger the relevant action.'}`), ]; - return this.invokeWithTool<{ actionName: string; reasoning: string }>(messages, tool); + const { actionName } = await this.invokeWithTool<{ actionName: string; reasoning: string }>( + messages, + tool, + ); + + return { actionName: this.findAction(schema, actionName)?.name ?? actionName }; } private buildSelectActionTool(schema: CollectionSchema): DynamicStructuredTool { @@ -235,15 +248,16 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< }); } - private resolveActionName(schema: CollectionSchema, displayName: string): string { - const action = - schema.actions.find(a => a.displayName === displayName) ?? - schema.actions.find(a => a.name === displayName); - - if (!action) { - throw new ActionNotFoundError(displayName, schema.collectionName); - } + private findActionByTechnicalName( + schema: CollectionSchema, + name: string, + ): ActionSchema | undefined { + return schema.actions.find(a => a.name === name); + } - return action.name; + private findAction(schema: CollectionSchema, name: string): ActionSchema | undefined { + return ( + schema.actions.find(a => a.displayName === name) ?? schema.actions.find(a => a.name === name) + ); } } diff --git a/packages/workflow-executor/src/executors/update-record-step-executor.ts b/packages/workflow-executor/src/executors/update-record-step-executor.ts index f978e16d04..9b0f8c28ad 100644 --- a/packages/workflow-executor/src/executors/update-record-step-executor.ts +++ b/packages/workflow-executor/src/executors/update-record-step-executor.ts @@ -189,9 +189,7 @@ export default class UpdateRecordStepExecutor extends RecordStepExecutor { const schema = await this.getCollectionSchema(selectedRecordRef.collectionName); - const fieldSchema = - this.findField(schema, pendingData?.name ?? '') ?? - this.findField(schema, pendingData?.displayName ?? ''); + const fieldSchema = this.findFieldByTechnicalName(schema, pendingData?.name); return coerceFieldValue(fieldSchema, value, selectedRecordRef.collectionName); } @@ -208,28 +206,34 @@ export default class UpdateRecordStepExecutor extends RecordStepExecutor { + ): Promise<{ fieldName: string; value: unknown }> { const tool = this.buildUpdateFieldTool(schema); const messages = [ this.buildContextMessage(), @@ -308,7 +312,10 @@ export default class UpdateRecordStepExecutor extends RecordStepExecutor(messages, tool); - return input; + return { + fieldName: this.resolveAiFieldName(schema, input.fieldName), + value: input.value, + }; } private buildUpdateFieldTool(schema: CollectionSchema): DynamicStructuredTool { @@ -347,14 +354,4 @@ export default class UpdateRecordStepExecutor extends RecordStepExecutor { runStore, stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated, - preRecordedArgs: { relationDisplayName: 'Order' }, + preRecordedArgs: { relationName: 'order' }, }), }); const executor = new LoadRelatedRecordStepExecutor(context); @@ -2744,6 +2744,16 @@ describe('LoadRelatedRecordStepExecutor', () => { expect(result.stepOutcome.status).toBe('success'); expect(bindTools).not.toHaveBeenCalled(); + // Pre-recorded reference is the technical name 'order'; the persisted displayName 'Order' + // is resolved from the schema, not received on the wire. + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionResult: expect.objectContaining({ + relation: { name: 'order', displayName: 'Order' }, + }), + }), + ); }); it('skips AI record selection when selectedRecordIndex is pre-recorded with HasMany', async () => { @@ -2768,7 +2778,7 @@ describe('LoadRelatedRecordStepExecutor', () => { agentPort: makeMockAgentPort(relatedData), stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated, - preRecordedArgs: { relationDisplayName: 'Address', selectedRecordIndex: 1 }, + preRecordedArgs: { relationName: 'address', selectedRecordIndex: 1 }, }), }); const executor = new LoadRelatedRecordStepExecutor(context); @@ -2806,7 +2816,7 @@ describe('LoadRelatedRecordStepExecutor', () => { agentPort: makeMockAgentPort(relatedData), stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated, - preRecordedArgs: { relationDisplayName: 'Address', selectedRecordIndex: 99 }, + preRecordedArgs: { relationName: 'address', selectedRecordIndex: 99 }, }), }); const executor = new LoadRelatedRecordStepExecutor(context); @@ -2816,6 +2826,71 @@ describe('LoadRelatedRecordStepExecutor', () => { expect(result.stepOutcome.status).toBe('error'); }); + it('returns error when a pre-recorded relationName does not resolve', async () => { + const { model } = makeMockModel(); + const context = makeContext({ + model, + stepDefinition: makeStep({ + executionType: StepExecutionMode.FullyAutomated, + preRecordedArgs: { relationName: 'does_not_exist' }, + }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + }); + + it('resolves a pre-recorded technical relationName to its own relation, not another whose displayName collides', async () => { + const { model } = makeMockModel(); + const runStore = makeMockRunStore(); + // Relation B's displayName ('order') equals relation A's technical fieldName ('order'). + const workflowPort = makeMockWorkflowPort({ + customers: makeCollectionSchema({ + fields: [ + { + fieldName: 'order', + displayName: 'Linked Address', + isRelationship: true, + relationType: 'BelongsTo', + relatedCollectionName: 'orders', + }, + { + fieldName: 'addr', + displayName: 'order', + isRelationship: true, + relationType: 'BelongsTo', + relatedCollectionName: 'addresses', + }, + ], + }), + }); + const context = makeContext({ + model, + runStore, + workflowPort, + stepDefinition: makeStep({ + executionType: StepExecutionMode.FullyAutomated, + preRecordedArgs: { relationName: 'order' }, + }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('success'); + // Follows relation A ('order' / 'Linked Address'), not B ('addr' / displayName 'order'). + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionResult: expect.objectContaining({ + relation: { name: 'order', displayName: 'Linked Address' }, + }), + }), + ); + }); + it('falls back to AI when no preRecordedArgs', async () => { const { model, bindTools } = makeMockModel({ relationName: 'Orders', reasoning: 'r' }); const context = makeContext({ diff --git a/packages/workflow-executor/test/executors/read-record-step-executor.test.ts b/packages/workflow-executor/test/executors/read-record-step-executor.test.ts index d483ce2d37..31e160cb8b 100644 --- a/packages/workflow-executor/test/executors/read-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/read-record-step-executor.test.ts @@ -895,7 +895,7 @@ describe('ReadRecordStepExecutor', () => { model: mockModel.model, runStore, stepDefinition: makeStep({ - preRecordedArgs: { fieldDisplayNames: ['Email'] }, + preRecordedArgs: { fieldNames: ['email'] }, }), }); const executor = new ReadRecordStepExecutor(context); @@ -904,6 +904,8 @@ describe('ReadRecordStepExecutor', () => { expect(result.stepOutcome.status).toBe('success'); expect(mockModel.bindTools).not.toHaveBeenCalled(); + // Pre-recorded reference is the technical name 'email'; the persisted displayName 'Email' + // is resolved from the schema, not received on the wire. expect(runStore.saveStepExecution).toHaveBeenCalledWith( 'run-1', expect.objectContaining({ @@ -956,7 +958,7 @@ describe('ReadRecordStepExecutor', () => { model: mockModel.model, runStore, stepDefinition: makeStep({ - preRecordedArgs: { selectedRecordStepIndex: 0, fieldDisplayNames: ['Email'] }, + preRecordedArgs: { selectedRecordStepIndex: 0, fieldNames: ['email'] }, }), }); const executor = new ReadRecordStepExecutor(context); @@ -985,7 +987,7 @@ describe('ReadRecordStepExecutor', () => { const context = makeContext({ model: mockModel.model, stepDefinition: makeStep({ - preRecordedArgs: { fieldDisplayNames: ['NonExistentField'] }, + preRecordedArgs: { fieldNames: ['non_existent_field'] }, }), }); const executor = new ReadRecordStepExecutor(context); @@ -995,6 +997,43 @@ describe('ReadRecordStepExecutor', () => { expect(result.stepOutcome.status).toBe('error'); }); + it('resolves a pre-recorded technical fieldName to its own field, not another whose displayName collides', async () => { + const runStore = makeMockRunStore(); + // Field B's displayName ('status') equals field A's technical fieldName ('status'). + const workflowPort = makeMockWorkflowPort({ + customers: makeCollectionSchema({ + fields: [ + { fieldName: 'status', displayName: 'Internal Note', isRelationship: false }, + { fieldName: 'note', displayName: 'status', isRelationship: false }, + ], + }), + }); + const agentPort = makeMockAgentPort({ + customers: { values: { status: 'active', note: 'B' } }, + }); + const context = makeContext({ + model: makeMockModel().model, + runStore, + workflowPort, + agentPort, + stepDefinition: makeStep({ preRecordedArgs: { fieldNames: ['status'] } }), + }); + const executor = new ReadRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('success'); + // Reads field A ('status'), not field B ('note' / displayName 'status'). + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionResult: { + fields: [{ value: 'active', name: 'status', displayName: 'Internal Note' }], + }, + }), + ); + }); + it('falls back to AI when no preRecordedArgs', async () => { const mockModel = makeMockModel({ fieldNames: ['email'] }); const context = makeContext({ model: mockModel.model }); 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 59648d2751..022b47c199 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 @@ -1055,7 +1055,7 @@ describe('TriggerRecordActionStepExecutor', () => { runStore, stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated, - preRecordedArgs: { actionDisplayName: 'Send Welcome Email' }, + preRecordedArgs: { actionName: 'send-welcome-email' }, }), }); const executor = new TriggerRecordActionStepExecutor(context); @@ -1068,6 +1068,14 @@ describe('TriggerRecordActionStepExecutor', () => { expect.objectContaining({ action: 'send-welcome-email' }), context.user, ); + // Pre-recorded reference is the technical name; the persisted displayName is resolved + // from the schema, not received on the wire. + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: { displayName: 'Send Welcome Email', name: 'send-welcome-email' }, + }), + ); }); it('still goes through awaiting-input when executionType is not FullyAutomated', async () => { @@ -1077,7 +1085,7 @@ describe('TriggerRecordActionStepExecutor', () => { model: mockModel.model, runStore, stepDefinition: makeStep({ - preRecordedArgs: { actionDisplayName: 'Send Welcome Email' }, + preRecordedArgs: { actionName: 'send-welcome-email' }, }), }); const executor = new TriggerRecordActionStepExecutor(context); @@ -1086,6 +1094,68 @@ describe('TriggerRecordActionStepExecutor', () => { expect(result.stepOutcome.status).toBe('awaiting-input'); expect(mockModel.bindTools).not.toHaveBeenCalled(); + // displayName is resolved from the schema even though only the technical name was sent. + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + pendingData: { displayName: 'Send Welcome Email', name: 'send-welcome-email' }, + }), + ); + }); + + it('returns error when a pre-recorded actionName does not resolve', async () => { + const context = makeContext({ + stepDefinition: makeStep({ + executionType: StepExecutionMode.FullyAutomated, + preRecordedArgs: { actionName: 'does-not-exist' }, + }), + }); + const executor = new TriggerRecordActionStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + }); + + it('resolves a pre-recorded technical actionName to its own action, not another whose displayName collides', async () => { + const runStore = makeMockRunStore(); + // Action B's displayName ('archive') equals action A's technical name ('archive'). + const workflowPort = makeMockWorkflowPort({ + customers: makeCollectionSchema({ + actions: [ + { + name: 'archive', + displayName: 'Send Welcome Email', + endpoint: '/forest/actions/archive', + }, + { name: 'send', displayName: 'archive', endpoint: '/forest/actions/send' }, + ], + }), + }); + const context = makeContext({ + runStore, + workflowPort, + stepDefinition: makeStep({ + executionType: StepExecutionMode.FullyAutomated, + preRecordedArgs: { actionName: 'archive' }, + }), + }); + const executor = new TriggerRecordActionStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('success'); + // Triggers action A ('archive'), not action B ('send' / displayName 'archive'). + expect(context.agentPort.executeAction).toHaveBeenCalledWith( + expect.objectContaining({ action: 'archive' }), + context.user, + ); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: { displayName: 'Send Welcome Email', name: 'archive' }, + }), + ); }); it('falls back to AI when no preRecordedArgs', async () => { diff --git a/packages/workflow-executor/test/executors/update-record-step-executor.test.ts b/packages/workflow-executor/test/executors/update-record-step-executor.test.ts index 48becbae9f..c822208968 100644 --- a/packages/workflow-executor/test/executors/update-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/update-record-step-executor.test.ts @@ -1160,7 +1160,7 @@ describe('UpdateRecordStepExecutor', () => { runStore, stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated, - preRecordedArgs: { fieldDisplayName: 'Status', value: 'active' }, + preRecordedArgs: { fieldName: 'status', value: 'active' }, }), }); const executor = new UpdateRecordStepExecutor(context); @@ -1173,6 +1173,14 @@ describe('UpdateRecordStepExecutor', () => { expect.objectContaining({ values: { status: 'active' } }), context.user, ); + // Pre-recorded reference is the technical name 'status'; the persisted displayName 'Status' + // is resolved from the schema, not received on the wire. + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: { displayName: 'Status', name: 'status', value: 'active' }, + }), + ); }); it('still goes through awaiting-input when executionType is not FullyAutomated', async () => { @@ -1182,7 +1190,7 @@ describe('UpdateRecordStepExecutor', () => { model: mockModel.model, runStore, stepDefinition: makeStep({ - preRecordedArgs: { fieldDisplayName: 'Status', value: 'active' }, + preRecordedArgs: { fieldName: 'status', value: 'active' }, }), }); const executor = new UpdateRecordStepExecutor(context); @@ -1191,9 +1199,30 @@ describe('UpdateRecordStepExecutor', () => { expect(result.stepOutcome.status).toBe('awaiting-input'); expect(mockModel.bindTools).not.toHaveBeenCalled(); + // displayName 'Status' is resolved from the schema even though only the technical name was sent. + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + pendingData: { displayName: 'Status', name: 'status', value: 'active' }, + }), + ); + }); + + it('returns error when a pre-recorded fieldName does not resolve', async () => { + const context = makeContext({ + stepDefinition: makeStep({ + executionType: StepExecutionMode.FullyAutomated, + preRecordedArgs: { fieldName: 'does_not_exist', value: 'x' }, + }), + }); + const executor = new UpdateRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); }); - it('falls back to AI when preRecordedArgs has no fieldDisplayName', async () => { + it('falls back to AI when preRecordedArgs has no fieldName', async () => { const mockModel = makeMockModel({ input: { fieldName: 'Status', value: 'active', reasoning: 'r' }, }); @@ -1211,11 +1240,11 @@ describe('UpdateRecordStepExecutor', () => { expect(mockModel.bindTools).toHaveBeenCalledTimes(1); }); - it('returns error when fieldDisplayName is provided without value', async () => { + it('returns error when fieldName is provided without value', async () => { const context = makeContext({ stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated, - preRecordedArgs: { fieldDisplayName: 'Status' }, + preRecordedArgs: { fieldName: 'status' }, }), }); const executor = new UpdateRecordStepExecutor(context); @@ -1225,7 +1254,7 @@ describe('UpdateRecordStepExecutor', () => { expect(result.stepOutcome.status).toBe('error'); }); - it('returns error when value is provided without fieldDisplayName', async () => { + it('returns error when value is provided without fieldName', async () => { const context = makeContext({ stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated, @@ -1251,7 +1280,7 @@ describe('UpdateRecordStepExecutor', () => { workflowPort, stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated, - preRecordedArgs: { fieldDisplayName: 'Age', value: 42 }, + preRecordedArgs: { fieldName: 'age', value: 42 }, }), }); const executor = new UpdateRecordStepExecutor(context); @@ -1264,6 +1293,49 @@ describe('UpdateRecordStepExecutor', () => { context.user, ); }); + + it('resolves a pre-recorded technical name to its own field, not another field whose displayName collides', async () => { + const runStore = makeMockRunStore(); + // Field B's displayName ('status') equals field A's technical fieldName ('status'). + // A pre-recorded technical name must resolve field A, never field B. + const workflowPort = makeMockWorkflowPort({ + customers: makeCollectionSchema({ + fields: [ + { + fieldName: 'status', + displayName: 'Internal Note', + isRelationship: false, + type: 'String', + }, + { fieldName: 'note', displayName: 'status', isRelationship: false, type: 'String' }, + ], + }), + }); + const context = makeContext({ + runStore, + workflowPort, + stepDefinition: makeStep({ + executionType: StepExecutionMode.FullyAutomated, + preRecordedArgs: { fieldName: 'status', value: 'active' }, + }), + }); + const executor = new UpdateRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('success'); + // Writes field A ('status'), not field B ('note' / displayName 'status'). + expect(context.agentPort.updateRecord).toHaveBeenCalledWith( + expect.objectContaining({ values: { status: 'active' } }), + context.user, + ); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionParams: { displayName: 'Internal Note', name: 'status', value: 'active' }, + }), + ); + }); }); describe('buildUpdateFieldTool — type-specific schemas', () => {