Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion packages/agent-client/src/approval-request-creator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export type ApprovalRequestPayload = {
actionName: string;
recordIds: (string | number)[];
inputs: ApprovalRequestInput[];
message?: string;
};

export type CreateApprovalRequest = (
Expand Down Expand Up @@ -43,6 +44,28 @@ 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;

// Best-effort: the approval already exists, so a comment failure must not fail the request.
if (id && payload.message) {
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 (error) {
// eslint-disable-next-line no-console
console.warn(
`Approval request ${id} created, but posting the reasoning comment failed`,
error,
);
}
}

return id ? { id } : undefined;
};
}
10 changes: 9 additions & 1 deletion packages/agent-client/src/domains/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ export type BaseActionContext = {
recordIds?: RecordId[];
};

export type ActionExecuteOptions = {
signedApprovalRequest?: Record<string, unknown>;
// Posted 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 } };
Expand Down Expand Up @@ -123,7 +129,8 @@ export default class Action {
this.createApprovalRequest = createApprovalRequest;
}

async execute(signedApprovalRequest?: Record<string, unknown>): Promise<ActionExecuteResult> {
async execute(options: ActionExecuteOptions = {}): Promise<ActionExecuteResult> {
const { signedApprovalRequest, approvalRequestMessage } = options;
const requestBody = {
data: {
attributes: {
Expand Down Expand Up @@ -161,6 +168,7 @@ export default class Action {
actionName: this.actionName,
recordIds: this.ids ?? [],
inputs,
...(approvalRequestMessage && { message: approvalRequestMessage }),
});
} catch (cause) {
throw new ApprovalRequestCreationError(cause);
Expand Down
73 changes: 73 additions & 0 deletions packages/agent-client/test/approval-request-creator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,79 @@ 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 (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',
renderingId: 42,
});

const result = await create({
collectionName: 'users',
actionName: 'refund',
recordIds: ['1'],
inputs: [],
message: 'some reasoning',
});

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 () => {
queryWithBearerToken.mockResolvedValue({ data: { id: 'req_42', type: 'action-approvals' } });
const create = makeCreateApprovalRequest({
Expand Down
30 changes: 29 additions & 1 deletion packages/agent-client/test/domains/action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

httpRequester = {
query: jest.fn(),
} as any;

Check warning on line 20 in packages/agent-client/test/domains/action.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (agent-client)

Unexpected any. Specify a different type

fieldsFormStates = {
getFieldValues: jest.fn().mockReturnValue({ email: 'test@example.com' }),
Expand All @@ -28,7 +28,7 @@
getType: () => 'String',
}),
getLayout: jest.fn().mockReturnValue([]),
} as any;

Check warning on line 31 in packages/agent-client/test/domains/action.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (agent-client)

Unexpected any. Specify a different type

action = new Action(
'users',
Expand Down Expand Up @@ -98,7 +98,7 @@

await action.execute();

const body = httpRequester.query.mock.calls[0][0].body as any;

Check warning on line 101 in packages/agent-client/test/domains/action.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (agent-client)

Unexpected any. Specify a different type
expect(body.data.attributes).not.toHaveProperty('smart_action_id');
});

Expand All @@ -106,7 +106,7 @@
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',
Expand Down Expand Up @@ -149,7 +149,7 @@
getType: () => 'String',
getValue: () => 'test@example.com',
},
] as any);

Check warning on line 152 in packages/agent-client/test/domains/action.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (agent-client)

Unexpected any. Specify a different type
const createApprovalRequest = jest.fn().mockResolvedValue(undefined);
const approvalAction = new Action(
'users',
Expand Down Expand Up @@ -184,8 +184,36 @@
expect(result).toEqual({ approvalRequested: true });
});

