From ef73293a70b2301d31ea08c65d20d0e72aa9366c Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Fri, 3 Jul 2026 10:41:05 +0200 Subject: [PATCH 1/3] feat(agent-client): attach AI reasoning as a comment on approval requests When an AI triggers an approval-gated custom action (via the MCP executeAction tool or the workflow-executor trigger-action step), the created approval request now carries the AI's reasoning as a comment so the approver understands why it was triggered. - agent-client: Action.execute() accepts an optional message; the approval request creator posts it to /api/action-approvals/:id/comments (best-effort). - mcp-server: executeAction exposes an optional `reasoning` field forwarded to the approval request. - workflow-executor: the trigger-action step captures the AI action-selection reasoning (previously discarded), falling back to the step prompt, and threads it through as approvalMessage. Co-Authored-By: Claude Fable 5 --- .../src/approval-request-creator.ts | 22 +++++- packages/agent-client/src/domains/action.ts | 6 +- .../test/approval-request-creator.test.ts | 70 +++++++++++++++++++ .../agent-client/test/domains/action.test.ts | 26 +++++++ .../mcp-server/src/tools/execute-action.ts | 16 ++++- .../test/tools/execute-action.test.ts | 28 ++++++++ .../src/adapters/agent-client-agent-port.ts | 4 +- .../trigger-record-action-step-executor.ts | 24 +++++-- .../workflow-executor/src/ports/agent-port.ts | 3 + .../adapters/agent-client-agent-port.test.ts | 19 +++++ ...rigger-record-action-step-executor.test.ts | 51 ++++++++++++-- 11 files changed, 252 insertions(+), 17 deletions(-) diff --git a/packages/agent-client/src/approval-request-creator.ts b/packages/agent-client/src/approval-request-creator.ts index 2303fc420a..e91552a9fb 100644 --- a/packages/agent-client/src/approval-request-creator.ts +++ b/packages/agent-client/src/approval-request-creator.ts @@ -7,6 +7,7 @@ export type ApprovalRequestPayload = { actionName: string; recordIds: (string | number)[]; inputs: ApprovalRequestInput[]; + message?: string; }; export type CreateApprovalRequest = ( @@ -43,6 +44,25 @@ export default function makeCreateApprovalRequest(options: { }, }); - return body?.data?.id ? { id: String(body.data.id) } : undefined; + const id = body?.data?.id ? String(body.data.id) : undefined; + + if (id && payload.message) { + // Best-effort: the approval already exists, so a comment failure must not turn the + // successful request into an error. + try { + await ServerUtils.queryWithBearerToken({ + forestServerUrl: options.forestServerUrl, + bearerToken: options.forestServerToken, + method: 'post', + path: `${APPROVAL_REQUEST_PATH}/${id}/comments`, + headers: { 'forest-rendering-id': String(options.renderingId) }, + body: { data: { attributes: { comment: payload.message } } }, + }); + } catch { + /* approval created; the message is optional context */ + } + } + + return id ? { id } : undefined; }; } diff --git a/packages/agent-client/src/domains/action.ts b/packages/agent-client/src/domains/action.ts index 652661e805..51a5ac067a 100644 --- a/packages/agent-client/src/domains/action.ts +++ b/packages/agent-client/src/domains/action.ts @@ -123,7 +123,10 @@ export default class Action { this.createApprovalRequest = createApprovalRequest; } - async execute(signedApprovalRequest?: Record): Promise { + async execute( + signedApprovalRequest?: Record, + approvalRequestMessage?: string, + ): Promise { const requestBody = { data: { attributes: { @@ -161,6 +164,7 @@ export default class Action { actionName: this.actionName, recordIds: this.ids ?? [], inputs, + ...(approvalRequestMessage && { message: approvalRequestMessage }), }); } catch (cause) { throw new ApprovalRequestCreationError(cause); diff --git a/packages/agent-client/test/approval-request-creator.test.ts b/packages/agent-client/test/approval-request-creator.test.ts index 4ce1df9f75..b39c1a7e22 100644 --- a/packages/agent-client/test/approval-request-creator.test.ts +++ b/packages/agent-client/test/approval-request-creator.test.ts @@ -46,6 +46,76 @@ describe('makeCreateApprovalRequest', () => { }); }); + it('posts the message as a comment on the created approval', async () => { + queryWithBearerToken.mockResolvedValueOnce({ data: { id: 'req_42' } }); + const create = makeCreateApprovalRequest({ + forestServerUrl: 'https://api.forestadmin.com', + forestServerToken: 'server-token', + renderingId: 42, + }); + + const result = await create({ + collectionName: 'users', + actionName: 'refund', + recordIds: ['1'], + inputs: [], + message: 'Refund requested by AI: duplicate payment detected', + }); + + expect(queryWithBearerToken).toHaveBeenCalledTimes(2); + expect(queryWithBearerToken).toHaveBeenLastCalledWith({ + forestServerUrl: 'https://api.forestadmin.com', + bearerToken: 'server-token', + method: 'post', + path: '/api/action-approvals/req_42/comments', + headers: { 'forest-rendering-id': '42' }, + body: { + data: { attributes: { comment: 'Refund requested by AI: duplicate payment detected' } }, + }, + }); + expect(result).toEqual({ id: 'req_42' }); + }); + + it('skips the comment when no approval id came back', async () => { + queryWithBearerToken.mockResolvedValueOnce({ data: {} }); + const create = makeCreateApprovalRequest({ + forestServerUrl: 'https://api.forestadmin.com', + forestServerToken: 'server-token', + renderingId: 42, + }); + + await create({ + collectionName: 'users', + actionName: 'refund', + recordIds: ['1'], + inputs: [], + message: 'some reasoning', + }); + + expect(queryWithBearerToken).toHaveBeenCalledTimes(1); + }); + + it('still returns the approval id when posting the comment fails', async () => { + queryWithBearerToken + .mockResolvedValueOnce({ data: { id: 'req_42' } }) + .mockRejectedValueOnce(new Error('comments route down')); + const create = makeCreateApprovalRequest({ + forestServerUrl: 'https://api.forestadmin.com', + forestServerToken: 'server-token', + renderingId: 42, + }); + + const result = await create({ + collectionName: 'users', + actionName: 'refund', + recordIds: ['1'], + inputs: [], + message: 'some reasoning', + }); + + expect(result).toEqual({ id: 'req_42' }); + }); + it('returns the approval id read from the server response data.id', async () => { queryWithBearerToken.mockResolvedValue({ data: { id: 'req_42', type: 'action-approvals' } }); const create = makeCreateApprovalRequest({ diff --git a/packages/agent-client/test/domains/action.test.ts b/packages/agent-client/test/domains/action.test.ts index 927152b359..a5ab3bbcc8 100644 --- a/packages/agent-client/test/domains/action.test.ts +++ b/packages/agent-client/test/domains/action.test.ts @@ -184,6 +184,32 @@ describe('Action', () => { expect(result).toEqual({ approvalRequested: true }); }); + it('forwards the approval message to the approval request creator', async () => { + fieldsFormStates.getFields.mockReturnValue([] as any); + const createApprovalRequest = jest.fn().mockResolvedValue(undefined); + const approvalAction = new Action( + 'users', + 'send-email', + httpRequester, + '/forest/actions/send-email', + fieldsFormStates, + ['1'], + undefined, + createApprovalRequest, + ); + httpRequester.query.mockRejectedValue( + new AgentHttpError(403, { + errors: [{ name: 'CustomActionRequiresApprovalError', detail: 'Needs approval' }], + }), + ); + + await approvalAction.execute(undefined, 'AI reasoning: user asked for a resend'); + + expect(createApprovalRequest).toHaveBeenCalledWith( + expect.objectContaining({ message: 'AI reasoning: user asked for a resend' }), + ); + }); + it('includes the approval request id when the creator returns one', async () => { fieldsFormStates.getFields.mockReturnValue([] as any); const createApprovalRequest = jest.fn().mockResolvedValue({ id: 'req_42' }); diff --git a/packages/mcp-server/src/tools/execute-action.ts b/packages/mcp-server/src/tools/execute-action.ts index 9c987ccc7d..0c6f7f3ab3 100644 --- a/packages/mcp-server/src/tools/execute-action.ts +++ b/packages/mcp-server/src/tools/execute-action.ts @@ -2,6 +2,8 @@ import type { ForestServerClient } from '../http-client'; import type { Logger } from '../server'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { z } from 'zod'; + import { createActionArgumentShape } from '../utils/action-helpers'; import { buildClientWithActions } from '../utils/agent-caller'; import registerToolWithLogging from '../utils/tool-with-logging'; @@ -12,6 +14,7 @@ interface ExecuteActionArgument { actionName: string; recordIds: (string | number)[] | null; values?: Record; + reasoning?: string; } export default function declareExecuteActionTool( @@ -20,7 +23,16 @@ export default function declareExecuteActionTool( logger: Logger, collectionNames: string[] = [], ): string { - const argumentShape = createActionArgumentShape(collectionNames); + const argumentShape = { + ...createActionArgumentShape(collectionNames), + reasoning: z + .string() + .optional() + .describe( + 'A clear explanation of why you are executing this action. ' + + 'Shown to the approver when the action requires an approval — always provide it.', + ), + }; return registerToolWithLogging( mcpServer, @@ -62,7 +74,7 @@ If you call executeAction with missing required fields, it will return an error await action.setFields(options.values); } - const result = await action.execute(); + const result = await action.execute(undefined, options.reasoning); if ('approvalRequested' in result) { return { diff --git a/packages/mcp-server/test/tools/execute-action.test.ts b/packages/mcp-server/test/tools/execute-action.test.ts index 190cbe61e2..eadddfa790 100644 --- a/packages/mcp-server/test/tools/execute-action.test.ts +++ b/packages/mcp-server/test/tools/execute-action.test.ts @@ -332,6 +332,34 @@ describe('declareExecuteActionTool', () => { }); }); + it('should forward the reasoning to execute so it reaches the approval request', async () => { + const mockExecute = jest.fn().mockResolvedValue({ approvalRequested: true }); + const mockAction = jest.fn().mockResolvedValue({ + execute: mockExecute, + setFields: jest.fn().mockResolvedValue(undefined), + }); + const mockCollection = jest.fn().mockReturnValue({ action: mockAction }); + mockBuildClientWithActions.mockResolvedValue({ + rpcClient: { collection: mockCollection }, + authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, + } as unknown as ReturnType); + + await registeredToolHandler( + { + collectionName: 'users', + actionName: 'refund', + recordIds: [1], + reasoning: 'Duplicate payment detected on order #42', + }, + mockExtra, + ); + + expect(mockExecute).toHaveBeenCalledWith( + undefined, + 'Duplicate payment detected on order #42', + ); + }); + describe('activity logging', () => { beforeEach(() => { const mockExecute = jest.fn().mockResolvedValue({ success: 'Action executed' }); 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 0e79649788..8868a1af7b 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -230,7 +230,7 @@ export default class AgentClientAgentPort implements AgentPort { } async executeAction( - { collection, action, id, values }: ExecuteActionQuery, + { collection, action, id, values, approvalMessage }: ExecuteActionQuery, { user, forestServerToken }: ActionCaller, ): Promise { return this.callAgent('executeAction', async () => { @@ -249,7 +249,7 @@ export default class AgentClientAgentPort implements AgentPort { } try { - const executeResult = await act.execute(); + const executeResult = await act.execute(undefined, approvalMessage); return typeof executeResult === 'object' && executeResult !== null && 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 a09af61908..c3763834e4 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 @@ -43,6 +43,9 @@ Important rules: interface ActionTarget extends ActionRef { selectedRecordRef: RecordRef; isGlobal?: boolean; + // AI reasoning (or the step prompt as fallback) forwarded to the approval request + // when the action is approval-gated, so the approver sees why it was triggered. + approvalMessage?: string; } export default class TriggerRecordActionStepExecutor extends RecordStepExecutor { @@ -141,18 +144,24 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< : await this.resolveRecordRef(await this.getAvailableRecordRefs(), step.prompt); const schema = await this.getCollectionSchema(selectedRecordRef.collectionName); const recordedAction = preRecordedArgs?.actionName; - const actionName = recordedAction ?? (await this.selectAction(schema, step.prompt)).actionName; + const selection = recordedAction + ? { actionName: recordedAction } + : await this.selectAction(schema, step.prompt); + const { actionName } = selection; const action = this.findActionByTechnicalName(schema, actionName); if (!action) { throw new ActionNotFoundError(actionName, schema.collectionName); } + const approvalMessage = ('reasoning' in selection && selection.reasoning) || step.prompt; + const target: ActionTarget = { selectedRecordRef, displayName: action.displayName, name: action.name, isGlobal: action.type === 'global', + ...(approvalMessage && { approvalMessage }), }; const form = await this.context.agent.getActionForm({ @@ -377,6 +386,7 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< // Global actions run on no record — omit the id so the approval isn't linked to one. ...(target.isGlobal ? {} : { id: selectedRecordRef.recordId }), ...(form && { values: form.values }), + ...(target.approvalMessage && { approvalMessage: target.approvalMessage }), }, { beforeCall: () => @@ -485,7 +495,7 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< private async selectAction( schema: CollectionSchema, prompt: string | undefined, - ): Promise<{ actionName: string }> { + ): Promise<{ actionName: string; reasoning?: string }> { const tool = this.buildSelectActionTool(schema); const messages = [ this.buildContextMessage(), @@ -497,12 +507,12 @@ export default class TriggerRecordActionStepExecutor extends RecordStepExecutor< new HumanMessage(`**Request**: ${prompt ?? 'Trigger the relevant action.'}`), ]; - const { actionName } = await this.invokeWithTool<{ actionName: string; reasoning: string }>( - messages, - tool, - ); + const { actionName, reasoning } = await this.invokeWithTool<{ + actionName: string; + reasoning: string; + }>(messages, tool); - return { actionName: this.findAction(schema, actionName)?.name ?? actionName }; + return { actionName: this.findAction(schema, actionName)?.name ?? actionName, reasoning }; } private buildSelectActionTool(schema: CollectionSchema): DynamicStructuredTool { diff --git a/packages/workflow-executor/src/ports/agent-port.ts b/packages/workflow-executor/src/ports/agent-port.ts index 8bb9d128ce..0e59298241 100644 --- a/packages/workflow-executor/src/ports/agent-port.ts +++ b/packages/workflow-executor/src/ports/agent-port.ts @@ -43,6 +43,9 @@ export type ExecuteActionQuery = { // Pre-filled form values. Set on the form before execution, going through the agent's // normal server-side validation — no bypass. Omitted for formless actions. values?: Record; + // AI explanation attached to the approval request when the action is approval-gated, + // so the approver sees why the action was triggered. + approvalMessage?: string; }; export type GetActionFormQuery = { 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 d6b0f78c5c..d35c3e4d17 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 @@ -784,6 +784,25 @@ describe('AgentClientAgentPort', () => { expect(result).toEqual({ approvalRequested: true, approvalRequest: { id: 'req_42' } }); }); + it('forwards the approvalMessage to execute so it reaches the approval request', async () => { + mockAction.execute.mockResolvedValue({ approvalRequested: true }); + + await port.executeAction( + { + collection: 'users', + action: 'sendEmail', + id: [1], + approvalMessage: 'AI reasoning: resend requested by the workflow', + }, + { user }, + ); + + expect(mockAction.execute).toHaveBeenCalledWith( + undefined, + 'AI reasoning: resend requested by the workflow', + ); + }); + it('wires the forestServer connection into agent-client when a server token is supplied', async () => { const portWithServer = new AgentClientAgentPort({ agentUrl: 'http://localhost:3310', 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 bcf25ca224..730806b536 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 @@ -227,7 +227,12 @@ describe('TriggerRecordActionStepExecutor', () => { expect(result.stepOutcome.status).toBe('success'); expect(agentPort.executeAction).toHaveBeenCalledWith( - { collection: 'customers', action: 'send-welcome-email', id: [42] }, + { + collection: 'customers', + action: 'send-welcome-email', + id: [42], + approvalMessage: 'User requested welcome email', + }, { user: expect.objectContaining({ id: 1 }), forestServerToken: undefined }, ); expect(runStore.saveStepExecution).toHaveBeenCalledWith( @@ -274,7 +279,11 @@ describe('TriggerRecordActionStepExecutor', () => { expect(result.stepOutcome.status).toBe('success'); // The query carries no `id` for a global action. const query = (agentPort.executeAction as jest.Mock).mock.calls[0][0]; - expect(query).toEqual({ collection: 'customers', action: 'send-welcome-email' }); + expect(query).toEqual({ + collection: 'customers', + action: 'send-welcome-email', + approvalMessage: 'User requested welcome email', + }); expect('id' in query).toBe(false); }); @@ -1176,7 +1185,12 @@ describe('TriggerRecordActionStepExecutor', () => { expect(result.stepOutcome.status).toBe('success'); expect(agentPort.executeAction).toHaveBeenCalledWith( - { collection: 'customers', action: 'archive', id: [42] }, + { + collection: 'customers', + action: 'archive', + id: [42], + approvalMessage: 'User wants to archive', + }, { user: expect.objectContaining({ id: 1 }), forestServerToken: undefined }, ); }); @@ -1206,7 +1220,12 @@ describe('TriggerRecordActionStepExecutor', () => { expect(result.stepOutcome.status).toBe('success'); expect(agentPort.executeAction).toHaveBeenCalledWith( - { collection: 'customers', action: 'archive', id: [42] }, + { + collection: 'customers', + action: 'archive', + id: [42], + approvalMessage: 'fallback to technical name', + }, { user: expect.objectContaining({ id: 1 }), forestServerToken: undefined }, ); }); @@ -1592,6 +1611,30 @@ describe('TriggerRecordActionStepExecutor', () => { ); }); + it('falls back to the step prompt as approvalMessage when the action is pre-recorded', async () => { + const agentPort = makeMockAgentPort(); + (agentPort.executeAction as jest.Mock).mockResolvedValue({ result: { ok: true } }); + const mockModel = makeMockModel(); + const context = makeContext({ + model: mockModel.model, + agentPort, + stepDefinition: makeStep({ + executionType: StepExecutionMode.FullyAutomated, + preRecordedArgs: { actionName: 'send-welcome-email' }, + }), + }); + const executor = new TriggerRecordActionStepExecutor(context); + + await executor.execute(); + + expect(agentPort.executeAction).toHaveBeenCalledWith( + expect.objectContaining({ + approvalMessage: 'Send a welcome email to the customer', + }), + expect.anything(), + ); + }); + it('still goes through awaiting-input when executionType is not FullyAutomated', async () => { const mockModel = makeMockModel(); const runStore = makeMockRunStore(); From 61ed56f379c0a38266f38ddd5aff9929069d0bf2 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Fri, 3 Jul 2026 10:50:54 +0200 Subject: [PATCH 2/3] refactor(agent-client): use options object for Action.execute + address review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Action.execute now takes an ActionExecuteOptions object instead of two positional optionals (signedApprovalRequest, approvalRequestMessage). - approval-request-creator: bind and warn on a failed comment POST instead of a bare silent catch (keeps best-effort semantics). - Add tests for the AI-empty-reasoning → step.prompt fallback and the no-message-omitted path in the trigger-action executor. Co-Authored-By: Claude Fable 5 --- .../src/approval-request-creator.ts | 11 ++++- packages/agent-client/src/domains/action.ts | 13 ++++-- .../test/approval-request-creator.test.ts | 5 ++- .../agent-client/test/domains/action.test.ts | 6 ++- .../mcp-server/src/tools/execute-action.ts | 2 +- .../test/tools/execute-action.test.ts | 7 ++-- .../src/adapters/agent-client-agent-port.ts | 2 +- .../adapters/agent-client-agent-port.test.ts | 7 ++-- ...rigger-record-action-step-executor.test.ts | 42 +++++++++++++++++++ 9 files changed, 76 insertions(+), 19 deletions(-) diff --git a/packages/agent-client/src/approval-request-creator.ts b/packages/agent-client/src/approval-request-creator.ts index e91552a9fb..fe197800c2 100644 --- a/packages/agent-client/src/approval-request-creator.ts +++ b/packages/agent-client/src/approval-request-creator.ts @@ -58,8 +58,15 @@ export default function makeCreateApprovalRequest(options: { headers: { 'forest-rendering-id': String(options.renderingId) }, body: { data: { attributes: { comment: payload.message } } }, }); - } catch { - /* approval created; the message is optional context */ + } catch (error) { + // The approval already exists; the comment is optional context, so don't fail the + // request. Warn (rather than swallow silently) so a persistently broken comments + // route — or a bug in this block — is discoverable. + // eslint-disable-next-line no-console + console.warn( + `Approval request ${id} created, but posting the reasoning comment failed`, + error, + ); } } diff --git a/packages/agent-client/src/domains/action.ts b/packages/agent-client/src/domains/action.ts index 51a5ac067a..bbf0ff0a77 100644 --- a/packages/agent-client/src/domains/action.ts +++ b/packages/agent-client/src/domains/action.ts @@ -83,6 +83,13 @@ export type BaseActionContext = { recordIds?: RecordId[]; }; +export type ActionExecuteOptions = { + // Signed approval token replayed when an approver confirms an approval-gated action. + signedApprovalRequest?: Record; + // AI reasoning attached as a comment on the approval request when the action is approval-gated. + approvalRequestMessage?: string; +}; + export type ActionExecuteResult = | { success: string; html?: string } | { approvalRequested: true; approvalRequest?: { id: string } }; @@ -123,10 +130,8 @@ export default class Action { this.createApprovalRequest = createApprovalRequest; } - async execute( - signedApprovalRequest?: Record, - approvalRequestMessage?: string, - ): Promise { + async execute(options: ActionExecuteOptions = {}): Promise { + const { signedApprovalRequest, approvalRequestMessage } = options; const requestBody = { data: { attributes: { diff --git a/packages/agent-client/test/approval-request-creator.test.ts b/packages/agent-client/test/approval-request-creator.test.ts index b39c1a7e22..c868f370ae 100644 --- a/packages/agent-client/test/approval-request-creator.test.ts +++ b/packages/agent-client/test/approval-request-creator.test.ts @@ -95,10 +95,11 @@ describe('makeCreateApprovalRequest', () => { expect(queryWithBearerToken).toHaveBeenCalledTimes(1); }); - it('still returns the approval id when posting the comment fails', async () => { + it('still returns the approval id (and warns) when posting the comment fails', async () => { queryWithBearerToken .mockResolvedValueOnce({ data: { id: 'req_42' } }) .mockRejectedValueOnce(new Error('comments route down')); + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); const create = makeCreateApprovalRequest({ forestServerUrl: 'https://api.forestadmin.com', forestServerToken: 'server-token', @@ -114,6 +115,8 @@ describe('makeCreateApprovalRequest', () => { }); expect(result).toEqual({ id: 'req_42' }); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('req_42'), expect.any(Error)); + warn.mockRestore(); }); it('returns the approval id read from the server response data.id', async () => { diff --git a/packages/agent-client/test/domains/action.test.ts b/packages/agent-client/test/domains/action.test.ts index a5ab3bbcc8..423cfe770e 100644 --- a/packages/agent-client/test/domains/action.test.ts +++ b/packages/agent-client/test/domains/action.test.ts @@ -106,7 +106,7 @@ describe('Action', () => { httpRequester.query.mockResolvedValue({ success: 'Action executed' }); const signedApprovalRequest = { token: 'approval-token', requesterId: '123' }; - await action.execute(signedApprovalRequest); + await action.execute({ signedApprovalRequest }); expect(httpRequester.query).toHaveBeenCalledWith({ method: 'post', @@ -203,7 +203,9 @@ describe('Action', () => { }), ); - await approvalAction.execute(undefined, 'AI reasoning: user asked for a resend'); + await approvalAction.execute({ + approvalRequestMessage: 'AI reasoning: user asked for a resend', + }); expect(createApprovalRequest).toHaveBeenCalledWith( expect.objectContaining({ message: 'AI reasoning: user asked for a resend' }), diff --git a/packages/mcp-server/src/tools/execute-action.ts b/packages/mcp-server/src/tools/execute-action.ts index 0c6f7f3ab3..7b28c2c910 100644 --- a/packages/mcp-server/src/tools/execute-action.ts +++ b/packages/mcp-server/src/tools/execute-action.ts @@ -74,7 +74,7 @@ If you call executeAction with missing required fields, it will return an error await action.setFields(options.values); } - const result = await action.execute(undefined, options.reasoning); + const result = await action.execute({ approvalRequestMessage: options.reasoning }); if ('approvalRequested' in result) { return { diff --git a/packages/mcp-server/test/tools/execute-action.test.ts b/packages/mcp-server/test/tools/execute-action.test.ts index eadddfa790..ca606ad68d 100644 --- a/packages/mcp-server/test/tools/execute-action.test.ts +++ b/packages/mcp-server/test/tools/execute-action.test.ts @@ -354,10 +354,9 @@ describe('declareExecuteActionTool', () => { mockExtra, ); - expect(mockExecute).toHaveBeenCalledWith( - undefined, - 'Duplicate payment detected on order #42', - ); + expect(mockExecute).toHaveBeenCalledWith({ + approvalRequestMessage: 'Duplicate payment detected on order #42', + }); }); describe('activity logging', () => { 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 8868a1af7b..d69c4e6a58 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -249,7 +249,7 @@ export default class AgentClientAgentPort implements AgentPort { } try { - const executeResult = await act.execute(undefined, approvalMessage); + const executeResult = await act.execute({ approvalRequestMessage: approvalMessage }); return typeof executeResult === 'object' && executeResult !== null && 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 d35c3e4d17..4533deb88a 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 @@ -797,10 +797,9 @@ describe('AgentClientAgentPort', () => { { user }, ); - expect(mockAction.execute).toHaveBeenCalledWith( - undefined, - 'AI reasoning: resend requested by the workflow', - ); + expect(mockAction.execute).toHaveBeenCalledWith({ + approvalRequestMessage: 'AI reasoning: resend requested by the workflow', + }); }); it('wires the forestServer connection into agent-client when a server token is supplied', async () => { 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 730806b536..830bce2107 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 @@ -253,6 +253,48 @@ describe('TriggerRecordActionStepExecutor', () => { ); }); + it('falls back to the step prompt as approvalMessage when the AI returns no reasoning', async () => { + const agentPort = makeMockAgentPort(); + (agentPort.executeAction as jest.Mock).mockResolvedValue({ result: { ok: true } }); + const mockModel = makeMockModel({ actionName: 'Send Welcome Email' }); + const context = makeContext({ + model: mockModel.model, + agentPort, + stepDefinition: makeStep({ + executionType: StepExecutionMode.FullyAutomated, + prompt: 'Send a welcome email to the customer', + }), + }); + const executor = new TriggerRecordActionStepExecutor(context); + + await executor.execute(); + + expect(agentPort.executeAction).toHaveBeenCalledWith( + expect.objectContaining({ approvalMessage: 'Send a welcome email to the customer' }), + expect.anything(), + ); + }); + + it('omits approvalMessage when the AI returns no reasoning and there is no prompt', async () => { + const agentPort = makeMockAgentPort(); + (agentPort.executeAction as jest.Mock).mockResolvedValue({ result: { ok: true } }); + const mockModel = makeMockModel({ actionName: 'Send Welcome Email' }); + const context = makeContext({ + model: mockModel.model, + agentPort, + stepDefinition: makeStep({ + executionType: StepExecutionMode.FullyAutomated, + prompt: undefined, + }), + }); + const executor = new TriggerRecordActionStepExecutor(context); + + await executor.execute(); + + const query = (agentPort.executeAction as jest.Mock).mock.calls[0][0]; + expect('approvalMessage' in query).toBe(false); + }); + it('does NOT attach a record when the action is global', async () => { const agentPort = makeMockAgentPort(); (agentPort.executeAction as jest.Mock).mockResolvedValue({ result: { ok: true } }); From 787b9bea5c216768489a78ee47a1fc4bd708567f Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Fri, 3 Jul 2026 11:01:26 +0200 Subject: [PATCH 3/3] docs(workflow-executor): trim redundant comments on approval message threading Keep only the non-obvious rationale (best-effort comment post, comment destination, prompt fallback); drop comments that restate self-documenting names. Co-Authored-By: Claude Fable 5 --- packages/agent-client/src/approval-request-creator.ts | 6 +----- packages/agent-client/src/domains/action.ts | 3 +-- .../src/executors/trigger-record-action-step-executor.ts | 2 -- packages/workflow-executor/src/ports/agent-port.ts | 3 +-- 4 files changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/agent-client/src/approval-request-creator.ts b/packages/agent-client/src/approval-request-creator.ts index fe197800c2..861b21c30c 100644 --- a/packages/agent-client/src/approval-request-creator.ts +++ b/packages/agent-client/src/approval-request-creator.ts @@ -46,9 +46,8 @@ export default function makeCreateApprovalRequest(options: { const id = body?.data?.id ? String(body.data.id) : undefined; + // Best-effort: the approval already exists, so a comment failure must not fail the request. if (id && payload.message) { - // Best-effort: the approval already exists, so a comment failure must not turn the - // successful request into an error. try { await ServerUtils.queryWithBearerToken({ forestServerUrl: options.forestServerUrl, @@ -59,9 +58,6 @@ export default function makeCreateApprovalRequest(options: { body: { data: { attributes: { comment: payload.message } } }, }); } catch (error) { - // The approval already exists; the comment is optional context, so don't fail the - // request. Warn (rather than swallow silently) so a persistently broken comments - // route — or a bug in this block — is discoverable. // eslint-disable-next-line no-console console.warn( `Approval request ${id} created, but posting the reasoning comment failed`, diff --git a/packages/agent-client/src/domains/action.ts b/packages/agent-client/src/domains/action.ts index bbf0ff0a77..44990589d9 100644 --- a/packages/agent-client/src/domains/action.ts +++ b/packages/agent-client/src/domains/action.ts @@ -84,9 +84,8 @@ export type BaseActionContext = { }; export type ActionExecuteOptions = { - // Signed approval token replayed when an approver confirms an approval-gated action. signedApprovalRequest?: Record; - // AI reasoning attached as a comment on the approval request when the action is approval-gated. + // Posted as a comment on the approval request when the action is approval-gated. approvalRequestMessage?: string; }; 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 c3763834e4..fd354e40f0 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 @@ -43,8 +43,6 @@ Important rules: interface ActionTarget extends ActionRef { selectedRecordRef: RecordRef; isGlobal?: boolean; - // AI reasoning (or the step prompt as fallback) forwarded to the approval request - // when the action is approval-gated, so the approver sees why it was triggered. approvalMessage?: string; } diff --git a/packages/workflow-executor/src/ports/agent-port.ts b/packages/workflow-executor/src/ports/agent-port.ts index 0e59298241..32228f2932 100644 --- a/packages/workflow-executor/src/ports/agent-port.ts +++ b/packages/workflow-executor/src/ports/agent-port.ts @@ -43,8 +43,7 @@ export type ExecuteActionQuery = { // Pre-filled form values. Set on the form before execution, going through the agent's // normal server-side validation — no bypass. Omitted for formless actions. values?: Record; - // AI explanation attached to the approval request when the action is approval-gated, - // so the approver sees why the action was triggered. + // AI reasoning, posted as a comment on the approval request when the action is approval-gated. approvalMessage?: string; };