From 6f2156a676eb7d7f4b80112e06f0a590ba8cf12c Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Fri, 10 Jul 2026 15:36:37 +0200 Subject: [PATCH 1/6] feat(agent-client): forward action error html and type the unknown-field error Carry the native action Error result's html on ActionFormValidationError and throw a typed UnknownActionFieldError from the strict setFields, so consumers can render error markup and route unknown fields to a client error. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/agent-client/src/domains/action.ts | 6 ++++-- packages/agent-client/src/errors.ts | 12 ++++++++++- packages/agent-client/src/index.ts | 2 ++ .../agent-client/test/domains/action.test.ts | 20 +++++++++++++++++-- 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/packages/agent-client/src/domains/action.ts b/packages/agent-client/src/domains/action.ts index 44990589d9..bf9b042a4e 100644 --- a/packages/agent-client/src/domains/action.ts +++ b/packages/agent-client/src/domains/action.ts @@ -22,6 +22,7 @@ import AgentHttpError, { ActionFormValidationError, ActionRequiresApprovalError, ApprovalRequestCreationError, + UnknownActionFieldError, } from '../errors'; // JSON:API error body the agent returns on a rejected action. @@ -35,6 +36,7 @@ type ActionErrorBody = { }[]; error?: string; message?: string; + html?: string; data?: { roleIdsAllowedToApprove?: number[] }; }; @@ -72,7 +74,7 @@ function toActionError(error: unknown): unknown { } if (error.status === 400 || error.status === 422) { - return new ActionFormValidationError(detail ?? 'The action form values were rejected.'); + return new ActionFormValidationError(detail ?? 'The action form values were rejected.', body.html); } return error; @@ -184,7 +186,7 @@ export default class Action { async setFields(fields: Record): Promise { for (const [fieldName, value] of Object.entries(fields)) { if (!this.doesFieldExist(fieldName)) { - throw new Error(`Field "${fieldName}" does not exist in this form`); + throw new UnknownActionFieldError(fieldName); } // eslint-disable-next-line no-await-in-loop diff --git a/packages/agent-client/src/errors.ts b/packages/agent-client/src/errors.ts index cf3e8e9670..c824c0cab0 100644 --- a/packages/agent-client/src/errors.ts +++ b/packages/agent-client/src/errors.ts @@ -19,12 +19,22 @@ export class ActionRequiresApprovalError extends Error { } export class ActionFormValidationError extends Error { - constructor(message: string) { + constructor( + message: string, + readonly html?: string, + ) { super(message); this.name = 'ActionFormValidationError'; } } +export class UnknownActionFieldError extends Error { + constructor(readonly fieldName: string) { + super(`Field "${fieldName}" does not exist in this form`); + this.name = 'UnknownActionFieldError'; + } +} + // The action is approval-gated, but filing the approval request failed — distinct from the action // itself failing, so the caller can tell the two apart. export class ApprovalRequestCreationError extends Error { diff --git a/packages/agent-client/src/index.ts b/packages/agent-client/src/index.ts index 0b4665c186..a7e3951ad3 100644 --- a/packages/agent-client/src/index.ts +++ b/packages/agent-client/src/index.ts @@ -13,6 +13,7 @@ import AgentHttpError, { ActionFormValidationError, ActionRequiresApprovalError, ApprovalRequestCreationError, + UnknownActionFieldError, } from './errors'; import HttpRequester from './http-requester'; @@ -26,6 +27,7 @@ export { ActionRequiresApprovalError, ActionFormValidationError, ApprovalRequestCreationError, + UnknownActionFieldError, }; export type { ActionEndpointsByCollection, diff --git a/packages/agent-client/test/domains/action.test.ts b/packages/agent-client/test/domains/action.test.ts index 423cfe770e..6810b2a084 100644 --- a/packages/agent-client/test/domains/action.test.ts +++ b/packages/agent-client/test/domains/action.test.ts @@ -303,6 +303,18 @@ describe('Action', () => { }); }); + it('forwards the html from a native action Error result', async () => { + httpRequester.query.mockRejectedValue( + new AgentHttpError(400, { error: 'Refund failed', html: 'Nope' }), + ); + + await expect(action.execute()).rejects.toMatchObject({ + name: 'ActionFormValidationError', + message: 'Refund failed', + html: 'Nope', + }); + }); + it('should propagate other HTTP errors unchanged', async () => { const error = new AgentHttpError(500, null); httpRequester.query.mockRejectedValue(error); @@ -338,14 +350,18 @@ describe('Action', () => { expect(callOrder).toEqual(['first', 'second', 'third']); }); - it('should throw when field does not exist', async () => { + it('should throw a typed UnknownActionFieldError when field does not exist', async () => { fieldsFormStates.getField.mockReturnValue(null); await expect( action.setFields({ nonexistent: 'value', }), - ).rejects.toThrow('Field "nonexistent" does not exist in this form'); + ).rejects.toMatchObject({ + name: 'UnknownActionFieldError', + fieldName: 'nonexistent', + message: 'Field "nonexistent" does not exist in this form', + }); }); }); From 605ed3e2cf836a18a7b9e6a0ad51c6132ff3a180 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Fri, 10 Jul 2026 15:36:44 +0200 Subject: [PATCH 2/6] feat(agent-bff): expose the action execute endpoint with result normalization Serve POST /v1/:collection/actions/:action/execute: reject unknown submitted fields via setFields, run the action, and normalize the agent result into a flat wrapper (success/error/webhook/redirect), returning 501 for File results. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/action/action-execute-mapper.ts | 81 ++++++ .../src/action/action-routes-middleware.ts | 108 +++++-- .../src/action/agent-action-client.ts | 11 +- .../agent-bff/src/http/bff-local-errors.ts | 7 + .../test/action/action-execute-mapper.test.ts | 68 +++++ .../action/action-routes-middleware.test.ts | 270 +++++++++++++++--- .../test/action/agent-action-client.test.ts | 4 +- 7 files changed, 489 insertions(+), 60 deletions(-) create mode 100644 packages/agent-bff/src/action/action-execute-mapper.ts create mode 100644 packages/agent-bff/test/action/action-execute-mapper.test.ts diff --git a/packages/agent-bff/src/action/action-execute-mapper.ts b/packages/agent-bff/src/action/action-execute-mapper.ts new file mode 100644 index 0000000000..d8dd7116fc --- /dev/null +++ b/packages/agent-bff/src/action/action-execute-mapper.ts @@ -0,0 +1,81 @@ +export interface ActionExecuteSuccessBody { + type: 'success'; + message: string | null; + invalidated: string[]; + html: string | null; +} + +export interface ActionExecuteWebhookBody { + type: 'webhook'; + url: unknown; + method: unknown; + headers: unknown; + body: unknown; +} + +export interface ActionExecuteRedirectBody { + type: 'redirect'; + path: unknown; +} + +export interface ActionExecuteUnsupportedBody { + error: { type: 'unsupported_action_result'; status: 501 }; +} + +export type ActionExecuteMappedBody = + | ActionExecuteSuccessBody + | ActionExecuteWebhookBody + | ActionExecuteRedirectBody + | ActionExecuteUnsupportedBody; + +export interface ActionExecuteMapped { + status: number; + body: ActionExecuteMappedBody; +} + +// Normalizes the agent's 200 execute payload into the flat BFF wrapper. The native ActionResult +// union never reaches the BFF (agent-client `execute()` is typed `{ success, html? }`), so we +// discriminate on the agent HTTP payload shape. A File result streams a binary with no JSON marker, +// so any unrecognized 200 body falls through to a structured 501 rather than being mislabelled. +export function mapActionExecuteResult(raw: unknown): ActionExecuteMapped { + const body = (typeof raw === 'object' && raw !== null ? raw : {}) as Record; + + if ('webhook' in body) { + const hook = ( + typeof body.webhook === 'object' && body.webhook !== null ? body.webhook : {} + ) as Record; + + return { + status: 200, + body: { + type: 'webhook', + url: hook.url, + method: hook.method, + headers: hook.headers, + body: hook.body, + }, + }; + } + + if ('redirectTo' in body) { + return { status: 200, body: { type: 'redirect', path: body.redirectTo } }; + } + + if ('success' in body || 'refresh' in body) { + const refresh = ( + typeof body.refresh === 'object' && body.refresh !== null ? body.refresh : {} + ) as { relationships?: unknown }; + + return { + status: 200, + body: { + type: 'success', + message: typeof body.success === 'string' ? body.success : null, + invalidated: Array.isArray(refresh.relationships) ? (refresh.relationships as string[]) : [], + html: typeof body.html === 'string' ? body.html : null, + }, + }; + } + + return { status: 501, body: { error: { type: 'unsupported_action_result', status: 501 } } }; +} diff --git a/packages/agent-bff/src/action/action-routes-middleware.ts b/packages/agent-bff/src/action/action-routes-middleware.ts index cde07412ac..956682b852 100644 --- a/packages/agent-bff/src/action/action-routes-middleware.ts +++ b/packages/agent-bff/src/action/action-routes-middleware.ts @@ -1,21 +1,29 @@ import type { AgentActionClient, AgentActionClientOptions } from './agent-action-client'; import type { Logger } from '../ports/logger-port'; import type ReadModelStore from '../read-model/read-model-store'; -import type { Middleware } from 'koa'; +import type { Context, Middleware } from 'koa'; +import { + ActionFormValidationError, + ActionRequiresApprovalError, + UnknownActionFieldError, +} from '@forestadmin/agent-client'; + +import { mapActionExecuteResult } from './action-execute-mapper'; import { mapActionForm } from './action-form-mapper'; import defaultCreateAgentActionClient, { extractRawLayout } from './agent-action-client'; +import { mapAgentError } from '../http/agent-error-mapper'; import { callAgent, decodeSegment, requireAgentToken, resolveReadModel, } from '../http/agent-route-helpers'; -import { invalidRequest, unknownAction } from '../http/bff-local-errors'; +import { actionRequiresApproval, invalidRequest, unknownAction } from '../http/bff-local-errors'; -const ACTION_FORM_ROUTE = /^\/agent\/v1\/([^/]+)\/actions\/([^/]+)\/form$/; +const ACTION_ROUTE = /^\/agent\/v1\/([^/]+)\/actions\/([^/]+)\/(form|execute)$/; -interface ActionFormRequestBody { +interface ActionRequestBody { recordIds?: unknown; values?: unknown; } @@ -50,6 +58,77 @@ export interface ActionRoutesMiddlewareOptions { createClient?: (options: AgentActionClientOptions) => AgentActionClient; } +async function handleForm( + ctx: Context, + client: AgentActionClient, + collection: string, + actionName: string, + recordIds: string[], + values: Record, + logger: Logger, +): Promise { + // Each agent-hitting call is wrapped on its own; getFields/extractRawLayout/mapping stay outside + // callAgent so a local BFF bug surfaces as a 500, not a mislabelled agent error. Fields and + // layout are read AFTER tryToSetFields because a change hook rebuilds them in place. + const action = await callAgent(() => client.loadAction(collection, actionName, recordIds), logger); + const skippedFields = await callAgent(() => action.tryToSetFields(values), logger); + + ctx.status = 200; + ctx.body = mapActionForm(action, skippedFields, extractRawLayout(action)); +} + +async function handleExecute( + ctx: Context, + client: AgentActionClient, + collection: string, + actionName: string, + recordIds: string[], + values: Record, + logger: Logger, +): Promise { + const action = await callAgent(() => client.loadAction(collection, actionName, recordIds), logger); + + // setFields is strict: an unknown submitted field is a client error (400), not a 500. A transport + // failure from the change-hook it triggers is a genuine agent error, so it goes to the mapper. + try { + await action.setFields(values); + } catch (error) { + if (error instanceof UnknownActionFieldError) throw invalidRequest(error.message); + throw mapAgentError(error, { logger }); + } + + // execute() cannot go through the generic callAgent: agent-client turns the native action Error + // (HTTP 400) into ActionFormValidationError, a non-AgentHttpError the mapper would mislabel as a + // transport 502. So the semantic outcomes are caught here, everything else falls to the mapper. + let raw: unknown; + + try { + raw = await action.execute(); + } catch (error) { + if (error instanceof ActionRequiresApprovalError) { + throw actionRequiresApproval( + error.message, + error.roleIdsAllowedToApprove + ? { roleIdsAllowedToApprove: error.roleIdsAllowedToApprove } + : undefined, + ); + } + + if (error instanceof ActionFormValidationError) { + ctx.status = 400; + ctx.body = { type: 'error', status: 400, message: error.message, html: error.html ?? null }; + + return; + } + + throw mapAgentError(error, { logger }); + } + + const { status, body } = mapActionExecuteResult(raw); + ctx.status = status; + ctx.body = body; +} + export default function createActionRoutesMiddleware({ store, agentUrl, @@ -57,7 +136,7 @@ export default function createActionRoutesMiddleware({ createClient = defaultCreateAgentActionClient, }: ActionRoutesMiddlewareOptions): Middleware { return async function actionRoutesMiddleware(ctx, next) { - const match = ACTION_FORM_ROUTE.exec(ctx.path); + const match = ACTION_ROUTE.exec(ctx.path); if (!match || ctx.method !== 'POST') { await next(); @@ -67,6 +146,7 @@ export default function createActionRoutesMiddleware({ const collection = decodeSegment(match[1], 'collection name'); const actionName = decodeSegment(match[2], 'action name'); + const verb = match[3]; const token = requireAgentToken(ctx); const readModel = await resolveReadModel(store); @@ -79,7 +159,7 @@ export default function createActionRoutesMiddleware({ throw unknownAction(`Unknown action: ${collection}.${actionName}`); } - const body = (ctx.request.body ?? {}) as ActionFormRequestBody; + const body = (ctx.request.body ?? {}) as ActionRequestBody; const recordIds = parseRecordIds(body.recordIds); const values = parseValues(body.values); @@ -89,16 +169,10 @@ export default function createActionRoutesMiddleware({ actionEndpoints: readModel.getActionEndpoints(), }); - // Each agent-hitting call is wrapped on its own; getFields/extractRawLayout/mapping stay outside - // callAgent so a local BFF bug surfaces as a 500, not a mislabelled agent error. Fields and - // layout are read AFTER tryToSetFields because a change hook rebuilds them in place. - const action = await callAgent( - () => client.loadActionForm(collection, actionName, recordIds), - logger, - ); - const skippedFields = await callAgent(() => action.tryToSetFields(values), logger); - - ctx.status = 200; - ctx.body = mapActionForm(action, skippedFields, extractRawLayout(action)); + if (verb === 'execute') { + await handleExecute(ctx, client, collection, actionName, recordIds, values, logger); + } else { + await handleForm(ctx, client, collection, actionName, recordIds, values, logger); + } }; } diff --git a/packages/agent-bff/src/action/agent-action-client.ts b/packages/agent-bff/src/action/agent-action-client.ts index c8ad34c0a6..a8c4dcdaa3 100644 --- a/packages/agent-bff/src/action/agent-action-client.ts +++ b/packages/agent-bff/src/action/agent-action-client.ts @@ -19,8 +19,15 @@ export interface ActionForm { getLayout(): unknown; } +// The form and execute endpoints load the same agent-client `Action` object; execute adds the +// strict setFields + execute members on top of the form ones. +export interface Action extends ActionForm { + setFields(values: Record): Promise; + execute(): Promise; +} + export interface AgentActionClient { - loadActionForm(collection: string, action: string, recordIds: string[]): Promise; + loadAction(collection: string, action: string, recordIds: string[]): Promise; } export interface AgentActionClientOptions { @@ -53,7 +60,7 @@ export default function createAgentActionClient({ const client = createRemoteAgentClient({ url: agentUrl, token, actionEndpoints }); return { - loadActionForm: (collection, action, recordIds) => + loadAction: (collection, action, recordIds) => client.collection(collection).action(action, { recordIds }), }; } diff --git a/packages/agent-bff/src/http/bff-local-errors.ts b/packages/agent-bff/src/http/bff-local-errors.ts index e4d37d95df..edfae8bb43 100644 --- a/packages/agent-bff/src/http/bff-local-errors.ts +++ b/packages/agent-bff/src/http/bff-local-errors.ts @@ -39,3 +39,10 @@ export function schemaUnavailable(message = 'The agent schema is unavailable'): export function unsupportedActionResult(message = 'Unsupported action result'): BffHttpError { return new BffHttpError(501, 'unsupported_action_result', message); } + +export function actionRequiresApproval( + message = 'This action requires an approval before it can run', + details?: unknown, +): BffHttpError { + return new BffHttpError(403, 'action_requires_approval', message, details); +} diff --git a/packages/agent-bff/test/action/action-execute-mapper.test.ts b/packages/agent-bff/test/action/action-execute-mapper.test.ts new file mode 100644 index 0000000000..503c6c5464 --- /dev/null +++ b/packages/agent-bff/test/action/action-execute-mapper.test.ts @@ -0,0 +1,68 @@ +import { mapActionExecuteResult } from '../../src/action/action-execute-mapper'; + +describe('mapActionExecuteResult', () => { + it('maps a Success payload, serializing refresh.relationships into invalidated', () => { + expect( + mapActionExecuteResult({ + success: 'Done', + html: 'ok', + refresh: { relationships: ['orders', 'items'] }, + }), + ).toEqual({ + status: 200, + body: { type: 'success', message: 'Done', invalidated: ['orders', 'items'], html: 'ok' }, + }); + }); + + it('defaults message and html to null and invalidated to [] when absent', () => { + expect(mapActionExecuteResult({ success: undefined })).toEqual({ + status: 200, + body: { type: 'success', message: null, invalidated: [], html: null }, + }); + }); + + it('treats a bare refresh payload as a Success', () => { + expect(mapActionExecuteResult({ refresh: { relationships: ['orders'] } })).toEqual({ + status: 200, + body: { type: 'success', message: null, invalidated: ['orders'], html: null }, + }); + }); + + it('maps a Webhook payload verbatim', () => { + expect( + mapActionExecuteResult({ + webhook: { url: 'https://x.test', method: 'POST', headers: { a: '1' }, body: { b: 2 } }, + }), + ).toEqual({ + status: 200, + body: { + type: 'webhook', + url: 'https://x.test', + method: 'POST', + headers: { a: '1' }, + body: { b: 2 }, + }, + }); + }); + + it('maps a Redirect payload to the path', () => { + expect(mapActionExecuteResult({ redirectTo: '/orders/1' })).toEqual({ + status: 200, + body: { type: 'redirect', path: '/orders/1' }, + }); + }); + + it('falls through to 501 for an unrecognized (File) payload', () => { + expect(mapActionExecuteResult({})).toEqual({ + status: 501, + body: { error: { type: 'unsupported_action_result', status: 501 } }, + }); + }); + + it('falls through to 501 for a non-object payload', () => { + expect(mapActionExecuteResult(null)).toEqual({ + status: 501, + body: { error: { type: 'unsupported_action_result', status: 501 } }, + }); + }); +}); diff --git a/packages/agent-bff/test/action/action-routes-middleware.test.ts b/packages/agent-bff/test/action/action-routes-middleware.test.ts index 4cf75cf2f5..54364f4879 100644 --- a/packages/agent-bff/test/action/action-routes-middleware.test.ts +++ b/packages/agent-bff/test/action/action-routes-middleware.test.ts @@ -1,8 +1,13 @@ -import type { ActionForm, AgentActionClient } from '../../src/action/agent-action-client'; +import type { Action, AgentActionClient } from '../../src/action/agent-action-client'; import type { Logger } from '../../src/ports/logger-port'; import type ReadModelStore from '../../src/read-model/read-model-store'; -import { AgentHttpError } from '@forestadmin/agent-client'; +import { + ActionFormValidationError, + ActionRequiresApprovalError, + AgentHttpError, + UnknownActionFieldError, +} from '@forestadmin/agent-client'; import { bodyParser } from '@koa/bodyparser'; import Koa from 'koa'; import request from 'supertest'; @@ -41,12 +46,14 @@ function makeAction({ skipped = [], postChange, execute = jest.fn(), + setFields = jest.fn(async () => {}), }: { fields?: FakeField[]; layout?: unknown[]; skipped?: string[]; postChange?: { fields?: FakeField[]; layout?: unknown[] }; execute?: jest.Mock; + setFields?: jest.Mock; } = {}) { const state = { fields: [...fields], layout: [...layout] }; const tryToSetFields = jest.fn(async () => { @@ -58,8 +65,9 @@ function makeAction({ return skipped; }); - const form: ActionForm & { tryToSetFields: jest.Mock; execute: jest.Mock } = { + const form: Action & { tryToSetFields: jest.Mock; execute: jest.Mock; setFields: jest.Mock } = { execute, + setFields, tryToSetFields, getFields: () => state.fields.map(f => ({ @@ -78,10 +86,10 @@ function makeAction({ } function clientOf( - form: ActionForm, - loadActionForm: jest.Mock = jest.fn(async () => form), -): AgentActionClient & { loadActionForm: jest.Mock } { - return { loadActionForm }; + form: Action, + loadAction: jest.Mock = jest.fn(async () => form), +): AgentActionClient & { loadAction: jest.Mock } { + return { loadAction }; } function buildApp( @@ -224,44 +232,44 @@ describe('action routes middleware', () => { it('passes an opaque composite recordId through unchanged', async () => { const form = makeAction(); - const loadActionForm = jest.fn(async () => form); - const app = buildApp(storeOf(readModel), clientOf(form, loadActionForm)); + const loadAction = jest.fn(async () => form); + const app = buildApp(storeOf(readModel), clientOf(form, loadAction)); await request(app.callback()) .post('/agent/v1/users/actions/approve/form') .send({ recordIds: ['1|2'] }); - expect(loadActionForm).toHaveBeenCalledWith('users', 'approve', ['1|2']); + expect(loadAction).toHaveBeenCalledWith('users', 'approve', ['1|2']); }); it('coerces a numeric zero recordId to a string so it survives the downstream filter', async () => { const form = makeAction(); - const loadActionForm = jest.fn(async () => form); - const app = buildApp(storeOf(readModel), clientOf(form, loadActionForm)); + const loadAction = jest.fn(async () => form); + const app = buildApp(storeOf(readModel), clientOf(form, loadAction)); await request(app.callback()) .post('/agent/v1/users/actions/approve/form') .send({ recordIds: [0] }); - expect(loadActionForm).toHaveBeenCalledWith('users', 'approve', ['0']); + expect(loadAction).toHaveBeenCalledWith('users', 'approve', ['0']); }); it('accepts an empty recordIds array for a global action', async () => { const form = makeAction(); - const loadActionForm = jest.fn(async () => form); - const app = buildApp(storeOf(readModel), clientOf(form, loadActionForm)); + const loadAction = jest.fn(async () => form); + const app = buildApp(storeOf(readModel), clientOf(form, loadAction)); const response = await request(app.callback()) .post('/agent/v1/users/actions/approve/form') .send({ recordIds: [] }); expect(response.status).toBe(200); - expect(loadActionForm).toHaveBeenCalledWith('users', 'approve', []); + expect(loadAction).toHaveBeenCalledWith('users', 'approve', []); }); it('returns 400 invalid_request with no agent call when recordIds is missing', async () => { - const loadActionForm = jest.fn(); - const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadActionForm)); + const loadAction = jest.fn(); + const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadAction)); const response = await request(app.callback()) .post('/agent/v1/users/actions/approve/form') @@ -269,36 +277,36 @@ describe('action routes middleware', () => { expect(response.status).toBe(400); expect(response.body.error).toMatchObject({ type: 'invalid_request', status: 400 }); - expect(loadActionForm).not.toHaveBeenCalled(); + expect(loadAction).not.toHaveBeenCalled(); }); it('returns 400 invalid_request when recordIds is not an array', async () => { - const loadActionForm = jest.fn(); - const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadActionForm)); + const loadAction = jest.fn(); + const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadAction)); const response = await request(app.callback()) .post('/agent/v1/users/actions/approve/form') .send({ recordIds: '42' }); expect(response.status).toBe(400); - expect(loadActionForm).not.toHaveBeenCalled(); + expect(loadAction).not.toHaveBeenCalled(); }); it('returns 400 invalid_request when values is not an object', async () => { - const loadActionForm = jest.fn(); - const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadActionForm)); + const loadAction = jest.fn(); + const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadAction)); const response = await request(app.callback()) .post('/agent/v1/users/actions/approve/form') .send({ recordIds: ['42'], values: 'nope' }); expect(response.status).toBe(400); - expect(loadActionForm).not.toHaveBeenCalled(); + expect(loadAction).not.toHaveBeenCalled(); }); it('returns 404 unknown_action for an action that is not exposed', async () => { - const loadActionForm = jest.fn(); - const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadActionForm)); + const loadAction = jest.fn(); + const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadAction)); const response = await request(app.callback()) .post('/agent/v1/users/actions/ghost/form') @@ -306,18 +314,18 @@ describe('action routes middleware', () => { expect(response.status).toBe(404); expect(response.body.error).toMatchObject({ type: 'unknown_action', status: 404 }); - expect(loadActionForm).not.toHaveBeenCalled(); + expect(loadAction).not.toHaveBeenCalled(); }); it('maps an agent failure on form load through the error contract', async () => { - const loadActionForm = jest.fn(async () => { + const loadAction = jest.fn(async () => { throw new AgentHttpError( 403, { errors: [{ status: 403, name: 'ForbiddenError' }] }, undefined, ); }); - const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadActionForm)); + const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadAction)); const response = await request(app.callback()) .post('/agent/v1/users/actions/approve/form') @@ -343,8 +351,8 @@ describe('action routes middleware', () => { }); it('returns 401 when no agent token is available', async () => { - const loadActionForm = jest.fn(); - const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadActionForm), { + const loadAction = jest.fn(); + const app = buildApp(storeOf(readModel), clientOf(makeAction(), loadAction), { agentToken: null, }); @@ -353,27 +361,27 @@ describe('action routes middleware', () => { .send({ recordIds: ['42'] }); expect(response.status).toBe(401); - expect(loadActionForm).not.toHaveBeenCalled(); + expect(loadAction).not.toHaveBeenCalled(); }); it('falls through to the next middleware for a non-POST request on the form path', async () => { - const loadActionForm = jest.fn(); - const app = buildAppWithTerminal(clientOf(makeAction(), loadActionForm)); + const loadAction = jest.fn(); + const app = buildAppWithTerminal(clientOf(makeAction(), loadAction)); const response = await request(app.callback()).get('/agent/v1/users/actions/approve/form'); expect(response.status).toBe(204); - expect(loadActionForm).not.toHaveBeenCalled(); + expect(loadAction).not.toHaveBeenCalled(); }); it('falls through to the next middleware for a path that is not an action form route', async () => { - const loadActionForm = jest.fn(); - const app = buildAppWithTerminal(clientOf(makeAction(), loadActionForm)); + const loadAction = jest.fn(); + const app = buildAppWithTerminal(clientOf(makeAction(), loadAction)); const response = await request(app.callback()).post('/agent/v1/users/list').send({}); expect(response.status).toBe(204); - expect(loadActionForm).not.toHaveBeenCalled(); + expect(loadAction).not.toHaveBeenCalled(); }); it('returns 503 schema_unavailable when the schema cannot be loaded', async () => { @@ -387,3 +395,187 @@ describe('action routes middleware', () => { expect(response.body.error).toMatchObject({ type: 'schema_unavailable', status: 503 }); }); }); + +describe('action execute', () => { + function execApp(client: AgentActionClient) { + return buildApp(storeOf(readModel), client); + } + + it('sets the submitted values then executes the action', async () => { + const setFields = jest.fn(async () => {}); + const execute = jest.fn(async () => ({ success: 'Done' })); + const form = makeAction({ setFields, execute }); + const loadAction = jest.fn(async () => form); + + await request(execApp(clientOf(form, loadAction)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'], values: { reason: 'x' } }); + + expect(loadAction).toHaveBeenCalledWith('users', 'approve', ['42']); + expect(setFields).toHaveBeenCalledWith({ reason: 'x' }); + expect(execute).toHaveBeenCalledTimes(1); + }); + + it('normalizes a Success result to 200 with invalidated as an array', async () => { + const form = makeAction({ + execute: jest.fn(async () => ({ + success: 'Refunded', + html: 'ok', + refresh: { relationships: ['orders'] }, + })), + }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ + type: 'success', + message: 'Refunded', + invalidated: ['orders'], + html: 'ok', + }); + }); + + it('normalizes a Webhook result to 200 passing its fields through verbatim', async () => { + const form = makeAction({ + execute: jest.fn(async () => ({ + webhook: { url: 'https://x.test', method: 'POST', headers: { a: '1' }, body: { b: 2 } }, + })), + }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ + type: 'webhook', + url: 'https://x.test', + method: 'POST', + headers: { a: '1' }, + body: { b: 2 }, + }); + }); + + it('normalizes a Redirect result to 200 with the path', async () => { + const form = makeAction({ execute: jest.fn(async () => ({ redirectTo: '/orders/1' })) }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ type: 'redirect', path: '/orders/1' }); + }); + + it('returns 501 unsupported_action_result for an unrecognized (File) result', async () => { + const form = makeAction({ execute: jest.fn(async () => ({})) }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(501); + expect(response.body).toEqual({ + error: { type: 'unsupported_action_result', status: 501 }, + }); + }); + + it('renders a native action Error as HTTP 400 with the forwarded html', async () => { + const form = makeAction({ + execute: jest.fn(async () => { + throw new ActionFormValidationError('Refund failed', 'Nope'); + }), + }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ + type: 'error', + status: 400, + message: 'Refund failed', + html: 'Nope', + }); + }); + + it('rejects an unknown submitted field as 400 invalid_request', async () => { + const setFields = jest.fn(async () => { + throw new UnknownActionFieldError('ghost'); + }); + const execute = jest.fn(); + const form = makeAction({ setFields, execute }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'], values: { ghost: 1 } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatchObject({ type: 'invalid_request', status: 400 }); + expect(execute).not.toHaveBeenCalled(); + }); + + it('maps an approval-gated action to 403 action_requires_approval with the allowed roles', async () => { + const form = makeAction({ + execute: jest.fn(async () => { + throw new ActionRequiresApprovalError('Needs approval', [7, 9]); + }), + }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(403); + expect(response.body.error).toMatchObject({ + type: 'action_requires_approval', + status: 403, + details: { roleIdsAllowedToApprove: [7, 9] }, + }); + }); + + it('maps a transport 5xx to agent_unavailable, distinct from the action-Error 400', async () => { + const form = makeAction({ + execute: jest.fn(async () => { + throw new AgentHttpError(502, null); + }), + }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(503); + expect(response.body.error).toMatchObject({ type: 'agent_unavailable', status: 503 }); + }); + + it('returns 400 invalid_request with no agent call when recordIds is missing', async () => { + const loadAction = jest.fn(); + const form = makeAction(); + + const response = await request(execApp(clientOf(form, loadAction)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ values: {} }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatchObject({ type: 'invalid_request', status: 400 }); + expect(loadAction).not.toHaveBeenCalled(); + }); + + it('returns 404 unknown_action for an action that is not exposed', async () => { + const loadAction = jest.fn(); + const form = makeAction(); + + const response = await request(execApp(clientOf(form, loadAction)).callback()) + .post('/agent/v1/users/actions/ghost/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(404); + expect(response.body.error).toMatchObject({ type: 'unknown_action', status: 404 }); + expect(loadAction).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/agent-bff/test/action/agent-action-client.test.ts b/packages/agent-bff/test/action/agent-action-client.test.ts index e5aaee8bfc..c6fd64574b 100644 --- a/packages/agent-bff/test/action/agent-action-client.test.ts +++ b/packages/agent-bff/test/action/agent-action-client.test.ts @@ -7,7 +7,7 @@ jest.mock('@forestadmin/agent-client', () => ({ createRemoteAgentClient: jest.fn const createRemoteAgentClientMock = createRemoteAgentClient as jest.Mock; describe('createAgentActionClient', () => { - it('loads the form via createRemoteAgentClient().collection(name).action(name, { recordIds })', async () => { + it('loads the action via createRemoteAgentClient().collection(name).action(name, { recordIds })', async () => { const loadedAction = { tag: 'action' }; const actionFn = jest.fn(async () => loadedAction); const collectionFn = jest.fn(() => ({ action: actionFn })); @@ -19,7 +19,7 @@ describe('createAgentActionClient', () => { token: 'agent-jwt', actionEndpoints, }); - const result = await client.loadActionForm('users', 'approve', ['1', '2']); + const result = await client.loadAction('users', 'approve', ['1', '2']); expect(createRemoteAgentClientMock).toHaveBeenCalledWith({ url: 'https://agent.example.com', From c4f48d50aeae93e952c17bccf594aa0694657ef8 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Fri, 10 Jul 2026 15:46:00 +0200 Subject: [PATCH 3/6] fix(agent-bff): harden execute result mapping per macroscope review Validate value shapes (not just key presence) when normalizing the execute result so malformed payloads fall through to 501 instead of a null-field 200, and keep an empty roleIdsAllowedToApprove array in the approval 403 details. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/action/action-execute-mapper.ts | 23 +++++++++++-------- .../src/action/action-routes-middleware.ts | 2 +- .../test/action/action-execute-mapper.test.ts | 13 ++++++++++- .../action/action-routes-middleware.test.ts | 15 ++++++++++++ 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/packages/agent-bff/src/action/action-execute-mapper.ts b/packages/agent-bff/src/action/action-execute-mapper.ts index d8dd7116fc..a0b7c17f45 100644 --- a/packages/agent-bff/src/action/action-execute-mapper.ts +++ b/packages/agent-bff/src/action/action-execute-mapper.ts @@ -40,10 +40,11 @@ export interface ActionExecuteMapped { export function mapActionExecuteResult(raw: unknown): ActionExecuteMapped { const body = (typeof raw === 'object' && raw !== null ? raw : {}) as Record; - if ('webhook' in body) { - const hook = ( - typeof body.webhook === 'object' && body.webhook !== null ? body.webhook : {} - ) as Record; + // Each branch validates the value shape, not just key presence: a malformed payload + // (`{ webhook: null }`, `{ redirectTo: {} }`, `{ success: {} }`) must fall through to the 501 + // path rather than be surfaced as a 200 with null fields the client cannot tell from a real one. + if (typeof body.webhook === 'object' && body.webhook !== null) { + const hook = body.webhook as Record; return { status: 200, @@ -57,21 +58,23 @@ export function mapActionExecuteResult(raw: unknown): ActionExecuteMapped { }; } - if ('redirectTo' in body) { + if (typeof body.redirectTo === 'string') { return { status: 200, body: { type: 'redirect', path: body.redirectTo } }; } - if ('success' in body || 'refresh' in body) { - const refresh = ( - typeof body.refresh === 'object' && body.refresh !== null ? body.refresh : {} - ) as { relationships?: unknown }; + const refresh = typeof body.refresh === 'object' && body.refresh !== null ? body.refresh : null; + + // A Success carries a string message and/or a refresh object; the agent always sends the refresh, + // so a bare refresh (message absent) still normalizes to a success with a null message. + if (typeof body.success === 'string' || refresh !== null) { + const relationships = (refresh as { relationships?: unknown } | null)?.relationships; return { status: 200, body: { type: 'success', message: typeof body.success === 'string' ? body.success : null, - invalidated: Array.isArray(refresh.relationships) ? (refresh.relationships as string[]) : [], + invalidated: Array.isArray(relationships) ? (relationships as string[]) : [], html: typeof body.html === 'string' ? body.html : null, }, }; diff --git a/packages/agent-bff/src/action/action-routes-middleware.ts b/packages/agent-bff/src/action/action-routes-middleware.ts index 956682b852..0a9f90650b 100644 --- a/packages/agent-bff/src/action/action-routes-middleware.ts +++ b/packages/agent-bff/src/action/action-routes-middleware.ts @@ -108,7 +108,7 @@ async function handleExecute( if (error instanceof ActionRequiresApprovalError) { throw actionRequiresApproval( error.message, - error.roleIdsAllowedToApprove + error.roleIdsAllowedToApprove !== undefined ? { roleIdsAllowedToApprove: error.roleIdsAllowedToApprove } : undefined, ); diff --git a/packages/agent-bff/test/action/action-execute-mapper.test.ts b/packages/agent-bff/test/action/action-execute-mapper.test.ts index 503c6c5464..2dc26b8ad0 100644 --- a/packages/agent-bff/test/action/action-execute-mapper.test.ts +++ b/packages/agent-bff/test/action/action-execute-mapper.test.ts @@ -15,7 +15,7 @@ describe('mapActionExecuteResult', () => { }); it('defaults message and html to null and invalidated to [] when absent', () => { - expect(mapActionExecuteResult({ success: undefined })).toEqual({ + expect(mapActionExecuteResult({ success: undefined, refresh: { relationships: [] } })).toEqual({ status: 200, body: { type: 'success', message: null, invalidated: [], html: null }, }); @@ -65,4 +65,15 @@ describe('mapActionExecuteResult', () => { body: { error: { type: 'unsupported_action_result', status: 501 } }, }); }); + + it.each([ + ['a null webhook', { webhook: null }], + ['a non-string redirectTo', { redirectTo: {} }], + ['a non-string success with no refresh', { success: {} }], + ])('falls through to 501 for a malformed payload: %s', (_label, payload) => { + expect(mapActionExecuteResult(payload)).toEqual({ + status: 501, + body: { error: { type: 'unsupported_action_result', status: 501 } }, + }); + }); }); diff --git a/packages/agent-bff/test/action/action-routes-middleware.test.ts b/packages/agent-bff/test/action/action-routes-middleware.test.ts index 54364f4879..e4e53ffc01 100644 --- a/packages/agent-bff/test/action/action-routes-middleware.test.ts +++ b/packages/agent-bff/test/action/action-routes-middleware.test.ts @@ -538,6 +538,21 @@ describe('action execute', () => { }); }); + it('keeps an empty roleIdsAllowedToApprove array in the 403 details', async () => { + const form = makeAction({ + execute: jest.fn(async () => { + throw new ActionRequiresApprovalError('Needs approval', []); + }), + }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(403); + expect(response.body.error.details).toEqual({ roleIdsAllowedToApprove: [] }); + }); + it('maps a transport 5xx to agent_unavailable, distinct from the action-Error 400', async () => { const form = makeAction({ execute: jest.fn(async () => { From 8151ba3c78ca6864beac6e811d8dbc9e65d714f7 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Wed, 15 Jul 2026 10:31:40 +0200 Subject: [PATCH 4/6] fix: lint --- .../agent-bff/src/action/action-routes-middleware.ts | 10 ++++++++-- .../test/action/action-execute-mapper.test.ts | 7 ++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/agent-bff/src/action/action-routes-middleware.ts b/packages/agent-bff/src/action/action-routes-middleware.ts index 0a9f90650b..2aa8525d69 100644 --- a/packages/agent-bff/src/action/action-routes-middleware.ts +++ b/packages/agent-bff/src/action/action-routes-middleware.ts @@ -70,7 +70,10 @@ async function handleForm( // Each agent-hitting call is wrapped on its own; getFields/extractRawLayout/mapping stay outside // callAgent so a local BFF bug surfaces as a 500, not a mislabelled agent error. Fields and // layout are read AFTER tryToSetFields because a change hook rebuilds them in place. - const action = await callAgent(() => client.loadAction(collection, actionName, recordIds), logger); + const action = await callAgent( + () => client.loadAction(collection, actionName, recordIds), + logger, + ); const skippedFields = await callAgent(() => action.tryToSetFields(values), logger); ctx.status = 200; @@ -86,7 +89,10 @@ async function handleExecute( values: Record, logger: Logger, ): Promise { - const action = await callAgent(() => client.loadAction(collection, actionName, recordIds), logger); + const action = await callAgent( + () => client.loadAction(collection, actionName, recordIds), + logger, + ); // setFields is strict: an unknown submitted field is a client error (400), not a 500. A transport // failure from the change-hook it triggers is a genuine agent error, so it goes to the mapper. diff --git a/packages/agent-bff/test/action/action-execute-mapper.test.ts b/packages/agent-bff/test/action/action-execute-mapper.test.ts index 2dc26b8ad0..ff92b8d983 100644 --- a/packages/agent-bff/test/action/action-execute-mapper.test.ts +++ b/packages/agent-bff/test/action/action-execute-mapper.test.ts @@ -10,7 +10,12 @@ describe('mapActionExecuteResult', () => { }), ).toEqual({ status: 200, - body: { type: 'success', message: 'Done', invalidated: ['orders', 'items'], html: 'ok' }, + body: { + type: 'success', + message: 'Done', + invalidated: ['orders', 'items'], + html: 'ok', + }, }); }); From 308d8619a715661454a8e3893e2476992db467ab Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Wed, 15 Jul 2026 11:39:33 +0200 Subject: [PATCH 5/6] test(agent-bff): cover execute error-routing branches and log the 501 fall-through Add the setFields agent-error fallback, approval-without-roles, error-body html-absent, and non-array refresh.relationships cases; log the unrecognized payload keys when execute maps to 501; correct the mapper rationale comment. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/action/action-execute-mapper.ts | 8 ++-- .../src/action/action-routes-middleware.ts | 9 ++++ .../test/action/action-execute-mapper.test.ts | 10 ++++ .../action/action-routes-middleware.test.ts | 47 +++++++++++++++++++ 4 files changed, 70 insertions(+), 4 deletions(-) diff --git a/packages/agent-bff/src/action/action-execute-mapper.ts b/packages/agent-bff/src/action/action-execute-mapper.ts index a0b7c17f45..35979c7f4a 100644 --- a/packages/agent-bff/src/action/action-execute-mapper.ts +++ b/packages/agent-bff/src/action/action-execute-mapper.ts @@ -33,10 +33,10 @@ export interface ActionExecuteMapped { body: ActionExecuteMappedBody; } -// Normalizes the agent's 200 execute payload into the flat BFF wrapper. The native ActionResult -// union never reaches the BFF (agent-client `execute()` is typed `{ success, html? }`), so we -// discriminate on the agent HTTP payload shape. A File result streams a binary with no JSON marker, -// so any unrecognized 200 body falls through to a structured 501 rather than being mislabelled. +// Normalizes the agent's 200 execute payload into the flat BFF wrapper. The execute result is +// untyped at the BFF boundary (`Action.execute(): Promise`), so we discriminate on the +// agent HTTP payload shape. A File result streams a binary with no JSON marker, so any unrecognized +// 200 body falls through to a structured 501 rather than being mislabelled. export function mapActionExecuteResult(raw: unknown): ActionExecuteMapped { const body = (typeof raw === 'object' && raw !== null ? raw : {}) as Record; diff --git a/packages/agent-bff/src/action/action-routes-middleware.ts b/packages/agent-bff/src/action/action-routes-middleware.ts index 2aa8525d69..b0294e16c2 100644 --- a/packages/agent-bff/src/action/action-routes-middleware.ts +++ b/packages/agent-bff/src/action/action-routes-middleware.ts @@ -131,6 +131,15 @@ async function handleExecute( } const { status, body } = mapActionExecuteResult(raw); + + // An unrecognized payload (a File stream, or a new agent result type) maps to a generic 501 with + // no trace of what it was; log the payload keys so the case can be diagnosed without the body. + if (status === 501) { + logger('Warn', 'Unrecognized action execute result mapped to 501', { + keys: typeof raw === 'object' && raw !== null ? Object.keys(raw) : typeof raw, + }); + } + ctx.status = status; ctx.body = body; } diff --git a/packages/agent-bff/test/action/action-execute-mapper.test.ts b/packages/agent-bff/test/action/action-execute-mapper.test.ts index ff92b8d983..016aefe5d3 100644 --- a/packages/agent-bff/test/action/action-execute-mapper.test.ts +++ b/packages/agent-bff/test/action/action-execute-mapper.test.ts @@ -33,6 +33,16 @@ describe('mapActionExecuteResult', () => { }); }); + it.each([ + ['relationships absent', { success: 'ok', refresh: {} }], + ['relationships not an array', { success: 'ok', refresh: { relationships: 'x' } }], + ])('falls back invalidated to [] when %s', (_label, payload) => { + expect(mapActionExecuteResult(payload)).toEqual({ + status: 200, + body: { type: 'success', message: 'ok', invalidated: [], html: null }, + }); + }); + it('maps a Webhook payload verbatim', () => { expect( mapActionExecuteResult({ diff --git a/packages/agent-bff/test/action/action-routes-middleware.test.ts b/packages/agent-bff/test/action/action-routes-middleware.test.ts index e4e53ffc01..011c5e1622 100644 --- a/packages/agent-bff/test/action/action-routes-middleware.test.ts +++ b/packages/agent-bff/test/action/action-routes-middleware.test.ts @@ -503,6 +503,21 @@ describe('action execute', () => { }); }); + it('defaults html to null on the error body when the action Error carries none', async () => { + const form = makeAction({ + execute: jest.fn(async () => { + throw new ActionFormValidationError('Refund failed'); + }), + }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ type: 'error', status: 400, message: 'Refund failed', html: null }); + }); + it('rejects an unknown submitted field as 400 invalid_request', async () => { const setFields = jest.fn(async () => { throw new UnknownActionFieldError('ghost'); @@ -519,6 +534,22 @@ describe('action execute', () => { expect(execute).not.toHaveBeenCalled(); }); + it('maps a non-unknown-field setFields rejection through the agent error contract', async () => { + const setFields = jest.fn(async () => { + throw new AgentHttpError(422, { errors: [{ status: 422, name: 'UnprocessableError' }] }); + }); + const execute = jest.fn(); + const form = makeAction({ setFields, execute }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'], values: { reason: 'x' } }); + + expect(response.status).toBe(422); + expect(response.body.error).toMatchObject({ type: 'unprocessable_entity', status: 422 }); + expect(execute).not.toHaveBeenCalled(); + }); + it('maps an approval-gated action to 403 action_requires_approval with the allowed roles', async () => { const form = makeAction({ execute: jest.fn(async () => { @@ -553,6 +584,22 @@ describe('action execute', () => { expect(response.body.error.details).toEqual({ roleIdsAllowedToApprove: [] }); }); + it('omits details when the approval error carries no roleIdsAllowedToApprove', async () => { + const form = makeAction({ + execute: jest.fn(async () => { + throw new ActionRequiresApprovalError('Needs approval'); + }), + }); + + const response = await request(execApp(clientOf(form)).callback()) + .post('/agent/v1/users/actions/approve/execute') + .send({ recordIds: ['42'] }); + + expect(response.status).toBe(403); + expect(response.body.error).toMatchObject({ type: 'action_requires_approval', status: 403 }); + expect(response.body.error.details).toBeUndefined(); + }); + it('maps a transport 5xx to agent_unavailable, distinct from the action-Error 400', async () => { const form = makeAction({ execute: jest.fn(async () => { From 676326069e2b09ccbf6e7ad7328475ee4ab52628 Mon Sep 17 00:00:00 2001 From: Nicolas Bouliol Date: Wed, 15 Jul 2026 11:48:52 +0200 Subject: [PATCH 6/6] style: apply prettier formatting to action execute changes Co-Authored-By: Claude Opus 4.8 (1M context) --- .../agent-bff/test/action/action-routes-middleware.test.ts | 7 ++++++- packages/agent-client/src/domains/action.ts | 5 ++++- packages/agent-client/src/errors.ts | 5 +---- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/agent-bff/test/action/action-routes-middleware.test.ts b/packages/agent-bff/test/action/action-routes-middleware.test.ts index 011c5e1622..bc72df3d8d 100644 --- a/packages/agent-bff/test/action/action-routes-middleware.test.ts +++ b/packages/agent-bff/test/action/action-routes-middleware.test.ts @@ -515,7 +515,12 @@ describe('action execute', () => { .send({ recordIds: ['42'] }); expect(response.status).toBe(400); - expect(response.body).toEqual({ type: 'error', status: 400, message: 'Refund failed', html: null }); + expect(response.body).toEqual({ + type: 'error', + status: 400, + message: 'Refund failed', + html: null, + }); }); it('rejects an unknown submitted field as 400 invalid_request', async () => { diff --git a/packages/agent-client/src/domains/action.ts b/packages/agent-client/src/domains/action.ts index bf9b042a4e..7e95cbf253 100644 --- a/packages/agent-client/src/domains/action.ts +++ b/packages/agent-client/src/domains/action.ts @@ -74,7 +74,10 @@ function toActionError(error: unknown): unknown { } if (error.status === 400 || error.status === 422) { - return new ActionFormValidationError(detail ?? 'The action form values were rejected.', body.html); + return new ActionFormValidationError( + detail ?? 'The action form values were rejected.', + body.html, + ); } return error; diff --git a/packages/agent-client/src/errors.ts b/packages/agent-client/src/errors.ts index c824c0cab0..53db583c35 100644 --- a/packages/agent-client/src/errors.ts +++ b/packages/agent-client/src/errors.ts @@ -19,10 +19,7 @@ export class ActionRequiresApprovalError extends Error { } export class ActionFormValidationError extends Error { - constructor( - message: string, - readonly html?: string, - ) { + constructor(message: string, readonly html?: string) { super(message); this.name = 'ActionFormValidationError'; }