it('forwards the approval message to the approval request creator', async () => {
fieldsFormStates.getFields.mockReturnValue([] as any);

Check warning on line 188 in packages/agent-client/test/domains/action.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (agent-client)

Unexpected any. Specify a different type
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({
approvalRequestMessage: '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);

Check warning on line 216 in packages/agent-client/test/domains/action.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (agent-client)

Unexpected any. Specify a different type
const createApprovalRequest = jest.fn().mockResolvedValue({ id: 'req_42' });
const approvalAction = new Action(
'users',
Expand All @@ -209,7 +237,7 @@
});

it('throws ApprovalRequestCreationError when filing the approval request fails', async () => {
fieldsFormStates.getFields.mockReturnValue([] as any);

Check warning on line 240 in packages/agent-client/test/domains/action.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (agent-client)

Unexpected any. Specify a different type
const createApprovalRequest = jest.fn().mockRejectedValue(new Error('forest server down'));
const approvalAction = new Action(
'users',
Expand Down Expand Up @@ -337,7 +365,7 @@
fieldsFormStates.getField.mockImplementation((fieldName: string) => {
if (fieldName === 'nonexistent') return null;

return { getName: () => fieldName, getType: () => 'String' } as any;

Check warning on line 368 in packages/agent-client/test/domains/action.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (agent-client)

Unexpected any. Specify a different type
});

const skipped = await action.tryToSetFields({
Expand All @@ -362,8 +390,8 @@
describe('getFields', () => {
it('should return action fields', () => {
fieldsFormStates.getFields.mockReturnValue([
{ getName: () => 'email', getType: () => 'String' } as any,

Check warning on line 393 in packages/agent-client/test/domains/action.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (agent-client)

Unexpected any. Specify a different type
{ getName: () => 'count', getType: () => 'Number' } as any,

Check warning on line 394 in packages/agent-client/test/domains/action.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (agent-client)

Unexpected any. Specify a different type
]);

const fields = action.getFields();
Expand Down
16 changes: 14 additions & 2 deletions packages/mcp-server/src/tools/execute-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -12,6 +14,7 @@ interface ExecuteActionArgument {
actionName: string;
recordIds: (string | number)[] | null;
values?: Record<string, unknown>;
reasoning?: string;
}

export default function declareExecuteActionTool(
Expand All @@ -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,
Expand Down Expand Up @@ -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({ approvalRequestMessage: options.reasoning });

if ('approvalRequested' in result) {
return {
Expand Down
27 changes: 27 additions & 0 deletions packages/mcp-server/test/tools/execute-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,33 @@ 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<typeof buildClientWithActions>);

await registeredToolHandler(
{
collectionName: 'users',
actionName: 'refund',
recordIds: [1],
reasoning: 'Duplicate payment detected on order #42',
},
mockExtra,
);

expect(mockExecute).toHaveBeenCalledWith({
approvalRequestMessage: 'Duplicate payment detected on order #42',
});
});

describe('activity logging', () => {
beforeEach(() => {
const mockExecute = jest.fn().mockResolvedValue({ success: 'Action executed' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ExecuteActionResult> {
return this.callAgent('executeAction', async () => {
Expand All @@ -249,7 +249,7 @@ export default class AgentClientAgentPort implements AgentPort {
}

try {
const executeResult = await act.execute();
const executeResult = await act.execute({ approvalRequestMessage: approvalMessage });

return typeof executeResult === 'object' &&
executeResult !== null &&
Expand Down
Comment thread
Scra3 marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Important rules:
interface ActionTarget extends ActionRef {
selectedRecordRef: RecordRef;
isGlobal?: boolean;
approvalMessage?: string;
}

export default class TriggerRecordActionStepExecutor extends RecordStepExecutor<TriggerActionStepDefinition> {
Expand Down Expand Up @@ -141,18 +142,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({
Expand Down Expand Up @@ -377,6 +384,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: () =>
Expand Down Expand Up @@ -485,7 +493,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(),
Expand All @@ -497,12 +505,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 {
Expand Down
2 changes: 2 additions & 0 deletions packages/workflow-executor/src/ports/agent-port.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ 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<string, unknown>;
// AI reasoning, posted as a comment on the approval request when the action is approval-gated.
approvalMessage?: string;
};

export type GetActionFormQuery = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@

const mocks = createMockClient();
({ mockCollection, mockRelation, mockAction } = mocks);
mockedCreateRemoteAgentClient.mockReturnValue(mocks.client as any);

Check warning on line 114 in packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts

View workflow job for this annotation

GitHub Actions / Linting & Testing (workflow-executor)

Unexpected any. Specify a different type

const schemaCache = new SchemaCache();
schemaCache.set(1, 'users', {
Expand Down Expand Up @@ -784,6 +784,24 @@
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({
approvalRequestMessage: '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',
Expand Down
Loading
Loading