From d58877aa71f7171c947adbb9df74a4f84137d365 Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Tue, 19 May 2026 21:22:30 +0200 Subject: [PATCH 1/5] fix(workflow-executor): preserve AI suggestion in pending data --- .../src/executors/base-step-executor.ts | 50 ++++--- .../load-related-record-step-executor.ts | 20 ++- .../trigger-record-action-step-executor.ts | 6 +- .../executors/update-record-step-executor.ts | 9 +- .../src/types/step-execution-data.ts | 22 ++- .../load-related-record-step-executor.test.ts | 95 +++++++++++++ ...rigger-record-action-step-executor.test.ts | 10 +- .../update-record-step-executor.test.ts | 125 ++++++++++++++++++ 8 files changed, 299 insertions(+), 38 deletions(-) diff --git a/packages/workflow-executor/src/executors/base-step-executor.ts b/packages/workflow-executor/src/executors/base-step-executor.ts index 8f6de37859..ed6f5c9c73 100644 --- a/packages/workflow-executor/src/executors/base-step-executor.ts +++ b/packages/workflow-executor/src/executors/base-step-executor.ts @@ -5,7 +5,7 @@ import type { IStepExecutor, StepExecutionResult, } from '../types/execution-context'; -import type { StepExecutionData } from '../types/step-execution-data'; +import type { StepExecutionData, WithUserConfirmation } from '../types/step-execution-data'; import type { StepDefinition } from '../types/validated/step-definition'; import type { StepStatus } from '../types/validated/step-outcome'; import type { @@ -27,7 +27,9 @@ import { import patchBodySchemas from '../http/pending-data-validators'; import StepSummaryBuilder from './summary/step-summary-builder'; -type WithPendingData = StepExecutionData & { pendingData?: object }; +type WithPendingData = StepExecutionData & { pendingData?: object } & WithUserConfirmation; + +type PatchBody = Record & { userConfirmed?: boolean }; export default abstract class BaseStepExecutor implements IStepExecutor @@ -211,36 +213,44 @@ export default abstract class BaseStepExecutor( pendingData?: unknown, ): Promise { const { type } = this.context.stepDefinition; const execution = await this.findPendingExecution(type); - if (pendingData !== undefined && execution) { - const schema = patchBodySchemas[execution.type]!; - const parsed = schema.safeParse(pendingData); + if (!execution) return undefined; - if (!parsed.success) { - throw new StepStateError( - `Invalid pending data: ${parsed.error.issues.map(i => i.message).join(', ')}`, - ); - } + if (pendingData === undefined) return execution; - const updated = { - ...execution, - pendingData: { ...(execution.pendingData as object), ...(parsed.data as object) }, - } as TExec; + const schema = patchBodySchemas[execution.type]!; + const parsed = schema.safeParse(pendingData); - await this.context.runStore.saveStepExecution( - this.context.runId, - updated as StepExecutionData, + if (!parsed.success) { + throw new StepStateError( + `Invalid pending data: ${parsed.error.issues.map(i => i.message).join(', ')}`, ); - - return updated; } - return execution; + const patchBody = parsed.data as PatchBody; + + // Last-write-wins: spread-merging would leak stale keys from prior PATCHes. + const updated: TExec = { + ...execution, + pendingData: { + ...execution.pendingData, + ...(patchBody.userConfirmed !== undefined + ? { userConfirmed: patchBody.userConfirmed } + : {}), + }, + userConfirmation: patchBody, + }; + + await this.context.runStore.saveStepExecution(this.context.runId, updated); + + return updated; } // userConfirmed branches: undefined → re-emit awaiting-input (PATCH not yet called); 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 23d42be8c5..6ac3ab1059 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 @@ -138,21 +138,29 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { - const { selectedRecordRef, pendingData } = execution; + const { selectedRecordRef, pendingData, userConfirmation } = execution; if (!pendingData) { throw new StepStateError(`Step at index ${this.context.stepIndex} has no pending data`); } - const { name, displayName, selectedRecordId } = pendingData; + const isString = (v: unknown): v is string => typeof v === 'string'; + const isRecordId = (v: unknown): v is Array => + Array.isArray(v) && v.every(e => typeof e === 'string' || typeof e === 'number'); - // Re-derive relatedCollectionName from schema using the (possibly updated) relation name. - // `name` is always a fieldName (set from field.fieldName in buildTarget) — search directly. + const name = isString(userConfirmation?.name) ? userConfirmation.name : pendingData.name; + const displayName = isString(userConfirmation?.displayName) + ? userConfirmation.displayName + : pendingData.displayName; + const selectedRecordId = isRecordId(userConfirmation?.selectedRecordId) + ? userConfirmation.selectedRecordId + : pendingData.selectedRecordId; + + // Re-derive relatedCollectionName because the user may have swapped the relation. const schema = await this.getCollectionSchema(selectedRecordRef.collectionName); const field = schema.fields.find(f => f.fieldName === name); const relatedCollectionName = field?.relatedCollectionName; 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 4b75633601..bf5358513c 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 @@ -68,11 +68,11 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< return this.handleConfirmationFlow( pending, async exec => { - const { selectedRecordRef, pendingData } = exec; + const { selectedRecordRef, pendingData, userConfirmation } = exec; // The frontend executes the action itself and posts the result back. // A confirmed step without actionResult is a broken frontend contract. - if (!pendingData || !('actionResult' in pendingData)) { + if (!pendingData || !userConfirmation || !('actionResult' in userConfirmation)) { throw new StepStateError( `Frontend confirmed action but did not provide actionResult ` + `(run "${this.context.runId}", step ${this.context.stepIndex})`, @@ -85,7 +85,7 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< name: pendingData.name, }; - return this.saveFrontendResult(target, pendingData.actionResult, exec); + return this.saveFrontendResult(target, userConfirmation.actionResult, exec); }, ); } 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 f27b17783b..51e797540f 100644 --- a/packages/workflow-executor/src/executors/update-record-step-executor.ts +++ b/packages/workflow-executor/src/executors/update-record-step-executor.ts @@ -130,10 +130,15 @@ export default class UpdateRecordStepExecutor extends RecordStepExecutor(pending, async exec => { - const { selectedRecordRef, pendingData } = exec; + const { selectedRecordRef, pendingData, userConfirmation } = exec; + const userValue = + userConfirmation && 'value' in userConfirmation + ? userConfirmation.value + : (pendingData as { value: unknown })?.value; const target: UpdateTarget = { selectedRecordRef, - ...(pendingData as FieldRef & { value: unknown }), + ...(pendingData as FieldRef), + value: userValue, }; return this.resolveAndUpdate(target, exec); diff --git a/packages/workflow-executor/src/types/step-execution-data.ts b/packages/workflow-executor/src/types/step-execution-data.ts index da1bce172c..359097099c 100644 --- a/packages/workflow-executor/src/types/step-execution-data.ts +++ b/packages/workflow-executor/src/types/step-execution-data.ts @@ -14,6 +14,12 @@ interface MutatingStepExecutionData extends BaseStepExecutionData { idempotencyPhase?: 'executing' | 'done'; } +// Parsed PATCH body kept beside `pendingData` so executors can read the user's +// final input without overwriting the AI suggestion. +export interface WithUserConfirmation { + userConfirmation?: Record; +} + // -- Condition -- export interface ConditionStepExecutionData extends BaseStepExecutionData { @@ -50,7 +56,9 @@ export interface ReadRecordStepExecutionData extends BaseStepExecutionData { // -- Update Record -- -export interface UpdateRecordStepExecutionData extends MutatingStepExecutionData { +export interface UpdateRecordStepExecutionData + extends MutatingStepExecutionData, + WithUserConfirmation { type: 'update-record'; executionParams?: FieldRef & { value: unknown }; // User confirmed → values returned by updateRecord. User rejected → skipped. @@ -73,13 +81,13 @@ export interface RelationRef { displayName: string; } -export interface TriggerRecordActionStepExecutionData extends MutatingStepExecutionData { +export interface TriggerRecordActionStepExecutionData + extends MutatingStepExecutionData, + WithUserConfirmation { type: 'trigger-action'; executionParams?: ActionRef; executionResult?: { success: true; actionResult: unknown } | { skipped: true }; - // When userConfirmed=true, actionResult is required: the frontend executes the action and - // posts the result back (the executor never re-executes on confirmation). - pendingData?: ActionRef & { userConfirmed?: boolean; actionResult?: unknown }; + pendingData?: ActionRef & { userConfirmed?: boolean }; selectedRecordRef: RecordRef; } @@ -123,7 +131,9 @@ export interface LoadRelatedRecordPendingData extends RelationRef { userConfirmed?: boolean; } -export interface LoadRelatedRecordStepExecutionData extends BaseStepExecutionData { +export interface LoadRelatedRecordStepExecutionData + extends BaseStepExecutionData, + WithUserConfirmation { type: 'load-related-record'; pendingData?: LoadRelatedRecordPendingData; selectedRecordRef: RecordRef; diff --git a/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts b/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts index e7fdab82ef..250ab6cb1e 100644 --- a/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts @@ -827,6 +827,101 @@ describe('LoadRelatedRecordStepExecutor', () => { }); }); + describe('confirmation with user override of selectedRecordId (Branch A)', () => { + it('preserves AI suggestion in pendingData and writes user choice to executionParams', async () => { + // Persisted state: AI suggested record [99], awaiting confirmation. + const execution = makePendingExecution({ + pendingData: { + displayName: 'Order', + name: 'order', + selectedRecordId: [99], + suggestedFields: ['status', 'amount'], + }, + }); + const agentPort = makeMockAgentPort(); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([execution]), + }); + // User confirms with a different record id: [42]. + const context = makeContext({ + agentPort, + runStore, + incomingPendingData: { userConfirmed: true, selectedRecordId: [42] }, + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('success'); + + // Final persisted execution must keep AI suggestion in pendingData + // and use the user-overridden record id in executionResult. + const finalSave = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(finalSave).toEqual( + expect.objectContaining({ + type: 'load-related-record', + pendingData: expect.objectContaining({ + displayName: 'Order', + name: 'order', + selectedRecordId: [99], // AI suggestion preserved + userConfirmed: true, + }), + executionResult: expect.objectContaining({ + record: expect.objectContaining({ collectionName: 'orders', recordId: [42] }), + }), + }), + ); + }); + }); + + describe('confirmation with user override of relation name (Branch A)', () => { + it('re-derives relatedCollectionName when the user switches to a different relation', async () => { + // AI suggested "order" (→ orders collection). User switches to "address" (→ addresses). + const execution = makePendingExecution({ + pendingData: { + displayName: 'Order', + name: 'order', + selectedRecordId: [99], + suggestedFields: [], + }, + }); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([execution]), + }); + const context = makeContext({ + runStore, + incomingPendingData: { + userConfirmed: true, + name: 'address', + displayName: 'Address', + selectedRecordId: [7], + }, + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('success'); + const finalSave = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(finalSave).toEqual( + expect.objectContaining({ + // AI suggestion preserved on pendingData + pendingData: expect.objectContaining({ + name: 'order', + displayName: 'Order', + selectedRecordId: [99], + }), + // User-overridden relation resolves to the addresses collection + executionParams: { name: 'address', displayName: 'Address' }, + executionResult: expect.objectContaining({ + relation: { name: 'address', displayName: 'Address' }, + record: expect.objectContaining({ collectionName: 'addresses', recordId: [7] }), + }), + }), + ); + }); + }); + describe('resolveFromSelection — relatedCollectionName resolution (Branch A)', () => { it('derives relatedCollectionName from schema when confirmed', async () => { const schema = makeCollectionSchema({ 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 f2c62ee988..ba74af2eb3 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 @@ -249,6 +249,9 @@ describe('TriggerRecordActionStepExecutor', () => { displayName: 'Send Welcome Email', name: 'send-welcome-email', userConfirmed: true, + }, + userConfirmation: { + userConfirmed: true, actionResult: { success: 'ok', html: '

Email queued

' }, }, selectedRecordRef: makeRecordRef(), @@ -280,7 +283,6 @@ describe('TriggerRecordActionStepExecutor', () => { displayName: 'Send Welcome Email', name: 'send-welcome-email', userConfirmed: true, - actionResult: { success: 'ok', html: '

Email queued

' }, }, }), ); @@ -295,6 +297,9 @@ describe('TriggerRecordActionStepExecutor', () => { displayName: 'Send Welcome Email', name: 'send-welcome-email', userConfirmed: true, + }, + userConfirmation: { + userConfirmed: true, actionResult: null, }, selectedRecordRef: makeRecordRef(), @@ -946,6 +951,9 @@ describe('TriggerRecordActionStepExecutor', () => { displayName: 'Send Welcome Email', name: 'send-welcome-email', userConfirmed: true, + }, + userConfirmation: { + userConfirmed: true, actionResult: { success: 'ok' }, }, selectedRecordRef: makeRecordRef(), 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 9c1866e092..0918bb205d 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 @@ -255,6 +255,131 @@ describe('UpdateRecordStepExecutor', () => { }); }); + describe('confirmation with user override (Branch A)', () => { + it('preserves AI suggestion in pendingData and writes user value to executionParams', async () => { + // Persisted state: AI proposed 'inactive', awaiting confirmation. + const execution: UpdateRecordStepExecutionData = { + type: 'update-record', + stepIndex: 0, + pendingData: { + displayName: 'Status', + name: 'status', + value: 'inactive', + }, + selectedRecordRef: makeRecordRef(), + }; + const updatedValues = { status: 'active' }; + const agentPort = makeMockAgentPort(updatedValues); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([execution]), + }); + // User confirms with a different value: 'active'. + const context = makeContext({ + agentPort, + runStore, + incomingPendingData: { userConfirmed: true, value: 'active' }, + }); + const executor = new UpdateRecordStepExecutor(context); + + await executor.execute(); + + // updateRecord must be called with the user-confirmed value. + expect(agentPort.updateRecord).toHaveBeenCalledWith( + { collection: 'customers', id: [42], values: { status: 'active' } }, + expect.objectContaining({ id: 1 }), + ); + + // Final persisted execution must keep AI suggestion in pendingData + // and the user value in executionParams. + const finalSave = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(finalSave).toEqual( + expect.objectContaining({ + type: 'update-record', + pendingData: expect.objectContaining({ + displayName: 'Status', + name: 'status', + value: 'inactive', // AI suggestion preserved + userConfirmed: true, + }), + executionParams: { displayName: 'Status', name: 'status', value: 'active' }, + executionResult: { updatedValues }, + }), + ); + }); + }); + + describe('accept-via-PATCH without value override (Branch A)', () => { + it('falls back to pendingData.value when userConfirmation has no value key', async () => { + const execution: UpdateRecordStepExecutionData = { + type: 'update-record', + stepIndex: 0, + pendingData: { displayName: 'Status', name: 'status', value: 'active' }, + selectedRecordRef: makeRecordRef(), + }; + const updatedValues = { status: 'active' }; + const agentPort = makeMockAgentPort(updatedValues); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([execution]), + }); + const context = makeContext({ + agentPort, + runStore, + incomingPendingData: { userConfirmed: true }, + }); + const executor = new UpdateRecordStepExecutor(context); + + await executor.execute(); + + expect(agentPort.updateRecord).toHaveBeenCalledWith( + { collection: 'customers', id: [42], values: { status: 'active' } }, + expect.objectContaining({ id: 1 }), + ); + const finalSave = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(finalSave).toEqual( + expect.objectContaining({ + executionParams: { displayName: 'Status', name: 'status', value: 'active' }, + userConfirmation: { userConfirmed: true }, + }), + ); + }); + }); + + describe('rejection via PATCH with userConfirmation set (Branch A)', () => { + it('skips the update and ignores any value in userConfirmation', async () => { + const execution: UpdateRecordStepExecutionData = { + type: 'update-record', + stepIndex: 0, + pendingData: { displayName: 'Status', name: 'status', value: 'inactive' }, + selectedRecordRef: makeRecordRef(), + }; + const agentPort = makeMockAgentPort(); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([execution]), + }); + const context = makeContext({ + agentPort, + runStore, + incomingPendingData: { userConfirmed: false, value: 'active' }, + }); + const executor = new UpdateRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('success'); + expect(agentPort.updateRecord).not.toHaveBeenCalled(); + const finalSave = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(finalSave).toEqual( + expect.objectContaining({ + executionResult: { skipped: true }, + pendingData: expect.objectContaining({ + value: 'inactive', + userConfirmed: false, + }), + }), + ); + }); + }); + describe('confirmation rejected (Branch A)', () => { it('skips the update when user rejects', async () => { const agentPort = makeMockAgentPort(); From 270ecf0585fac57aa9629bba275cfe24b0199431 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 19 May 2026 23:20:25 +0200 Subject: [PATCH 2/5] refactor(workflow-executor): type userConfirmation precisely per step and re-derive displayName from schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Export named Zod schemas and inferred types from pending-data-validators (UpdateRecordConfirmation, TriggerActionConfirmation, McpConfirmation, LoadRelatedRecordConfirmation) - Make WithUserConfirmation generic so each interface carries the exact confirmation shape instead of Record - Add WithUserConfirmation to McpStepExecutionData (was missing despite mcp executor writing userConfirmation) - Remove isString/isRecordId runtime guards in resolveFromSelection — now unnecessary with precise typing - Re-derive displayName from FieldSchema instead of accepting it from the PATCH body; remove displayName from loadRelatedRecordPatchSchema and contract - Fix inaccurate comment "Keeps pendingData immutable" in patchAndReloadPendingData Co-Authored-By: Claude Sonnet 4.6 --- WORKFLOW-EXECUTOR-CONTRACT.md | 2 +- .../src/executors/base-step-executor.ts | 6 +- .../load-related-record-step-executor.ts | 22 ++-- .../src/http/pending-data-validators.ts | 104 ++++++++++-------- .../src/types/step-execution-data.ts | 18 ++- .../load-related-record-step-executor.test.ts | 1 - 6 files changed, 82 insertions(+), 71 deletions(-) diff --git a/WORKFLOW-EXECUTOR-CONTRACT.md b/WORKFLOW-EXECUTOR-CONTRACT.md index 0a7bcf98ef..672ad52999 100644 --- a/WORKFLOW-EXECUTOR-CONTRACT.md +++ b/WORKFLOW-EXECUTOR-CONTRACT.md @@ -306,8 +306,8 @@ interface LoadRelatedRecordPendingData { { userConfirmed: boolean; name?: string; // override relation - displayName?: string; selectedRecordId?: Array; // min 1 element + // displayName is NOT accepted — derived from FieldSchema after resolving name. } ``` diff --git a/packages/workflow-executor/src/executors/base-step-executor.ts b/packages/workflow-executor/src/executors/base-step-executor.ts index ed6f5c9c73..06a48162bd 100644 --- a/packages/workflow-executor/src/executors/base-step-executor.ts +++ b/packages/workflow-executor/src/executors/base-step-executor.ts @@ -213,8 +213,8 @@ export default abstract class BaseStepExecutor( pendingData?: unknown, ): Promise { @@ -245,7 +245,7 @@ export default abstract class BaseStepExecutor typeof v === 'string'; - const isRecordId = (v: unknown): v is Array => - Array.isArray(v) && v.every(e => typeof e === 'string' || typeof e === 'number'); - - const name = isString(userConfirmation?.name) ? userConfirmation.name : pendingData.name; - const displayName = isString(userConfirmation?.displayName) - ? userConfirmation.displayName - : pendingData.displayName; - const selectedRecordId = isRecordId(userConfirmation?.selectedRecordId) - ? userConfirmation.selectedRecordId - : pendingData.selectedRecordId; - - // Re-derive relatedCollectionName because the user may have swapped the relation. + const name = userConfirmation?.name ?? pendingData.name; + const selectedRecordId = userConfirmation?.selectedRecordId ?? pendingData.selectedRecordId; + + // Re-derive relatedCollectionName and displayName because the user may have swapped the relation. const schema = await this.getCollectionSchema(selectedRecordRef.collectionName); const field = schema.fields.find(f => f.fieldName === name); - const relatedCollectionName = field?.relatedCollectionName; - if (!relatedCollectionName) { + if (!field?.relatedCollectionName) { throw new StepStateError( `Step at index ${this.context.stepIndex} could not resolve relatedCollectionName for relation "${name}"`, ); } + const { displayName, relatedCollectionName } = field; + const record: RecordRef = { collectionName: relatedCollectionName, recordId: selectedRecordId, diff --git a/packages/workflow-executor/src/http/pending-data-validators.ts b/packages/workflow-executor/src/http/pending-data-validators.ts index 95f3376c1d..7332b41f8f 100644 --- a/packages/workflow-executor/src/http/pending-data-validators.ts +++ b/packages/workflow-executor/src/http/pending-data-validators.ts @@ -1,53 +1,67 @@ -import type { StepExecutionData } from '../types/step-execution-data'; - import { z } from 'zod'; // Per-step-type schemas for the `pendingData` payload sent by the front via // POST /runs/:runId/trigger. Consumed by step executors to validate `incomingPendingData` // before applying user confirmation or override. Schemas use .strict() to reject unknown fields. -const patchBodySchemas: Partial> = { - 'update-record': z - .object({ - userConfirmed: z.boolean(), - value: z.string().optional(), // user may override the AI-proposed value - }) - .strict(), - - 'trigger-action': z - .object({ - userConfirmed: z.boolean(), - // Opaque action result from the frontend. Required when userConfirmed=true; the - // presence check lives in the step-executor so a descriptive StepStateError can - // name the runId/stepIndex — not achievable from inside a zod schema. - actionResult: z.unknown().optional(), - }) - .strict(), - - mcp: z.object({ userConfirmed: z.boolean() }).strict(), - - 'load-related-record': z - .object({ - userConfirmed: z.boolean(), - // User may intentionally switch to a different relation than the one the AI selected. - // The executor re-derives relatedCollectionName from FieldSchema when processing the confirmation. - name: z.string().optional(), - displayName: z.string().optional(), - // User may override the AI-selected record; must be non-empty when provided. - selectedRecordId: z - .array(z.union([z.string(), z.number()])) - .min(1) - .optional(), - }) - .strict(), - // relatedCollectionName and suggestedFields are NOT accepted — internal executor data. - - guidance: z - .object({ - userInput: z.string().optional(), - }) - .strict(), - - condition: z.object({ selectedOption: z.string() }).strict(), + +const updateRecordPatchSchema = z + .object({ + userConfirmed: z.boolean(), + value: z.string().optional(), // user may override the AI-proposed value + }) + .strict(); + +const triggerActionPatchSchema = z + .object({ + userConfirmed: z.boolean(), + // Opaque action result from the frontend. Required when userConfirmed=true; the + // presence check lives in the step-executor so a descriptive StepStateError can + // name the runId/stepIndex — not achievable from inside a zod schema. + actionResult: z.unknown().optional(), + }) + .strict(); + +const mcpPatchSchema = z.object({ userConfirmed: z.boolean() }).strict(); + +const loadRelatedRecordPatchSchema = z + .object({ + userConfirmed: z.boolean(), + // User may intentionally switch to a different relation than the one the AI selected. + // The executor re-derives relatedCollectionName and displayName from FieldSchema when + // processing the confirmation. + name: z.string().optional(), + // User may override the AI-selected record; must be non-empty when provided. + selectedRecordId: z + .array(z.union([z.string(), z.number()])) + .min(1) + .optional(), + }) + .strict(); +// relatedCollectionName, displayName and suggestedFields are NOT accepted — internal executor data. + +const guidancePatchSchema = z + .object({ + userInput: z.string().optional(), + }) + .strict(); + +const conditionPatchSchema = z.object({ selectedOption: z.string() }).strict(); + +// Inferred types — consumed by step-execution-data.ts to type `userConfirmation` precisely, +// removing the need for runtime type guards in executors. +export type UpdateRecordConfirmation = z.infer; +export type TriggerActionConfirmation = z.infer; +export type McpConfirmation = z.infer; +export type LoadRelatedRecordConfirmation = z.infer; +export type GuidanceConfirmation = z.infer; + +const patchBodySchemas: Partial> = { + 'update-record': updateRecordPatchSchema, + 'trigger-action': triggerActionPatchSchema, + mcp: mcpPatchSchema, + 'load-related-record': loadRelatedRecordPatchSchema, + guidance: guidancePatchSchema, + condition: conditionPatchSchema, }; export default patchBodySchemas; diff --git a/packages/workflow-executor/src/types/step-execution-data.ts b/packages/workflow-executor/src/types/step-execution-data.ts index 359097099c..49edc7d99d 100644 --- a/packages/workflow-executor/src/types/step-execution-data.ts +++ b/packages/workflow-executor/src/types/step-execution-data.ts @@ -1,6 +1,12 @@ /** @draft Types derived from the workflow-executor spec -- subject to change. */ import type { RecordRef } from './validated/collection'; +import type { + LoadRelatedRecordConfirmation, + McpConfirmation, + TriggerActionConfirmation, + UpdateRecordConfirmation, +} from '../http/pending-data-validators'; // -- Base -- @@ -16,8 +22,8 @@ interface MutatingStepExecutionData extends BaseStepExecutionData { // Parsed PATCH body kept beside `pendingData` so executors can read the user's // final input without overwriting the AI suggestion. -export interface WithUserConfirmation { - userConfirmation?: Record; +export interface WithUserConfirmation = Record> { + userConfirmation?: T; } // -- Condition -- @@ -58,7 +64,7 @@ export interface ReadRecordStepExecutionData extends BaseStepExecutionData { export interface UpdateRecordStepExecutionData extends MutatingStepExecutionData, - WithUserConfirmation { + WithUserConfirmation { type: 'update-record'; executionParams?: FieldRef & { value: unknown }; // User confirmed → values returned by updateRecord. User rejected → skipped. @@ -83,7 +89,7 @@ export interface RelationRef { export interface TriggerRecordActionStepExecutionData extends MutatingStepExecutionData, - WithUserConfirmation { + WithUserConfirmation { type: 'trigger-action'; executionParams?: ActionRef; executionResult?: { success: true; actionResult: unknown } | { skipped: true }; @@ -103,7 +109,7 @@ export interface McpToolCall extends McpToolRef { input: Record; } -export interface McpStepExecutionData extends MutatingStepExecutionData { +export interface McpStepExecutionData extends MutatingStepExecutionData, WithUserConfirmation { type: 'mcp'; executionParams?: McpToolCall; executionResult?: @@ -133,7 +139,7 @@ export interface LoadRelatedRecordPendingData extends RelationRef { export interface LoadRelatedRecordStepExecutionData extends BaseStepExecutionData, - WithUserConfirmation { + WithUserConfirmation { type: 'load-related-record'; pendingData?: LoadRelatedRecordPendingData; selectedRecordRef: RecordRef; diff --git a/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts b/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts index 250ab6cb1e..663924407e 100644 --- a/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts @@ -893,7 +893,6 @@ describe('LoadRelatedRecordStepExecutor', () => { incomingPendingData: { userConfirmed: true, name: 'address', - displayName: 'Address', selectedRecordId: [7], }, }); From df8e7a9e280ce67def2133f7a26425fe402f0ecd Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 19 May 2026 23:31:03 +0200 Subject: [PATCH 3/5] fix(workflow-executor): reject name override without selectedRecordId in load-related-record patch Sending a different relation name without a new record ID would silently reuse the AI-suggested record ID from the original collection, producing a wrong or non-existent record in the new relation. Zod refine now enforces that selectedRecordId is required when name is overridden. Co-Authored-By: Claude Sonnet 4.6 --- .../src/http/pending-data-validators.ts | 7 +++- .../test/http/pending-data-validators.test.ts | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/packages/workflow-executor/src/http/pending-data-validators.ts b/packages/workflow-executor/src/http/pending-data-validators.ts index 7332b41f8f..5bfd822472 100644 --- a/packages/workflow-executor/src/http/pending-data-validators.ts +++ b/packages/workflow-executor/src/http/pending-data-validators.ts @@ -31,12 +31,17 @@ const loadRelatedRecordPatchSchema = z // processing the confirmation. name: z.string().optional(), // User may override the AI-selected record; must be non-empty when provided. + // Required when overriding the relation name — the original record ID belongs to a + // different collection and cannot be reused for the new relation. selectedRecordId: z .array(z.union([z.string(), z.number()])) .min(1) .optional(), }) - .strict(); + .strict() + .refine(data => !data.name || data.selectedRecordId !== undefined, { + message: 'selectedRecordId is required when overriding the relation name', + }); // relatedCollectionName, displayName and suggestedFields are NOT accepted — internal executor data. const guidancePatchSchema = z diff --git a/packages/workflow-executor/test/http/pending-data-validators.test.ts b/packages/workflow-executor/test/http/pending-data-validators.test.ts index a1c3634e2f..fff25f2d12 100644 --- a/packages/workflow-executor/test/http/pending-data-validators.test.ts +++ b/packages/workflow-executor/test/http/pending-data-validators.test.ts @@ -64,4 +64,36 @@ describe('patchBodySchemas', () => { expect(() => schema.parse({ userConfirmed: 'yes' })).toThrow(); }); }); + + describe('load-related-record', () => { + const schema = patchBodySchemas['load-related-record']; + if (!schema) throw new Error('load-related-record schema not registered'); + + it('accepts confirmation with no overrides', () => { + expect(schema.parse({ userConfirmed: true })).toEqual({ userConfirmed: true }); + }); + + it('accepts confirmation with selectedRecordId override only', () => { + expect(schema.parse({ userConfirmed: true, selectedRecordId: [42] })).toEqual({ + userConfirmed: true, + selectedRecordId: [42], + }); + }); + + it('accepts confirmation with both name and selectedRecordId (relation override)', () => { + expect( + schema.parse({ userConfirmed: true, name: 'address', selectedRecordId: [7] }), + ).toEqual({ userConfirmed: true, name: 'address', selectedRecordId: [7] }); + }); + + it('rejects name override without selectedRecordId — original record ID belongs to a different collection', () => { + expect(() => schema.parse({ userConfirmed: true, name: 'address' })).toThrow( + 'selectedRecordId is required when overriding the relation name', + ); + }); + + it('rejects unknown fields (strict schema)', () => { + expect(() => schema.parse({ userConfirmed: true, extra: 'leak' })).toThrow(); + }); + }); }); From 86d20c8bccc3db66315d40985ab3e5f782d685d4 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 19 May 2026 23:35:55 +0200 Subject: [PATCH 4/5] style(workflow-executor): fix prettier formatting in step-execution-data and validators test Co-Authored-By: Claude Sonnet 4.6 --- packages/workflow-executor/src/types/step-execution-data.ts | 4 +++- .../test/http/pending-data-validators.test.ts | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/workflow-executor/src/types/step-execution-data.ts b/packages/workflow-executor/src/types/step-execution-data.ts index 49edc7d99d..71d66e3e33 100644 --- a/packages/workflow-executor/src/types/step-execution-data.ts +++ b/packages/workflow-executor/src/types/step-execution-data.ts @@ -109,7 +109,9 @@ export interface McpToolCall extends McpToolRef { input: Record; } -export interface McpStepExecutionData extends MutatingStepExecutionData, WithUserConfirmation { +export interface McpStepExecutionData + extends MutatingStepExecutionData, + WithUserConfirmation { type: 'mcp'; executionParams?: McpToolCall; executionResult?: diff --git a/packages/workflow-executor/test/http/pending-data-validators.test.ts b/packages/workflow-executor/test/http/pending-data-validators.test.ts index fff25f2d12..e9ed9cb0b4 100644 --- a/packages/workflow-executor/test/http/pending-data-validators.test.ts +++ b/packages/workflow-executor/test/http/pending-data-validators.test.ts @@ -81,9 +81,9 @@ describe('patchBodySchemas', () => { }); it('accepts confirmation with both name and selectedRecordId (relation override)', () => { - expect( - schema.parse({ userConfirmed: true, name: 'address', selectedRecordId: [7] }), - ).toEqual({ userConfirmed: true, name: 'address', selectedRecordId: [7] }); + expect(schema.parse({ userConfirmed: true, name: 'address', selectedRecordId: [7] })).toEqual( + { userConfirmed: true, name: 'address', selectedRecordId: [7] }, + ); }); it('rejects name override without selectedRecordId — original record ID belongs to a different collection', () => { From 96808c060b6555a92235b0f2a559f1cada634f1b Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 19 May 2026 23:46:47 +0200 Subject: [PATCH 5/5] fix(workflow-executor): reject empty string name and fix refine condition for load-related-record Co-Authored-By: Claude Sonnet 4.6 --- .../workflow-executor/src/http/pending-data-validators.ts | 4 ++-- .../test/http/pending-data-validators.test.ts | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/workflow-executor/src/http/pending-data-validators.ts b/packages/workflow-executor/src/http/pending-data-validators.ts index 5bfd822472..5cac31584b 100644 --- a/packages/workflow-executor/src/http/pending-data-validators.ts +++ b/packages/workflow-executor/src/http/pending-data-validators.ts @@ -29,7 +29,7 @@ const loadRelatedRecordPatchSchema = z // User may intentionally switch to a different relation than the one the AI selected. // The executor re-derives relatedCollectionName and displayName from FieldSchema when // processing the confirmation. - name: z.string().optional(), + name: z.string().min(1).optional(), // User may override the AI-selected record; must be non-empty when provided. // Required when overriding the relation name — the original record ID belongs to a // different collection and cannot be reused for the new relation. @@ -39,7 +39,7 @@ const loadRelatedRecordPatchSchema = z .optional(), }) .strict() - .refine(data => !data.name || data.selectedRecordId !== undefined, { + .refine(data => data.name === undefined || data.selectedRecordId !== undefined, { message: 'selectedRecordId is required when overriding the relation name', }); // relatedCollectionName, displayName and suggestedFields are NOT accepted — internal executor data. diff --git a/packages/workflow-executor/test/http/pending-data-validators.test.ts b/packages/workflow-executor/test/http/pending-data-validators.test.ts index e9ed9cb0b4..09504bb910 100644 --- a/packages/workflow-executor/test/http/pending-data-validators.test.ts +++ b/packages/workflow-executor/test/http/pending-data-validators.test.ts @@ -92,6 +92,10 @@ describe('patchBodySchemas', () => { ); }); + it('rejects empty string name — empty string is not a valid relation name', () => { + expect(() => schema.parse({ userConfirmed: true, name: '' })).toThrow(); + }); + it('rejects unknown fields (strict schema)', () => { expect(() => schema.parse({ userConfirmed: true, extra: 'leak' })).toThrow(); });