From 0350a9729d4e8f59ac37780b4e542859f664282b Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Sun, 21 Jun 2026 13:57:56 +0200 Subject: [PATCH] feat(workflow-executor): action-form structure + execute-with-values + typed errors (PRD-509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Foundation for AI-assisted/Full-AI action forms (PRD-57). Extends the agent port over agent-client (mirrors the MCP server's get-action-form/execute-action): - getActionForm(query): full field list (type, required, enum options, value) + canExecute + requiredFields + skippedFields (values dropped by change hooks) - executeAction(query, values?): set values (strict) then execute via the agent's normal server-side validation — no bypass - ActionRequiresApprovalError (403 CustomActionRequiresApprovalError + roleIdsAllowedToApprove) distinct from a plain permission 403; the port never self-signs an approval request - ActionFormValidationError for unknown-field setFields + 400/422 rejections - getActionFormInfo kept intact (no caller regression) fixes PRD-509 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/adapters/agent-client-agent-port.ts | 106 ++++++++++++- packages/workflow-executor/src/errors.ts | 29 ++++ .../workflow-executor/src/ports/agent-port.ts | 42 ++++- .../adapters/agent-client-agent-port.test.ts | 146 +++++++++++++++++- 4 files changed, 318 insertions(+), 5 deletions(-) diff --git a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts index 0e128a3c49..987f264393 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -1,7 +1,10 @@ import type { + ActionForm, + ActionFormField, AgentPort, ExecuteActionQuery, GetActionFormInfoQuery, + GetActionFormQuery, GetRecordQuery, GetRelatedDataQuery, GetSingleRelatedDataQuery, @@ -17,6 +20,8 @@ import { HttpRequester, createRemoteAgentClient } from '@forestadmin/agent-clien import jsonwebtoken from 'jsonwebtoken'; import { + ActionFormValidationError, + ActionRequiresApprovalError, AgentPortError, AgentProbeError, RecordNotFoundError, @@ -24,6 +29,56 @@ import { extractErrorMessage, } from '../errors'; +// The agent-client throws `new Error(JSON.stringify({ error: { status, text }, body }))` on a non-2xx +// (superagent's toError sets status + text = raw response body). Parse it back to (status, body). +function parseAgentHttpError(error: unknown): { status?: number; text?: string } { + if (!(error instanceof Error)) return {}; + + try { + const parsed = JSON.parse(error.message) as { error?: { status?: number; text?: string } }; + + return { status: parsed.error?.status, text: parsed.error?.text }; + } catch { + return {}; + } +} + +// The agent rejects an approval-gated action with HTTP 403 CustomActionRequiresApprovalError, +// carrying roleIdsAllowedToApprove. Pull the roles out of the response body when present. +function extractRoleIdsAllowedToApprove(text: string): number[] | undefined { + try { + const body = JSON.parse(text) as { + errors?: { data?: { roleIdsAllowedToApprove?: number[] } }[]; + data?: { roleIdsAllowedToApprove?: number[] }; + }; + + return ( + body.errors?.[0]?.data?.roleIdsAllowedToApprove ?? + body.data?.roleIdsAllowedToApprove ?? + undefined + ); + } catch { + return undefined; + } +} + +// Map an action `execute()` failure to a typed error so the step executor can route it (PRD-509): +// approval-403 and validation rejections become distinct fallback-worthy errors; everything else +// (plain permission 403, infra 5xx, network) stays raw → wrapped as AgentPortError = step error. +function mapActionExecutionError(action: string, cause: unknown): unknown { + const { status, text } = parseAgentHttpError(cause); + + if (status === 403 && text && text.includes('CustomActionRequiresApprovalError')) { + return new ActionRequiresApprovalError(action, extractRoleIdsAllowedToApprove(text)); + } + + if (status === 400 || status === 422) { + return new ActionFormValidationError(action, cause); + } + + return cause; +} + function toCamelCase(name: string): string { return name.replace(/_([a-zA-Z0-9])/g, (_, c: string) => c.toUpperCase()); } @@ -192,7 +247,7 @@ export default class AgentClientAgentPort implements AgentPort { } async executeAction( - { collection, action, id }: ExecuteActionQuery, + { collection, action, id, values }: ExecuteActionQuery, user: StepUser, ): Promise { return this.callAgent('executeAction', async () => { @@ -200,7 +255,21 @@ export default class AgentClientAgentPort implements AgentPort { const recordIds = id?.length ? [id] : []; const act = await client.collection(collection).action(action, { recordIds }); - return act.execute(); + if (values) { + // setFields is strict (mirrors MCP execute-action): an unknown field is a config/drift + // problem, surfaced as a validation error rather than a silent skip. + try { + await act.setFields(values); + } catch (cause) { + throw new ActionFormValidationError(action, cause); + } + } + + try { + return await act.execute(); + } catch (cause) { + throw mapActionExecutionError(action, cause); + } }); } @@ -216,6 +285,39 @@ export default class AgentClientAgentPort implements AgentPort { }); } + async getActionForm( + { collection, action, id, values }: GetActionFormQuery, + user: StepUser, + ): Promise { + return this.callAgent('getActionForm', async () => { + const client = this.createClient(user); + const act = await client.collection(collection).action(action, { recordIds: [id] }); + + // Soft-apply so dependent fields are revealed by change hooks; unknown fields (dropped by a + // prior hook) come back in skippedFields rather than throwing (mirrors MCP get-action-form). + const skippedFields = values ? await act.tryToSetFields(values) : []; + + const fields = act.getFields().map((field): ActionFormField => { + const base = { + name: field.getName(), + type: field.getType(), + value: field.getValue(), + isRequired: field.isRequired() ?? false, + }; + + return field.getType() === 'Enum' + ? { ...base, enumValues: act.getEnumField(field.getName()).getOptions() ?? undefined } + : base; + }); + + const requiredFields = fields + .filter(field => field.isRequired && (field.value === undefined || field.value === null)) + .map(field => field.name); + + return { fields, canExecute: requiredFields.length === 0, requiredFields, skippedFields }; + }); + } + private async callAgent(operation: string, fn: () => Promise): Promise { try { return await fn(); diff --git a/packages/workflow-executor/src/errors.ts b/packages/workflow-executor/src/errors.ts index c5bffda409..e690b7ad3a 100644 --- a/packages/workflow-executor/src/errors.ts +++ b/packages/workflow-executor/src/errors.ts @@ -126,6 +126,35 @@ export class UnsupportedActionFormError extends WorkflowExecutorError { } } +// The action submission was rejected by the agent's server-side validation (bad/missing values), +// NOT an infra failure (PRD-509). Full AI (PRD-512) treats this as a fallback-to-AI-assisted reason +// so a human can fix the values and resubmit. +export class ActionFormValidationError extends WorkflowExecutorError { + constructor(actionName: string, cause?: unknown) { + super( + `Action "${actionName}" rejected the submitted form values`, + 'The submitted form values were rejected. Please review the form and try again.', + ); + this.cause = cause; + } +} + +// The action requires an Approval: the agent rejects a programmatic submit upfront with HTTP 403 +// CustomActionRequiresApprovalError (PRD-509). Distinct from a plain permission 403 — Full AI +// (PRD-512) falls back to AI-assisted so the native front handles the approval flow. The executor +// MUST NOT self-sign an approval request. +export class ActionRequiresApprovalError extends WorkflowExecutorError { + readonly roleIdsAllowedToApprove?: number[]; + + constructor(actionName: string, roleIdsAllowedToApprove?: number[]) { + super( + `Action "${actionName}" requires an approval and cannot be submitted programmatically`, + 'This action requires an approval. Please review and submit it manually.', + ); + this.roleIdsAllowedToApprove = roleIdsAllowedToApprove; + } +} + export class RunStorePortError extends UnavailableError { constructor(operation: string, cause: unknown) { super( diff --git a/packages/workflow-executor/src/ports/agent-port.ts b/packages/workflow-executor/src/ports/agent-port.ts index ceb3a75022..72cc9494b2 100644 --- a/packages/workflow-executor/src/ports/agent-port.ts +++ b/packages/workflow-executor/src/ports/agent-port.ts @@ -38,7 +38,44 @@ export type GetSingleRelatedDataQuery = { fields?: string[]; }; -export type ExecuteActionQuery = { collection: string; action: string; id?: Id[] }; +export type ExecuteActionQuery = { + collection: string; + action: string; + id?: Id[]; + // Pre-filled form values (PRD-509). Set on the form before execution, going through the agent's + // normal server-side validation — no bypass. Omitted for formless actions. + values?: Record; +}; + +export type GetActionFormQuery = { + collection: string; + action: string; + id: Id[]; + // Optional values to apply before reading the form back (fires the change hooks so dependent + // fields appear/update). Soft-applied: unknown fields are reported in `skippedFields`. + values?: Record; +}; + +// One field of an action form, flattened for the executor/AI (PRD-509). Mirrors the MCP +// get-action-form tool's field shape. +export type ActionFormField = { + name: string; + type: string; + value?: unknown; + isRequired: boolean; + // Allowed values for an Enum field — the AI must pick one of these or leave the field empty. + enumValues?: string[]; +}; + +export type ActionForm = { + fields: ActionFormField[]; + // True when every required field has a value (the agent-client form state's completeness check). + canExecute: boolean; + // Required fields still missing a value (empty when canExecute is true). + requiredFields: string[]; + // Fields from `values` that no longer exist in the form (dropped by dynamic-form change hooks). + skippedFields: string[]; +}; export type GetActionFormInfoQuery = { collection: string; action: string; id: Id[] }; @@ -63,6 +100,9 @@ export interface AgentPort { // Old Ruby agents with hooks.load=false return 404; agent-client falls back to the fields // passed via ActionEndpointsByCollection (populated from the orchestrator's schema). getActionFormInfo(query: GetActionFormInfoQuery, user: StepUser): Promise<{ hasForm: boolean }>; + // Full form structure for AI form-filling (PRD-509): field list (types, required, enum options), + // completeness (canExecute / requiredFields), and any values dropped by change hooks. + getActionForm(query: GetActionFormQuery, user: StepUser): Promise; // Startup healthcheck. Throws AgentProbeError on network error, timeout, or non-2xx. // JWT is not verified here — it's validated naturally when the first step runs. probe(): Promise; diff --git a/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts b/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts index f46f0c759b..678f280335 100644 --- a/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts +++ b/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts @@ -4,7 +4,13 @@ import { HttpRequester, createRemoteAgentClient } from '@forestadmin/agent-clien import jsonwebtoken from 'jsonwebtoken'; import AgentClientAgentPort from '../../src/adapters/agent-client-agent-port'; -import { AgentPortError, AgentProbeError, RecordNotFoundError } from '../../src/errors'; +import { + ActionFormValidationError, + ActionRequiresApprovalError, + AgentPortError, + AgentProbeError, + RecordNotFoundError, +} from '../../src/errors'; import SchemaCache from '../../src/schema-cache'; jest.mock('@forestadmin/agent-client', () => ({ @@ -20,7 +26,13 @@ const mockedIs404Error = HttpRequester.is404Error as jest.MockedFunction< >; function createMockClient() { - const mockAction = { execute: jest.fn(), getFields: jest.fn().mockReturnValue([]) }; + const mockAction = { + execute: jest.fn(), + getFields: jest.fn().mockReturnValue([]), + setFields: jest.fn().mockResolvedValue(undefined), + tryToSetFields: jest.fn().mockResolvedValue([]), + getEnumField: jest.fn(), + }; const mockRelation = { list: jest.fn() }; const mockCollection = { list: jest.fn(), @@ -723,6 +735,136 @@ describe('AgentClientAgentPort', () => { port.executeAction({ collection: 'users', action: 'sendEmail', id: [1] }, user), ).rejects.toThrow('Action failed'); }); + + it('sets pre-filled values (strict) before executing (PRD-509)', async () => { + mockAction.execute.mockResolvedValue({ success: 'done' }); + + await port.executeAction( + { collection: 'users', action: 'refund', id: [1], values: { amount: 50 } }, + user, + ); + + expect(mockAction.setFields).toHaveBeenCalledWith({ amount: 50 }); + expect(mockAction.execute).toHaveBeenCalled(); + }); + + it('maps a setFields failure (unknown field) to ActionFormValidationError', async () => { + mockAction.setFields.mockRejectedValue(new Error('Field "x" does not exist in this form')); + + await expect( + port.executeAction( + { collection: 'users', action: 'refund', id: [1], values: { x: 1 } }, + user, + ), + ).rejects.toBeInstanceOf(ActionFormValidationError); + expect(mockAction.execute).not.toHaveBeenCalled(); + }); + + it('maps an approval-403 to ActionRequiresApprovalError with the allowed roles (PRD-509)', async () => { + mockAction.execute.mockRejectedValue( + new Error( + JSON.stringify({ + error: { + status: 403, + text: JSON.stringify({ + errors: [ + { + name: 'CustomActionRequiresApprovalError', + data: { roleIdsAllowedToApprove: [7, 9] }, + }, + ], + }), + }, + body: null, + }), + ), + ); + + const error = await port + .executeAction({ collection: 'users', action: 'refund', id: [1] }, user) + .catch((e: unknown) => e); + + expect(error).toBeInstanceOf(ActionRequiresApprovalError); + expect((error as ActionRequiresApprovalError).roleIdsAllowedToApprove).toEqual([7, 9]); + }); + + it('maps a 422 validation rejection to ActionFormValidationError', async () => { + mockAction.execute.mockRejectedValue( + new Error(JSON.stringify({ error: { status: 422, text: 'invalid' }, body: null })), + ); + + await expect( + port.executeAction({ collection: 'users', action: 'refund', id: [1] }, user), + ).rejects.toBeInstanceOf(ActionFormValidationError); + }); + + it('leaves a plain permission 403 as a generic AgentPortError (step error, not a fallback)', async () => { + mockAction.execute.mockRejectedValue( + new Error(JSON.stringify({ error: { status: 403, text: 'Forbidden' }, body: null })), + ); + + await expect( + port.executeAction({ collection: 'users', action: 'refund', id: [1] }, user), + ).rejects.toBeInstanceOf(AgentPortError); + }); + }); + + describe('getActionForm', () => { + function makeField(over: { name: string; type?: string; value?: unknown; required?: boolean }) { + return { + getName: () => over.name, + getType: () => over.type ?? 'String', + getValue: () => over.value, + isRequired: () => over.required ?? false, + }; + } + + it('returns the field list, completeness and skipped fields (PRD-509)', async () => { + mockAction.tryToSetFields.mockResolvedValue(['ghost']); + mockAction.getFields.mockReturnValue([ + makeField({ name: 'amount', type: 'Number', value: 50, required: true }), + makeField({ name: 'reason', type: 'String', required: true }), + makeField({ name: 'tier', type: 'Enum', value: 'gold', required: false }), + ]); + mockAction.getEnumField.mockReturnValue({ getOptions: () => ['gold', 'silver'] }); + + const form = await port.getActionForm( + { collection: 'users', action: 'refund', id: [1], values: { amount: 50, ghost: 1 } }, + user, + ); + + expect(mockAction.tryToSetFields).toHaveBeenCalledWith({ amount: 50, ghost: 1 }); + expect(form.fields).toEqual([ + { name: 'amount', type: 'Number', value: 50, isRequired: true }, + { name: 'reason', type: 'String', value: undefined, isRequired: true }, + { + name: 'tier', + type: 'Enum', + value: 'gold', + isRequired: false, + enumValues: ['gold', 'silver'], + }, + ]); + // 'reason' is required and empty → not executable yet; 'ghost' was dropped by the form. + expect(form.canExecute).toBe(false); + expect(form.requiredFields).toEqual(['reason']); + expect(form.skippedFields).toEqual(['ghost']); + }); + + it('reports canExecute true when all required fields have values', async () => { + mockAction.getFields.mockReturnValue([ + makeField({ name: 'amount', type: 'Number', value: 50, required: true }), + ]); + + const form = await port.getActionForm( + { collection: 'users', action: 'refund', id: [1] }, + user, + ); + + expect(form.canExecute).toBe(true); + expect(form.requiredFields).toEqual([]); + expect(mockAction.tryToSetFields).not.toHaveBeenCalled(); + }); }); describe('getActionFormInfo', () => {