diff --git a/packages/agent-testing/src/forest-admin-client-mock.ts b/packages/agent-testing/src/forest-admin-client-mock.ts index 5bd4cb6342..508bc4e4c6 100644 --- a/packages/agent-testing/src/forest-admin-client-mock.ts +++ b/packages/agent-testing/src/forest-admin-client-mock.ts @@ -62,6 +62,35 @@ export default class ForestAdminClientMock implements ForestAdminClient { updateActivityLogStatus: () => Promise.resolve(), }; + readonly workflowsService: ForestAdminClient['workflowsService'] = { + listMcpEnabledWorkflows: () => Promise.resolve([]), + getMcpWorkflowById: () => + Promise.resolve({ + workflowId: 'wf-1', + name: 'Refund order', + collectionName: 'orders', + mcpEnabled: true, + }), + triggerMcpWorkflow: () => Promise.resolve({ runId: '1', runState: 'loading' }), + getMcpWorkflowRun: () => + Promise.resolve({ + id: 1, + userId: 1, + renderingId: 1, + collectionId: 'orders', + workflowId: 'wf-1', + bpmnVersion: '1', + selectedRecordId: '1', + runState: 'loading', + engine: 'orchestrator', + triggerType: 'mcp', + lockedAt: null, + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + workflowHistory: [], + }), + }; + readonly permissionService: any; readonly authService: any; diff --git a/packages/agent-testing/test/forest-admin-client-mock.test.ts b/packages/agent-testing/test/forest-admin-client-mock.test.ts new file mode 100644 index 0000000000..d165d99ff8 --- /dev/null +++ b/packages/agent-testing/test/forest-admin-client-mock.test.ts @@ -0,0 +1,66 @@ +import ForestAdminClientMock from '../src/forest-admin-client-mock'; + +describe('ForestAdminClientMock', () => { + describe('workflowsService', () => { + it('should resolve an empty list of MCP-enabled workflows', async () => { + const client = new ForestAdminClientMock(); + + await expect( + client.workflowsService.listMcpEnabledWorkflows({ + forestServerToken: 'token', + renderingId: '1', + }), + ).resolves.toEqual([]); + }); + + it('should resolve an mcp-enabled workflow when fetching one by id', async () => { + const client = new ForestAdminClientMock(); + + await expect( + client.workflowsService.getMcpWorkflowById({ + forestServerToken: 'token', + renderingId: '1', + workflowId: 'wf-1', + }), + ).resolves.toEqual( + expect.objectContaining({ + workflowId: 'wf-1', + mcpEnabled: true, + }), + ); + }); + + it('should resolve a loading run when triggering a workflow', async () => { + const client = new ForestAdminClientMock(); + + await expect( + client.workflowsService.triggerMcpWorkflow({ + forestServerToken: 'token', + renderingId: '1', + workflowId: 'wf-1', + recordId: '42', + }), + ).resolves.toEqual({ runId: '1', runState: 'loading' }); + }); + + it('should resolve a loading hydrated run when fetching a workflow run', async () => { + const client = new ForestAdminClientMock(); + + await expect( + client.workflowsService.getMcpWorkflowRun({ + forestServerToken: 'token', + renderingId: '1', + runId: '1', + }), + ).resolves.toEqual( + expect.objectContaining({ + id: 1, + runState: 'loading', + engine: 'orchestrator', + triggerType: 'mcp', + workflowHistory: [], + }), + ); + }); + }); +}); diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index f0f9070ac7..44b92ff6d6 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -390,6 +390,7 @@ export default class Agent extends FrameworkMounter const forestServerClient = new ForestServerClientImpl( this.options.forestAdminClient.schemaService, this.options.forestAdminClient.activityLogsService, + this.options.forestAdminClient.workflowsService, this.options.forestServerUrl, ); diff --git a/packages/agent/test/__factories__/forest-admin-client.ts b/packages/agent/test/__factories__/forest-admin-client.ts index ab7189ccc8..c1097fb551 100644 --- a/packages/agent/test/__factories__/forest-admin-client.ts +++ b/packages/agent/test/__factories__/forest-admin-client.ts @@ -54,6 +54,12 @@ const forestAdminClientFactory = ForestAdminClientFactory.define(() => ({ createMcpActivityLog: jest.fn(), updateActivityLogStatus: jest.fn(), }, + workflowsService: { + listMcpEnabledWorkflows: jest.fn(), + getMcpWorkflowById: jest.fn(), + triggerMcpWorkflow: jest.fn(), + getMcpWorkflowRun: jest.fn(), + }, subscribeToServerEvents: jest.fn(), close: jest.fn(), onRefreshCustomizations: jest.fn(), diff --git a/packages/forestadmin-client/src/build-application-services.ts b/packages/forestadmin-client/src/build-application-services.ts index 78158f8668..dbecac4085 100644 --- a/packages/forestadmin-client/src/build-application-services.ts +++ b/packages/forestadmin-client/src/build-application-services.ts @@ -22,6 +22,7 @@ import UserPermissionService from './permissions/user-permission'; import SchemaService from './schema'; import ContextVariablesInstantiator from './utils/context-variables-instantiator'; import defaultLogger from './utils/default-logger'; +import WorkflowsService from './workflows'; export default function buildApplicationServices( forestAdminServerInterface: ForestAdminServerInterface, @@ -31,6 +32,7 @@ export default function buildApplicationServices( renderingPermission: RenderingPermissionService; schema: SchemaService; activityLogs: ActivityLogsService; + workflows: WorkflowsService; contextVariables: ContextVariablesInstantiator; ipWhitelist: IpWhiteListService; permission: PermissionService; @@ -89,6 +91,7 @@ export default function buildApplicationServices( ipWhitelist: new IpWhiteListService(forestAdminServerInterface, optionsWithDefaults), schema: new SchemaService(forestAdminServerInterface, optionsWithDefaults), activityLogs: new ActivityLogsService(forestAdminServerInterface, optionsWithDefaults), + workflows: new WorkflowsService(forestAdminServerInterface, optionsWithDefaults), auth: forestAdminServerInterface.makeAuthService(optionsWithDefaults), modelCustomizationService: new ModelCustomizationFromApiService( forestAdminServerInterface, diff --git a/packages/forestadmin-client/src/forest-admin-client-with-cache.ts b/packages/forestadmin-client/src/forest-admin-client-with-cache.ts index 198ade6471..a2a0c65526 100644 --- a/packages/forestadmin-client/src/forest-admin-client-with-cache.ts +++ b/packages/forestadmin-client/src/forest-admin-client-with-cache.ts @@ -19,6 +19,7 @@ import type { PermissionService, } from './types'; import type ContextVariablesInstantiator from './utils/context-variables-instantiator'; +import type WorkflowsService from './workflows'; import verifyAndExtractApproval from './permissions/verify-approval'; @@ -37,6 +38,7 @@ export default class ForestAdminClientWithCache implements ForestAdminClient { public readonly mcpServerConfigService: McpServerConfigService, protected readonly eventsSubscription: BaseEventsSubscriptionService, protected readonly eventsHandler: RefreshEventsHandlerService, + public readonly workflowsService: WorkflowsService, ) {} verifySignedActionParameters(signedParameters: string): TSignedParameters { diff --git a/packages/forestadmin-client/src/index.ts b/packages/forestadmin-client/src/index.ts index 27ed0240a3..6229b4cba2 100644 --- a/packages/forestadmin-client/src/index.ts +++ b/packages/forestadmin-client/src/index.ts @@ -28,8 +28,26 @@ export { ActivityLogType, CreateActivityLogParams, UpdateActivityLogStatusParams, + McpWorkflow, + McpWorkflowLookup, + ListMcpWorkflowsParams, + GetMcpWorkflowByIdParams, + TriggerMcpWorkflowParams, + GetMcpWorkflowRunParams, + WorkflowRunState, + WorkflowRunEngine, + WorkflowRunTriggerType, + WorkflowStepType, + WorkflowTaskType, + WorkflowStepOutgoing, + WorkflowStepDefinition, + WorkflowHistoryStepContext, + WorkflowHistoryStep, + HydratedWorkflowRun, + WorkflowRunTriggerResult, // Service interfaces for MCP ActivityLogsServiceInterface, + WorkflowsServiceInterface, SchemaServiceInterface, } from './types'; export { IpWhitelistConfiguration } from './ip-whitelist/types'; @@ -66,6 +84,7 @@ export default function createForestAdminClient( ipWhitelist, schema, activityLogs, + workflows, auth, modelCustomizationService, mcpServerConfigService, @@ -87,6 +106,7 @@ export default function createForestAdminClient( mcpServerConfigService, eventsSubscription, eventsHandler, + workflows, ); } @@ -105,6 +125,7 @@ export { default as ServerUtils } from './utils/server'; // export is necessary for the agent-generator package export { default as SchemaService, SchemaServiceOptions } from './schema'; export { default as ActivityLogsService, ActivityLogsOptions } from './activity-logs'; +export { default as WorkflowsService, WorkflowsServiceOptions } from './workflows'; export * from './auth/errors'; export * from './utils/errors'; diff --git a/packages/forestadmin-client/src/permissions/forest-http-api.ts b/packages/forestadmin-client/src/permissions/forest-http-api.ts index 4c3da9a211..4193e4a788 100644 --- a/packages/forestadmin-client/src/permissions/forest-http-api.ts +++ b/packages/forestadmin-client/src/permissions/forest-http-api.ts @@ -8,7 +8,12 @@ import type { ForestAdminClientOptions, ForestAdminServerInterface, ForestSchemaCollection, + HydratedWorkflowRun, IpWhitelistRulesResponse, + McpWorkflow, + McpWorkflowLookup, + WorkflowHistoryStepContext, + WorkflowRunTriggerResult, } from '../types'; import type { HttpOptions } from '../utils/http-options'; @@ -17,6 +22,74 @@ import JSONAPISerializer from 'json-api-serializer'; import AuthService from '../auth'; import ServerUtils from '../utils/server'; +/** + * These routes exist only for the MCP transport, so the source header belongs to the call rather + * than to the caller's configuration: the embedded `mountAiMcpServer` path builds its services from + * the shared client options, which carry no headers, and was silently sending MCP traffic + * unlabelled. Callers can still override it. + */ +const MCP_SOURCE_HEADER = { 'Forest-Application-Source': 'MCP' } as const; + +/** + * Projects the run onto the declared contract instead of forwarding the response as-is. The + * getWorkflowRun tool stringifies this straight into an LLM's context, and the orchestrator builds + * a second, executor-facing shape of the same run that carries a `userProfile` with a live Forest + * `serverToken`. The MCP route uses a different builder today, but the two return types are + * assignable to one another, so nothing structural keeps them apart — this whitelist is the + * guardrail rather than the type annotation. + * + * `stepDefinition` is passed through whole: its task-type-specific fields are the organisation's own + * workflow configuration, deliberately surfaced so the model can reason about the step. Per-step + * `context` is not — it is a closed interface here but an open bag server-side, so forwarding it + * whole would let a future orchestrator field reach the model with nothing to catch it. + */ +function projectStepContext( + context: WorkflowHistoryStepContext | undefined, +): WorkflowHistoryStepContext | undefined { + if (!context) return context; + + return { + manuallyCompleted: context.manuallyCompleted, + completedBy: context.completedBy, + subWorkflowVersion: context.subWorkflowVersion, + selectedOption: context.selectedOption, + error: context.error, + childrenWorkflowId: context.childrenWorkflowId, + escalationState: context.escalationState, + awaitingInputReason: context.awaitingInputReason, + }; +} + +function projectHydratedRun(run: HydratedWorkflowRun): HydratedWorkflowRun { + return { + id: run.id, + userId: run.userId, + renderingId: run.renderingId, + collectionId: run.collectionId, + workflowId: run.workflowId, + bpmnVersion: run.bpmnVersion, + selectedRecordId: run.selectedRecordId, + runState: run.runState, + engine: run.engine, + triggerType: run.triggerType, + lockedAt: run.lockedAt, + createdAt: run.createdAt, + updatedAt: run.updatedAt, + workflowHistory: (run.workflowHistory ?? []).map(step => ({ + stepName: step.stepName, + stepIndex: step.stepIndex, + originalStepIndex: step.originalStepIndex, + done: step.done, + revised: step.revised, + cancelled: step.cancelled, + childrenWorkflowId: step.childrenWorkflowId, + isCardStep: step.isCardStep, + context: projectStepContext(step.context), + stepDefinition: step.stepDefinition, + })), + }; +} + export default class ForestHttpApi implements ForestAdminServerInterface { async getEnvironmentPermissions(options: HttpOptions): Promise { return ServerUtils.query(options, 'get', '/liana/v4/permissions/environment'); @@ -130,7 +203,7 @@ export default class ForestHttpApi implements ForestAdminServerInterface { path: '/api/activity-logs-requests/mcp', bearerToken: options.bearerToken, body, - headers: options.headers, + headers: { ...MCP_SOURCE_HEADER, ...options.headers }, }); return activityLog; @@ -151,4 +224,91 @@ export default class ForestHttpApi implements ForestAdminServerInterface { headers: options.headers, }); } + + async listMcpEnabledWorkflows( + options: ActivityLogHttpOptions, + renderingId: string, + collectionName?: string, + ): Promise { + const query = collectionName ? `?collectionName=${encodeURIComponent(collectionName)}` : ''; + + const workflows = await ServerUtils.queryWithBearerToken({ + forestServerUrl: options.forestServerUrl, + method: 'get', + path: `/api/workflow-orchestrator/mcp-workflows${query}`, + bearerToken: options.bearerToken, + headers: { 'forest-rendering-id': renderingId, ...MCP_SOURCE_HEADER, ...options.headers }, + }); + + // Projected for the same reason as the run: the listWorkflows tool stringifies this array + // straight into an LLM's context, so a column added to the server query must not reach a + // model's prompt without a change here. + return (workflows ?? []).map(workflow => ({ + workflowId: workflow.workflowId, + name: workflow.name, + collectionName: workflow.collectionName, + })); + } + + async getMcpWorkflowById( + options: ActivityLogHttpOptions, + renderingId: string, + workflowId: string, + ): Promise { + const workflow = await ServerUtils.queryWithBearerToken({ + forestServerUrl: options.forestServerUrl, + method: 'get', + path: `/api/workflow-orchestrator/mcp-workflows/${encodeURIComponent(workflowId)}`, + bearerToken: options.bearerToken, + headers: { 'forest-rendering-id': renderingId, ...MCP_SOURCE_HEADER, ...options.headers }, + }); + + // Projected like the other three MCP routes. This payload does not reach a model directly, but + // `name` is written verbatim into a persisted Activity Log label, and the type is an unvalidated + // cast of the HTTP response — so a column added server-side would otherwise arrive unannounced. + return { + workflowId: workflow.workflowId, + name: workflow.name, + collectionName: workflow.collectionName, + mcpEnabled: workflow.mcpEnabled, + }; + } + + async triggerMcpWorkflow( + options: ActivityLogHttpOptions, + renderingId: string, + workflowId: string, + recordId: string, + ): Promise { + // The orchestrator returns a numeric runId; normalize it to the string form expected by + // getMcpWorkflowRun. + const result = await ServerUtils.queryWithBearerToken< + Omit & { runId: number | string } + >({ + forestServerUrl: options.forestServerUrl, + method: 'post', + path: `/api/workflow-orchestrator/mcp-workflows/${encodeURIComponent(workflowId)}/start`, + bearerToken: options.bearerToken, + body: { recordId }, + headers: { 'forest-rendering-id': renderingId, ...MCP_SOURCE_HEADER, ...options.headers }, + }); + + return { runId: String(result.runId), runState: result.runState }; + } + + async getMcpWorkflowRun( + options: ActivityLogHttpOptions, + renderingId: string, + runId: string, + ): Promise { + const run = await ServerUtils.queryWithBearerToken({ + forestServerUrl: options.forestServerUrl, + method: 'get', + path: `/api/workflow-orchestrator/mcp-workflows/runs/${encodeURIComponent(runId)}`, + bearerToken: options.bearerToken, + headers: { 'forest-rendering-id': renderingId, ...MCP_SOURCE_HEADER, ...options.headers }, + }); + + return projectHydratedRun(run); + } } diff --git a/packages/forestadmin-client/src/types.ts b/packages/forestadmin-client/src/types.ts index 5a2d83b664..018c04ec3c 100644 --- a/packages/forestadmin-client/src/types.ts +++ b/packages/forestadmin-client/src/types.ts @@ -53,6 +53,7 @@ export interface ForestAdminClient { readonly authService: ForestAdminAuthServiceInterface; readonly schemaService: SchemaServiceInterface; readonly activityLogsService: ActivityLogsServiceInterface; + readonly workflowsService: WorkflowsServiceInterface; verifySignedActionParameters(signedParameters: string): TSignedParameters; @@ -252,7 +253,8 @@ export type ActivityLogAction = | 'update' | 'delete' | 'listRelatedData' - | 'describeCollection'; + | 'describeCollection' + | 'triggerWorkflow'; export type ActivityLogType = 'read' | 'write'; @@ -282,6 +284,177 @@ export interface ActivityLogsServiceInterface { updateActivityLogStatus: (params: UpdateActivityLogStatusParams) => Promise; } +/** + * An MCP-enabled workflow, as returned by the Forest server's workflow listing endpoint. + */ +export interface McpWorkflow { + workflowId: string; + name: string; + collectionName: string | null; +} + +export interface ListMcpWorkflowsParams { + forestServerToken: string; + renderingId: string; + collectionName?: string; +} + +/** + * A single workflow resolved by id: the match is resolved inside Postgres, so the client never + * receives or deserializes the full workflow list. Unlike the listing, this also carries + * `mcpEnabled`: an existing-but-MCP-disabled workflow is returned with `mcpEnabled: false` rather + * than hidden, so the caller can tell an unknown workflow from a disabled one and reject the + * latter without starting a run. `name` is what makes the fail-closed audit label possible in the + * enabled case; the start endpoint stays the guard that refuses a disabled trigger. + */ +export interface McpWorkflowLookup { + /** Echo of the requested id — the server returns it so the payload is self-describing. */ + workflowId: string; + name: string; + collectionName: string | null; + mcpEnabled: boolean; +} + +export interface GetMcpWorkflowByIdParams { + forestServerToken: string; + renderingId: string; + workflowId: string; +} + +/** + * The lifecycle state of a workflow run, as persisted by the orchestrator. + */ +export type WorkflowRunState = 'started' | 'pending' | 'loading' | 'aborted' | 'finished'; + +/** + * The outcome of starting a workflow run: the run continues asynchronously server-side. + * `runId` is normalized to a string so it can be fed back to `getMcpWorkflowRun` as-is. + * Workflow name and collection are not part of this contract — the audit label is resolved + * up front via `getMcpWorkflowById`. + */ +export interface WorkflowRunTriggerResult { + runId: string; + runState: WorkflowRunState; +} + +export interface TriggerMcpWorkflowParams { + forestServerToken: string; + renderingId: string; + workflowId: string; + recordId: string; +} + +export type WorkflowRunEngine = 'orchestrator' | 'browser'; + +export type WorkflowRunTriggerType = 'manual' | 'webhook' | 'mcp'; + +export type WorkflowStepType = + | 'task' + | 'condition' + | 'end' + | 'escalation' + | 'start-sub-workflow' + | 'close-sub-workflow'; + +export type WorkflowTaskType = + | 'guideline' + | 'trigger-action' + | 'get-data' + | 'update-data' + | 'load-related-record' + | 'mcp-server'; + +export interface WorkflowStepOutgoing { + stepId: string; + buttonText: string | null; + buttonColor?: string | null; + answer?: string; +} + +/** + * The resolved BPMN definition of a step. Common fields are typed; task-type-specific + * fields (completionType, inputType, preRecordedArgs, mcpServerId, ...) vary by `taskType` + * and are left open via the index signature. + */ +export interface WorkflowStepDefinition { + type: WorkflowStepType; + title: string; + prompt?: string; + executionType: 'manual' | 'automated-with-confirmation' | 'fully-automated'; + automaticCompletion: boolean; + outgoing: WorkflowStepOutgoing[]; + taskType?: WorkflowTaskType; + [key: string]: unknown; +} + +export interface WorkflowHistoryStepContext { + manuallyCompleted?: true; + completedBy?: number; + subWorkflowVersion?: string; + selectedOption?: string; + error?: string; + childrenWorkflowId?: string; + escalationState?: 'escalated' | 'error'; + awaitingInputReason?: 'needs-oauth-reauth'; +} + +/** + * One entry in a run's history: the step and its resolved definition plus per-step context. + */ +export interface WorkflowHistoryStep { + stepName: string; + stepIndex: number; + originalStepIndex?: number; + done: boolean; + revised?: boolean; + cancelled?: boolean; + childrenWorkflowId?: string; + isCardStep: boolean; + context?: WorkflowHistoryStepContext; + stepDefinition: WorkflowStepDefinition; +} + +/** + * The full hydrated workflow run as returned by the orchestrator for MCP consumption. + * It exposes the whole run — including internal fields (userId, bpmnVersion, collectionId, + * step indices, per-step context) — so the LLM has maximum context about where the run is + * and what each step does. The orchestrator carries no record payload (records live in the + * executor), but identifiers (`selectedRecordId`) and executor-reported error strings + * (`context.error`) may still contain customer data. + */ +export interface HydratedWorkflowRun { + id: number; + userId: number; + renderingId: number; + collectionId: string; + workflowId: string; + bpmnVersion: string; + selectedRecordId: string; + runState: WorkflowRunState; + engine: WorkflowRunEngine; + triggerType: WorkflowRunTriggerType; + lockedAt: string | null; + createdAt: string; + updatedAt: string; + workflowHistory: WorkflowHistoryStep[]; +} + +export interface GetMcpWorkflowRunParams { + forestServerToken: string; + renderingId: string; + runId: string; +} + +/** + * Service interface for workflow operations (MCP-related). + */ +export interface WorkflowsServiceInterface { + listMcpEnabledWorkflows: (params: ListMcpWorkflowsParams) => Promise; + getMcpWorkflowById: (params: GetMcpWorkflowByIdParams) => Promise; + triggerMcpWorkflow: (params: TriggerMcpWorkflowParams) => Promise; + getMcpWorkflowRun: (params: GetMcpWorkflowRunParams) => Promise; +} + /** * Service interface for schema operations (extended for MCP). */ @@ -320,6 +493,29 @@ export interface ForestAdminServerInterface { id: string, body: object, ) => Promise; + + // Workflow operations + listMcpEnabledWorkflows?: ( + options: ActivityLogHttpOptions, + renderingId: string, + collectionName?: string, + ) => Promise; + getMcpWorkflowById?: ( + options: ActivityLogHttpOptions, + renderingId: string, + workflowId: string, + ) => Promise; + triggerMcpWorkflow?: ( + options: ActivityLogHttpOptions, + renderingId: string, + workflowId: string, + recordId: string, + ) => Promise; + getMcpWorkflowRun?: ( + options: ActivityLogHttpOptions, + renderingId: string, + runId: string, + ) => Promise; } export type ActivityLogHttpOptions = { diff --git a/packages/forestadmin-client/src/workflows/index.ts b/packages/forestadmin-client/src/workflows/index.ts new file mode 100644 index 0000000000..e001b8e1ca --- /dev/null +++ b/packages/forestadmin-client/src/workflows/index.ts @@ -0,0 +1,94 @@ +import type { + ForestAdminServerInterface, + GetMcpWorkflowByIdParams, + GetMcpWorkflowRunParams, + HydratedWorkflowRun, + ListMcpWorkflowsParams, + McpWorkflow, + McpWorkflowLookup, + TriggerMcpWorkflowParams, + WorkflowRunTriggerResult, +} from '../types'; + +export type WorkflowsServiceOptions = { + forestServerUrl: string; + headers?: Record; +}; + +type WorkflowTransportMethod = + | 'listMcpEnabledWorkflows' + | 'getMcpWorkflowById' + | 'triggerMcpWorkflow' + | 'getMcpWorkflowRun'; + +export default class WorkflowsService { + constructor( + private forestAdminServerInterface: ForestAdminServerInterface, + private options: WorkflowsServiceOptions, + ) {} + + // The transport interface methods are optional so external implementations keep compiling; + // resolve the bound method or fail with a uniform message. + private resolveTransportMethod( + name: K, + ): NonNullable { + const method = this.forestAdminServerInterface[name]; + + if (!method) { + throw new Error(`The configured Forest server transport does not support ${name}.`); + } + + return method.bind(this.forestAdminServerInterface) as NonNullable< + ForestAdminServerInterface[K] + >; + } + + private httpOptions(forestServerToken: string) { + return { + forestServerUrl: this.options.forestServerUrl, + bearerToken: forestServerToken, + headers: this.options.headers, + }; + } + + async listMcpEnabledWorkflows(params: ListMcpWorkflowsParams): Promise { + const { forestServerToken, renderingId, collectionName } = params; + + return this.resolveTransportMethod('listMcpEnabledWorkflows')( + this.httpOptions(forestServerToken), + renderingId, + collectionName, + ); + } + + async getMcpWorkflowById(params: GetMcpWorkflowByIdParams): Promise { + const { forestServerToken, renderingId, workflowId } = params; + + return this.resolveTransportMethod('getMcpWorkflowById')( + this.httpOptions(forestServerToken), + renderingId, + workflowId, + ); + } + + async triggerMcpWorkflow(params: TriggerMcpWorkflowParams): Promise { + const { forestServerToken, renderingId, workflowId, recordId } = params; + + return this.resolveTransportMethod('triggerMcpWorkflow')( + this.httpOptions(forestServerToken), + renderingId, + workflowId, + recordId, + ); + } + + async getMcpWorkflowRun(params: GetMcpWorkflowRunParams): Promise { + const { forestServerToken, renderingId, runId } = params; + + return this.resolveTransportMethod('getMcpWorkflowRun')( + this.httpOptions(forestServerToken), + renderingId, + runId, + ); + } +} diff --git a/packages/forestadmin-client/test/__factories__/forest-admin-client.ts b/packages/forestadmin-client/test/__factories__/forest-admin-client.ts index 6a5ccf5746..3c2b0040dd 100644 --- a/packages/forestadmin-client/test/__factories__/forest-admin-client.ts +++ b/packages/forestadmin-client/test/__factories__/forest-admin-client.ts @@ -13,6 +13,7 @@ import permissionServiceFactory from './permissions/permission'; import renderingPermissionsFactory from './permissions/rendering-permission'; import schemaServiceFactory from './schema'; import contextVariablesInstantiatorFactory from './utils/context-variables-instantiator'; +import workflowsServiceFactory from './workflows'; import ForestAdminClient from '../../src/forest-admin-client-with-cache'; export class ForestAdminClientFactory extends Factory { @@ -41,6 +42,7 @@ const forestAdminClientFactory = ForestAdminClientFactory.define( mcpServerConfigServiceFactory.build(), eventsSubscriptionServiceFactory.build(), nativeRefreshEventsHandlerServiceFactory.build(), + workflowsServiceFactory.build(), ), ); diff --git a/packages/forestadmin-client/test/__factories__/forest-admin-server-interface.ts b/packages/forestadmin-client/test/__factories__/forest-admin-server-interface.ts index d89fb5817e..1924091734 100644 --- a/packages/forestadmin-client/test/__factories__/forest-admin-server-interface.ts +++ b/packages/forestadmin-client/test/__factories__/forest-admin-server-interface.ts @@ -18,6 +18,11 @@ const forestAdminServerInterface = { createActivityLog: jest.fn(), createMcpActivityLog: jest.fn(), updateActivityLogStatus: jest.fn(), + // Workflow operations + listMcpEnabledWorkflows: jest.fn(), + getMcpWorkflowById: jest.fn(), + triggerMcpWorkflow: jest.fn(), + getMcpWorkflowRun: jest.fn(), }), }; diff --git a/packages/forestadmin-client/test/__factories__/index.ts b/packages/forestadmin-client/test/__factories__/index.ts index 52d5fccfb4..68dacdb062 100644 --- a/packages/forestadmin-client/test/__factories__/index.ts +++ b/packages/forestadmin-client/test/__factories__/index.ts @@ -11,6 +11,7 @@ export { default as forestAdminClientOptions } from './forest-admin-client-optio export { default as ipWhiteList } from './ip-whitelist'; export { default as schema } from './schema'; export { default as activityLogs } from './activity-logs'; +export { default as workflows } from './workflows'; export { default as auth } from './auth'; export { default as modelCustomization } from './model-customizations/model-customization-from-api'; export { default as mcpServerConfig } from './mcp-server-config'; diff --git a/packages/forestadmin-client/test/__factories__/workflows/index.ts b/packages/forestadmin-client/test/__factories__/workflows/index.ts new file mode 100644 index 0000000000..7dfca54595 --- /dev/null +++ b/packages/forestadmin-client/test/__factories__/workflows/index.ts @@ -0,0 +1,11 @@ +import { Factory } from 'fishery'; + +import WorkflowsService from '../../../src/workflows'; +import forestAdminClientOptions from '../forest-admin-client-options'; +import forestAdminServerInterface from '../forest-admin-server-interface'; + +const workflowsServiceFactory = Factory.define(() => { + return new WorkflowsService(forestAdminServerInterface.build(), forestAdminClientOptions.build()); +}); + +export default workflowsServiceFactory; diff --git a/packages/forestadmin-client/test/forest-admin-client-with-cache.test.ts b/packages/forestadmin-client/test/forest-admin-client-with-cache.test.ts index cd8a3e5d5b..fce2a59301 100644 --- a/packages/forestadmin-client/test/forest-admin-client-with-cache.test.ts +++ b/packages/forestadmin-client/test/forest-admin-client-with-cache.test.ts @@ -10,6 +10,63 @@ jest.mock('../src/permissions/verify-approval', () => ({ const verifyAndExtractApprovalMock = verifyAndExtractApproval as jest.Mock; describe('ForestAdminClientWithCache', () => { + describe('constructor wiring', () => { + it('should assign each positional service to its matching property', () => { + const options = factories.forestAdminClientOptions.build(); + const permissionService = factories.permission.build(); + const renderingPermissionService = factories.renderingPermission.build(); + const contextVariablesInstantiator = factories.contextVariablesInstantiator.build(); + const chartHandler = factories.chartHandler.build(); + const ipWhitelistService = factories.ipWhiteList.build(); + const schemaService = factories.schema.build(); + const activityLogsService = factories.activityLogs.build(); + const authService = factories.auth.build(); + const modelCustomizationService = factories.modelCustomization.build(); + const mcpServerConfigService = factories.mcpServerConfig.build(); + const eventsSubscription = factories.eventsSubscription.build(); + const eventsHandler = factories.eventsHandler.build(); + const workflowsService = factories.workflows.build(); + + const forestAdminClient = new ForestAdminClient( + options, + permissionService, + renderingPermissionService, + contextVariablesInstantiator, + chartHandler, + ipWhitelistService, + schemaService, + activityLogsService, + authService, + modelCustomizationService, + mcpServerConfigService, + eventsSubscription, + eventsHandler, + workflowsService, + ); + + // All 14 positions, not just the public ones: this test exists to catch a silent argument + // shift, and two adjacent services of the same shape (eventsSubscription / eventsHandler, + // renderingPermission / ipWhitelist) would swap unnoticed if only the public members were + // asserted. Reading the protected ones needs the cast. + const wired = forestAdminClient as unknown as Record; + + expect(wired.options).toBe(options); + expect(wired.permissionService).toBe(permissionService); + expect(wired.renderingPermissionService).toBe(renderingPermissionService); + expect(wired.contextVariablesInstantiator).toBe(contextVariablesInstantiator); + expect(wired.chartHandler).toBe(chartHandler); + expect(wired.ipWhitelistService).toBe(ipWhitelistService); + expect(wired.schemaService).toBe(schemaService); + expect(wired.activityLogsService).toBe(activityLogsService); + expect(wired.authService).toBe(authService); + expect(wired.modelCustomizationService).toBe(modelCustomizationService); + expect(wired.mcpServerConfigService).toBe(mcpServerConfigService); + expect(wired.eventsSubscription).toBe(eventsSubscription); + expect(wired.eventsHandler).toBe(eventsHandler); + expect(wired.workflowsService).toBe(workflowsService); + }); + }); + describe('getIpWhitelistConfiguration', () => { it('should delegate to the given service', async () => { const whiteListService = factories.ipWhiteList.build({ @@ -30,6 +87,7 @@ describe('ForestAdminClientWithCache', () => { factories.mcpServerConfig.build(), factories.eventsSubscription.build(), factories.eventsHandler.build(), + factories.workflows.build(), ); const config = await forestAdminClient.getIpWhitelistConfiguration(); @@ -58,6 +116,7 @@ describe('ForestAdminClientWithCache', () => { factories.mcpServerConfig.build(), factories.eventsSubscription.build(), factories.eventsHandler.build(), + factories.workflows.build(), ); const result = await forestAdminClient.postSchema({ @@ -92,6 +151,7 @@ describe('ForestAdminClientWithCache', () => { factories.mcpServerConfig.build(), factories.eventsSubscription.build(), factories.eventsHandler.build(), + factories.workflows.build(), ); verifyAndExtractApprovalMock.mockReturnValue(signedParameters); @@ -121,6 +181,7 @@ describe('ForestAdminClientWithCache', () => { factories.mcpServerConfig.build(), factories.eventsSubscription.build(), factories.eventsHandler.build(), + factories.workflows.build(), ); await forestAdminClient.markScopesAsUpdated(42); @@ -146,6 +207,7 @@ describe('ForestAdminClientWithCache', () => { factories.mcpServerConfig.build(), factories.eventsSubscription.build(), factories.eventsHandler.build(), + factories.workflows.build(), ); await forestAdminClient.markScopesAsUpdated(42); @@ -172,6 +234,7 @@ describe('ForestAdminClientWithCache', () => { factories.mcpServerConfig.build(), factories.eventsSubscription.build(), factories.eventsHandler.build(), + factories.workflows.build(), ); (renderingPermissionService.getScope as jest.Mock).mockResolvedValue('scope'); @@ -208,6 +271,7 @@ describe('ForestAdminClientWithCache', () => { factories.mcpServerConfig.build(), eventsSubscriptionService, factories.eventsHandler.build(), + factories.workflows.build(), ); await forestAdminClient.subscribeToServerEvents(); @@ -233,6 +297,7 @@ describe('ForestAdminClientWithCache', () => { factories.mcpServerConfig.build(), eventsSubscriptionService, factories.eventsHandler.build(), + factories.workflows.build(), ); forestAdminClient.close(); @@ -258,6 +323,7 @@ describe('ForestAdminClientWithCache', () => { factories.mcpServerConfig.build(), factories.eventsSubscription.build(), eventsHandlerService, + factories.workflows.build(), ); const handler = jest.fn(); @@ -285,6 +351,7 @@ describe('ForestAdminClientWithCache', () => { factories.mcpServerConfig.build(), factories.eventsSubscription.build(), eventsHandlerService, + factories.workflows.build(), ); const handler = jest.fn(); diff --git a/packages/forestadmin-client/test/permissions/forest-http-api.test.ts b/packages/forestadmin-client/test/permissions/forest-http-api.test.ts index 75ba76222c..1f01172d4c 100644 --- a/packages/forestadmin-client/test/permissions/forest-http-api.test.ts +++ b/packages/forestadmin-client/test/permissions/forest-http-api.test.ts @@ -175,7 +175,7 @@ describe('ForestHttpApi', () => { path: '/api/activity-logs-requests/mcp', bearerToken: 'bearer-token', body, - headers: { 'Custom-Header': 'value' }, + headers: { 'Forest-Application-Source': 'MCP', 'Custom-Header': 'value' }, }); expect(result).toEqual(mockActivityLog); }); @@ -206,4 +206,418 @@ describe('ForestHttpApi', () => { }); }); }); + + describe('listMcpEnabledWorkflows', () => { + it('should GET the workflows endpoint with the rendering id header', async () => { + const workflows = [{ workflowId: 'wf-1', name: 'Refund order', collectionName: 'orders' }]; + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue(workflows); + + const result = await new ForestHttpApi().listMcpEnabledWorkflows( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + ); + + expect(ServerUtils.queryWithBearerToken).toHaveBeenCalledWith({ + forestServerUrl: options.forestServerUrl, + method: 'get', + path: '/api/workflow-orchestrator/mcp-workflows', + bearerToken: 'bearer-token', + headers: { + 'forest-rendering-id': '12345', + 'Forest-Application-Source': 'MCP', + }, + }); + expect(result).toEqual(workflows); + }); + + it('should strip fields the contract does not declare from each listed workflow', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue([ + { + workflowId: 'wf-1', + name: 'Refund order', + collectionName: 'orders', + // A column added to the server query must not reach the model's prompt on its own. + createdByEmail: 'ops@example.com', + bpmnAwsS3Identifier: 'internal/path.bpmn', + }, + ]); + + const result = await new ForestHttpApi().listMcpEnabledWorkflows( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + ); + + expect(result).toEqual([ + { workflowId: 'wf-1', name: 'Refund order', collectionName: 'orders' }, + ]); + expect(JSON.stringify(result)).not.toContain('ops@example.com'); + expect(JSON.stringify(result)).not.toContain('internal/path.bpmn'); + }); + + it('should return an empty array when the server answers with no body', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue(undefined); + + const result = await new ForestHttpApi().listMcpEnabledWorkflows( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + ); + + expect(result).toEqual([]); + }); + + it('should append the collectionName filter as a url-encoded query param', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue([]); + + await new ForestHttpApi().listMcpEnabledWorkflows( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + 'sales orders', + ); + + expect(ServerUtils.queryWithBearerToken).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'get', + path: '/api/workflow-orchestrator/mcp-workflows?collectionName=sales%20orders', + headers: { + 'forest-rendering-id': '12345', + 'Forest-Application-Source': 'MCP', + }, + }), + ); + }); + }); + + describe('getMcpWorkflowById', () => { + it('should GET the by-id workflow endpoint with the rendering id header', async () => { + const workflow = { + workflowId: 'wf-1', + name: 'Refund order', + collectionName: 'orders', + mcpEnabled: true, + }; + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue(workflow); + + const result = await new ForestHttpApi().getMcpWorkflowById( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + 'wf-1', + ); + + expect(ServerUtils.queryWithBearerToken).toHaveBeenCalledWith({ + forestServerUrl: options.forestServerUrl, + method: 'get', + path: '/api/workflow-orchestrator/mcp-workflows/wf-1', + bearerToken: 'bearer-token', + headers: { + 'forest-rendering-id': '12345', + 'Forest-Application-Source': 'MCP', + }, + }); + expect(result).toEqual(workflow); + }); + + it('should drop any field the contract does not declare', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ + workflowId: 'wf-1', + name: 'Refund order', + collectionName: 'orders', + mcpEnabled: true, + // A column the server could add tomorrow. `name` from this payload is written into a + // persisted audit label, so the response is a whitelist like the other three MCP routes. + internalOwnerEmail: 'ops@acme.corp', + }); + + const result = await new ForestHttpApi().getMcpWorkflowById( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + 'wf-1', + ); + + expect(Object.keys(result).sort()).toEqual([ + 'collectionName', + 'mcpEnabled', + 'name', + 'workflowId', + ]); + }); + + it('should url-encode the workflow id in the path', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ + workflowId: 'wf/with space', + name: 'Refund order', + collectionName: null, + mcpEnabled: false, + }); + + await new ForestHttpApi().getMcpWorkflowById( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + 'wf/with space', + ); + + expect(ServerUtils.queryWithBearerToken).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'get', + path: '/api/workflow-orchestrator/mcp-workflows/wf%2Fwith%20space', + }), + ); + }); + }); + + describe('triggerMcpWorkflow', () => { + it('should POST the record id to the workflow start endpoint with the rendering id header', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ + runId: 7, + runState: 'loading', + }); + + const result = await new ForestHttpApi().triggerMcpWorkflow( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + 'wf-1', + '42', + ); + + expect(ServerUtils.queryWithBearerToken).toHaveBeenCalledWith({ + forestServerUrl: options.forestServerUrl, + method: 'post', + path: '/api/workflow-orchestrator/mcp-workflows/wf-1/start', + bearerToken: 'bearer-token', + body: { recordId: '42' }, + headers: { + 'forest-rendering-id': '12345', + 'Forest-Application-Source': 'MCP', + }, + }); + expect(result).toEqual({ runId: '7', runState: 'loading' }); + }); + + it('should normalize a numeric runId returned by the server to a string', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ + runId: 7, + runState: 'loading', + }); + + const result = await new ForestHttpApi().triggerMcpWorkflow( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + 'wf-1', + '42', + ); + + expect(result.runId).toBe('7'); + }); + + it('should url-encode the workflow id in the path', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ + runId: 1, + runState: 'loading', + }); + + await new ForestHttpApi().triggerMcpWorkflow( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + 'wf/with space', + '42', + ); + + expect(ServerUtils.queryWithBearerToken).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'post', + path: '/api/workflow-orchestrator/mcp-workflows/wf%2Fwith%20space/start', + }), + ); + }); + }); + + describe('getMcpWorkflowRun', () => { + it('should GET the workflow run endpoint with the rendering id header', async () => { + const runStatus = { + id: 7, + userId: 42, + renderingId: 12345, + collectionId: 'orders', + workflowId: 'wf-1', + bpmnVersion: '3', + selectedRecordId: '99', + runState: 'finished', + engine: 'orchestrator', + triggerType: 'mcp', + lockedAt: null, + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:05:00.000Z', + workflowHistory: [], + }; + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue(runStatus); + + const result = await new ForestHttpApi().getMcpWorkflowRun( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + '7', + ); + + expect(ServerUtils.queryWithBearerToken).toHaveBeenCalledWith({ + forestServerUrl: options.forestServerUrl, + method: 'get', + path: '/api/workflow-orchestrator/mcp-workflows/runs/7', + bearerToken: 'bearer-token', + headers: { + 'forest-rendering-id': '12345', + 'Forest-Application-Source': 'MCP', + }, + }); + expect(result).toEqual(runStatus); + }); + + it('should strip fields the contract does not declare, including a leaked serverToken', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ + id: 7, + runState: 'started', + workflowHistory: [ + { stepName: 'Review', stepIndex: 0, done: false, stepDefinition: { type: 'task' } }, + ], + // What the executor-facing build of the same run carries. + collectionName: 'orders', + userProfile: { email: 'ops@example.com', serverToken: 'super-secret-forest-token' }, + }); + + const result = await new ForestHttpApi().getMcpWorkflowRun( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + '7', + ); + + expect(result).not.toHaveProperty('userProfile'); + expect(result).not.toHaveProperty('collectionName'); + expect(JSON.stringify(result)).not.toContain('super-secret-forest-token'); + expect(result.id).toBe(7); + expect(result.runState).toBe('started'); + }); + + it('should strip fields the contract does not declare from a per-step context', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ + runState: 'started', + workflowHistory: [ + { + stepName: 'Approve', + stepIndex: 0, + done: false, + stepDefinition: { type: 'escalation' }, + context: { + escalationState: 'escalated', + error: 'record 42 not found', + // The step context is a closed interface here but an open bag server-side. + completedByUser: { email: 'ops@example.com', serverToken: 'leaked-token' }, + }, + }, + ], + }); + + const result = await new ForestHttpApi().getMcpWorkflowRun( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + '7', + ); + + expect(result.workflowHistory[0].context).not.toHaveProperty('completedByUser'); + expect(JSON.stringify(result)).not.toContain('leaked-token'); + // The declared fields the docs tell readers to diagnose with survive. + expect(result.workflowHistory[0].context).toMatchObject({ + escalationState: 'escalated', + error: 'record 42 not found', + }); + }); + + it('should leave a step with no context untouched', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ + runState: 'started', + workflowHistory: [ + { stepName: 'Review', stepIndex: 0, done: true, stepDefinition: { type: 'task' } }, + ], + }); + + const result = await new ForestHttpApi().getMcpWorkflowRun( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + '7', + ); + + expect(result.workflowHistory[0].context).toBeUndefined(); + }); + + it('should keep the task-type-specific stepDefinition fields the model reasons about', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ + runState: 'started', + workflowHistory: [ + { + stepName: 'Refund', + stepIndex: 0, + done: false, + isCardStep: false, + context: { error: 'boom' }, + stepDefinition: { + type: 'task', + taskType: 'update-data', + preRecordedArgs: { fieldName: 'status', value: 'refunded' }, + }, + }, + ], + }); + + const result = await new ForestHttpApi().getMcpWorkflowRun( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + '7', + ); + + expect(result.workflowHistory[0]).toEqual({ + stepName: 'Refund', + stepIndex: 0, + originalStepIndex: undefined, + done: false, + revised: undefined, + cancelled: undefined, + childrenWorkflowId: undefined, + isCardStep: false, + context: { error: 'boom' }, + stepDefinition: { + type: 'task', + taskType: 'update-data', + preRecordedArgs: { fieldName: 'status', value: 'refunded' }, + }, + }); + }); + + it('should tolerate a run served without a workflowHistory', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ runState: 'pending' }); + + const result = await new ForestHttpApi().getMcpWorkflowRun( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + '7', + ); + + expect(result.workflowHistory).toEqual([]); + }); + + it('should url-encode the run id in the path', async () => { + (ServerUtils.queryWithBearerToken as jest.Mock).mockResolvedValue({ + runState: 'started', + workflowHistory: [], + }); + + await new ForestHttpApi().getMcpWorkflowRun( + { forestServerUrl: options.forestServerUrl, bearerToken: 'bearer-token' }, + '12345', + 'run/with space', + ); + + expect(ServerUtils.queryWithBearerToken).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'get', + path: '/api/workflow-orchestrator/mcp-workflows/runs/run%2Fwith%20space', + }), + ); + }); + }); }); diff --git a/packages/forestadmin-client/test/utils/server.test.ts b/packages/forestadmin-client/test/utils/server.test.ts index 7a5f48164f..fa3f782c4a 100644 --- a/packages/forestadmin-client/test/utils/server.test.ts +++ b/packages/forestadmin-client/test/utils/server.test.ts @@ -322,5 +322,24 @@ describe('ServerUtils', () => { }), ).rejects.toThrow('The requested resource was not found on Forest Admin server.'); }); + + it('should surface the server detail on a 409 conflict (already-running workflow run)', async () => { + nock(options.forestServerUrl) + .post('/api/workflow-orchestrator/mcp-workflows/wf-1/start') + .reply(409, { errors: [{ detail: 'A run is already ongoing on this record' }] }); + + await expect( + ServerUtils.queryWithBearerToken({ + forestServerUrl: options.forestServerUrl, + method: 'post', + path: '/api/workflow-orchestrator/mcp-workflows/wf-1/start', + bearerToken: 'valid-token', + body: { recordId: '42' }, + }), + ).rejects.toMatchObject({ + message: 'A run is already ongoing on this record', + status: 409, + }); + }); }); }); diff --git a/packages/forestadmin-client/test/workflows/index.test.ts b/packages/forestadmin-client/test/workflows/index.test.ts new file mode 100644 index 0000000000..b4b1bfcd76 --- /dev/null +++ b/packages/forestadmin-client/test/workflows/index.test.ts @@ -0,0 +1,299 @@ +import type { ForestAdminServerInterface, McpWorkflow } from '../../src/types'; + +import WorkflowsService from '../../src/workflows'; +import * as factories from '../__factories__'; + +describe('WorkflowsService', () => { + const options = { + forestServerUrl: 'http://forestadmin-server.com', + }; + let mockForestAdminServerInterface: jest.Mocked; + + beforeEach(() => { + jest.clearAllMocks(); + mockForestAdminServerInterface = + factories.forestAdminServerInterface.build() as jest.Mocked; + }); + + describe('listMcpEnabledWorkflows', () => { + const workflows: McpWorkflow[] = [ + { workflowId: 'wf-1', name: 'Refund order', collectionName: 'orders' }, + ]; + + it('should forward the bearer token and rendering id to the transport', async () => { + mockForestAdminServerInterface.listMcpEnabledWorkflows.mockResolvedValue(workflows); + + const service = new WorkflowsService(mockForestAdminServerInterface, options); + const result = await service.listMcpEnabledWorkflows({ + forestServerToken: 'test-token', + renderingId: '12345', + }); + + expect(result).toEqual(workflows); + expect(mockForestAdminServerInterface.listMcpEnabledWorkflows).toHaveBeenCalledWith( + { forestServerUrl: options.forestServerUrl, bearerToken: 'test-token', headers: undefined }, + '12345', + undefined, + ); + }); + + it('should forward the collectionName filter when provided', async () => { + mockForestAdminServerInterface.listMcpEnabledWorkflows.mockResolvedValue(workflows); + + const service = new WorkflowsService(mockForestAdminServerInterface, options); + await service.listMcpEnabledWorkflows({ + forestServerToken: 'test-token', + renderingId: '12345', + collectionName: 'orders', + }); + + expect(mockForestAdminServerInterface.listMcpEnabledWorkflows).toHaveBeenCalledWith( + expect.objectContaining({ bearerToken: 'test-token' }), + '12345', + 'orders', + ); + }); + + it('should pass custom headers when provided', async () => { + mockForestAdminServerInterface.listMcpEnabledWorkflows.mockResolvedValue(workflows); + + const service = new WorkflowsService(mockForestAdminServerInterface, { + ...options, + headers: { 'Forest-Application-Source': 'MCP' }, + }); + await service.listMcpEnabledWorkflows({ + forestServerToken: 'test-token', + renderingId: '12345', + }); + + expect(mockForestAdminServerInterface.listMcpEnabledWorkflows).toHaveBeenCalledWith( + expect.objectContaining({ + bearerToken: 'test-token', + headers: { 'Forest-Application-Source': 'MCP' }, + }), + '12345', + undefined, + ); + }); + + it('should throw when the transport does not implement listMcpEnabledWorkflows', async () => { + delete (mockForestAdminServerInterface as Partial) + .listMcpEnabledWorkflows; + + const service = new WorkflowsService(mockForestAdminServerInterface, options); + + await expect( + service.listMcpEnabledWorkflows({ forestServerToken: 'test-token', renderingId: '12345' }), + ).rejects.toThrow('does not support listMcpEnabledWorkflows'); + }); + }); + + describe('getMcpWorkflowById', () => { + const workflow = { + workflowId: 'wf-1', + name: 'Refund order', + collectionName: 'orders', + mcpEnabled: true, + }; + + it('should forward the identity and workflowId to the transport and return the workflow', async () => { + mockForestAdminServerInterface.getMcpWorkflowById.mockResolvedValue(workflow); + + const service = new WorkflowsService(mockForestAdminServerInterface, options); + const result = await service.getMcpWorkflowById({ + forestServerToken: 'test-token', + renderingId: '12345', + workflowId: 'wf-1', + }); + + expect(result).toEqual(workflow); + expect(mockForestAdminServerInterface.getMcpWorkflowById).toHaveBeenCalledWith( + { forestServerUrl: options.forestServerUrl, bearerToken: 'test-token', headers: undefined }, + '12345', + 'wf-1', + ); + }); + + it('should pass custom headers when provided', async () => { + mockForestAdminServerInterface.getMcpWorkflowById.mockResolvedValue(workflow); + + const service = new WorkflowsService(mockForestAdminServerInterface, { + ...options, + headers: { 'Forest-Application-Source': 'MCP' }, + }); + await service.getMcpWorkflowById({ + forestServerToken: 'test-token', + renderingId: '12345', + workflowId: 'wf-1', + }); + + expect(mockForestAdminServerInterface.getMcpWorkflowById).toHaveBeenCalledWith( + expect.objectContaining({ + bearerToken: 'test-token', + headers: { 'Forest-Application-Source': 'MCP' }, + }), + '12345', + 'wf-1', + ); + }); + + it('should throw when the transport does not implement getMcpWorkflowById', async () => { + delete (mockForestAdminServerInterface as Partial) + .getMcpWorkflowById; + + const service = new WorkflowsService(mockForestAdminServerInterface, options); + + await expect( + service.getMcpWorkflowById({ + forestServerToken: 'test-token', + renderingId: '12345', + workflowId: 'wf-1', + }), + ).rejects.toThrow('does not support getMcpWorkflowById'); + }); + }); + + describe('triggerMcpWorkflow', () => { + it('should forward the identity, workflowId and recordId to the transport and return the run', async () => { + mockForestAdminServerInterface.triggerMcpWorkflow.mockResolvedValue({ + runId: '7', + runState: 'loading', + }); + + const service = new WorkflowsService(mockForestAdminServerInterface, options); + const result = await service.triggerMcpWorkflow({ + forestServerToken: 'test-token', + renderingId: '12345', + workflowId: 'wf-1', + recordId: '42', + }); + + expect(result).toEqual({ runId: '7', runState: 'loading' }); + expect(mockForestAdminServerInterface.triggerMcpWorkflow).toHaveBeenCalledWith( + { forestServerUrl: options.forestServerUrl, bearerToken: 'test-token', headers: undefined }, + '12345', + 'wf-1', + '42', + ); + }); + + it('should pass custom headers when provided', async () => { + mockForestAdminServerInterface.triggerMcpWorkflow.mockResolvedValue({ + runId: '7', + runState: 'loading', + }); + + const service = new WorkflowsService(mockForestAdminServerInterface, { + ...options, + headers: { 'Forest-Application-Source': 'MCP' }, + }); + await service.triggerMcpWorkflow({ + forestServerToken: 'test-token', + renderingId: '12345', + workflowId: 'wf-1', + recordId: '42', + }); + + expect(mockForestAdminServerInterface.triggerMcpWorkflow).toHaveBeenCalledWith( + expect.objectContaining({ + bearerToken: 'test-token', + headers: { 'Forest-Application-Source': 'MCP' }, + }), + '12345', + 'wf-1', + '42', + ); + }); + + it('should throw when the transport does not implement triggerMcpWorkflow', async () => { + delete (mockForestAdminServerInterface as Partial) + .triggerMcpWorkflow; + + const service = new WorkflowsService(mockForestAdminServerInterface, options); + + await expect( + service.triggerMcpWorkflow({ + forestServerToken: 'test-token', + renderingId: '12345', + workflowId: 'wf-1', + recordId: '42', + }), + ).rejects.toThrow('does not support triggerMcpWorkflow'); + }); + }); + + describe('getMcpWorkflowRun', () => { + const runStatus = { + id: 7, + userId: 42, + renderingId: 12345, + collectionId: 'orders', + workflowId: 'wf-1', + bpmnVersion: '3', + selectedRecordId: '99', + runState: 'finished' as const, + engine: 'orchestrator' as const, + triggerType: 'mcp' as const, + lockedAt: null, + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:05:00.000Z', + workflowHistory: [], + }; + + it('should forward the identity and runId to the transport and return the status', async () => { + mockForestAdminServerInterface.getMcpWorkflowRun.mockResolvedValue(runStatus); + + const service = new WorkflowsService(mockForestAdminServerInterface, options); + const result = await service.getMcpWorkflowRun({ + forestServerToken: 'test-token', + renderingId: '12345', + runId: '7', + }); + + expect(result).toEqual(runStatus); + expect(mockForestAdminServerInterface.getMcpWorkflowRun).toHaveBeenCalledWith( + { forestServerUrl: options.forestServerUrl, bearerToken: 'test-token', headers: undefined }, + '12345', + '7', + ); + }); + + it('should pass custom headers when provided', async () => { + mockForestAdminServerInterface.getMcpWorkflowRun.mockResolvedValue(runStatus); + + const service = new WorkflowsService(mockForestAdminServerInterface, { + ...options, + headers: { 'Forest-Application-Source': 'MCP' }, + }); + await service.getMcpWorkflowRun({ + forestServerToken: 'test-token', + renderingId: '12345', + runId: '7', + }); + + expect(mockForestAdminServerInterface.getMcpWorkflowRun).toHaveBeenCalledWith( + expect.objectContaining({ + bearerToken: 'test-token', + headers: { 'Forest-Application-Source': 'MCP' }, + }), + '12345', + '7', + ); + }); + + it('should throw when the transport does not implement getMcpWorkflowRun', async () => { + delete (mockForestAdminServerInterface as Partial) + .getMcpWorkflowRun; + + const service = new WorkflowsService(mockForestAdminServerInterface, options); + + await expect( + service.getMcpWorkflowRun({ + forestServerToken: 'test-token', + renderingId: '12345', + runId: '7', + }), + ).rejects.toThrow('does not support getMcpWorkflowRun'); + }); + }); +}); diff --git a/packages/mcp-server/CLAUDE.md b/packages/mcp-server/CLAUDE.md index 4358f33e30..0a4eee3a3f 100644 --- a/packages/mcp-server/CLAUDE.md +++ b/packages/mcp-server/CLAUDE.md @@ -14,8 +14,10 @@ Key flows that only make sense across files: - **Per-request MCP server.** `handleMcpRequest` builds a *fresh* `McpServer` + `StreamableHTTPServerTransport` per request (`sessionIdGenerator: undefined`, stateless) and closes the transport on response end. Tool registration in `createMcpServer()` therefore re-runs on every call. - **OAuth = pass-through to Forest Admin.** `ForestOAuthProvider` (`src/forest-oauth-provider.ts`) does not store tokens. `/oauth/authorize` redirects to the Forest app; `exchange*` calls relay to the Forest server's `/oauth/token`, then `generateAccessToken` re-signs a JWT with `authSecret` carrying the Forest token under the `serverToken` claim. `verifyAccessToken` verifies that JWT and builds `AuthInfo.extra` with `forestServerToken: decoded.serverToken` plus `environmentApiEndpoint` — the latter is **not** in the JWT, it's read from the provider's own `this.environmentApiEndpoint` field (set during env discovery). Both lifetimes mirror the Forest token's own `exp`, optionally shortened by the `tokenTtl` option (`src/utils/token-ttl.ts`) — always a `min`, never a `max`, and capped at the point the TTL is computed so the advertised `expires_in` matches the signed JWT (except on the already-expired-upstream branch, which advertises 3600). The refresh cap is anchored on a `sessionStartedAt` claim carried across refreshes and never re-stamped: Forest grants a full refresh lifetime on every refresh, so a per-refresh cap would slide forever and never force a re-login. `clientsStore.getClient` is the single choke point all three OAuth paths (authorize, code exchange, refresh) resolve clients through — when the `allowedOAuthClients` option is set, it rejects any client whose registered redirect URIs are not all http(s) URIs on an allowed domain or subdomain, by throwing `InvalidClientError` (rendered as a 400 `invalid_client` by the SDK handlers, never a redirect). DCR registration itself happens on the Forest server and is never blocked here. -- **Tools call the live agent, not this server.** Each tool in `src/tools/*` is a `declareXxxTool(mcpServer, forestServerClient, logger, collectionNames)` factory. At call time `buildClient(extra)` (`src/utils/agent-caller.ts`) reads `extra.authInfo` (`forestServerToken` + `environmentApiEndpoint` from `AuthInfo.extra`) and builds a `createRemoteAgentClient` from `@forestadmin/agent-client` — i.e. the tool RPCs into the user's actual running agent. `forestServerClient` (`src/http-client`, wrapping `@forestadmin/forestadmin-client`'s `SchemaService`/`ActivityLogsService`) is used only for schema fetch and activity logging, not data. -- **Two cross-cutting wrappers, always used together.** `registerToolWithLogging` (`src/utils/tool-with-logging.ts`) registers the tool and converts thrown errors into `{ isError: true }` results (per MCP spec) instead of protocol errors. Inside the handler, `withActivityLog` (`src/utils/with-activity-log.ts`) brackets the operation with a pending→succeeded/failed Forest activity log and runs `parseAgentError` + optional `errorEnhancer` (e.g. `list` appends sortable field names on "Invalid sort"). +- **Record-level tools call the live agent, not this server.** Each tool in `src/tools/*` is a `declareXxxTool(mcpServer, forestServerClient, logger, collectionNames)` factory. At call time `buildClient(extra)` (`src/utils/agent-caller.ts`) reads `extra.authInfo` (`forestServerToken` + `environmentApiEndpoint` from `AuthInfo.extra`) and builds a `createRemoteAgentClient` from `@forestadmin/agent-client` — i.e. the tool RPCs into the user's actual running agent. `forestServerClient` (`src/http-client`, wrapping `@forestadmin/forestadmin-client`'s `SchemaService` / `ActivityLogsService` / `WorkflowsService`) carries schema fetch, activity logging, and — for the workflow tools only — orchestrator data. +- **The workflow tools are the exception.** `listWorkflows` / `triggerWorkflow` / `getWorkflowRun` never reach the agent: workflows live in the orchestrator, so these go through `forestServerClient.workflowsService` over the `/api/workflow-orchestrator/mcp-workflows/*` HTTP contract. Two consequences worth knowing before editing them: responses are **projected onto an explicit whitelist** in `forest-http-api.ts` (their payload is stringified straight into a model's context, so a new server field must not arrive by itself) — with one deliberate hole, `stepDefinition`, forwarded whole so the model can reason about the step, which means a field added to a *step* type does reach the model unannounced; and `triggerWorkflow` resolves the workflow by id **before** anything is written so its audit label exists ahead of the side effect. +- **Two cross-cutting wrappers.** Every tool uses `registerToolWithLogging`; most, but not all, also use `withActivityLog` — `getActionForm`, `listWorkflows`, `getWorkflowRun` and `requestActionFileUpload` are unaudited (tracked in PRD-967), so do not assume a tool writes an activity log without checking. `registerToolWithLogging` (`src/utils/tool-with-logging.ts`) registers the tool and converts thrown errors into `{ isError: true }` results (per MCP spec) instead of protocol errors. Inside the handler, `withActivityLog` (`src/utils/with-activity-log.ts`) brackets the operation with a pending→succeeded/failed Forest activity log and runs `parseAgentError` + optional `errorEnhancer` (e.g. `list` appends sortable field names on "Invalid sort"). +- **Audit fail policy lives in one place.** `createPendingActivityLog` (`src/utils/activity-logs-creator.ts`) decides what happens when the audit log cannot be created, for every tool: a **write** is blocked (no unaudited side effect), a **read** proceeds with a warning (an audit-store outage must not take down the read surface), and an **authorization refusal (401/403) propagates either way** — it is not an outage. This covers both a rejection and a `200` with no log id. Changing it changes behaviour for all tools at once; the policy is pinned in both directions in `test/utils/activity-logs-creator.test.ts`. - **Action file uploads are an opt-in side-channel** (`src/file-uploads/`), on by default: `EphemeralStorage` holds the objects in memory unless `fileUploads.storage` provides a backend. `fileUploads: false` is the off switch (it drops `requestActionFileUpload` from the enabled set, so the tool, the upload endpoint and the `executeAction` instructions all follow); leaving the tool out of `enabledTools` does the same, at the cost of freezing the allowlist. The `requestActionFileUpload` tool (`src/tools/request-action-file-upload.ts`) returns a pre-authorized upload URL plus a user-bound JWT handle signed with `authSecret`. `FieldGetter.getType()` returns the wire value (`['File']`) because agent-bff and workflow-executor put it straight into API responses; `getTypeName()` is the collapsed `'FileList'`, for dispatch and for what this server reports to a model. It is a tool rather than an HTTP route so clients discover it through `tools/list` instead of a sentence in a description. It checks the `mcp:action` scope itself, because `/mcp` only requires `mcp:read` while the route it replaced required `mcp:action`. `executeAction` swaps `"$uploadedFile:"` values for the downloaded `File` object (`resolve.ts`) and lets **agent-client** encode it — this package never builds a data uri itself. `parseFileReference` (`file-reference.ts`) is the single place that recognizes a reference, so SEP-2631 file URIs can be added there without touching the resolution path. `getActionForm` leaves handles unresolved on purpose, because it echoes values back into the model's context — and withholds them from `tryToSetFields` so a change hook fired by another field never reads `.buffer` off a handle string, while still echoing them back as the field's value and counting them as filling a required field, or `canExecute` could never become true. `download` is **not** consume-on-read: `resolve.ts` fetches every reference before `setFields` and `execute`, so a later failure must leave the objects retryable. With no `storage`, `EphemeralStorage` holds the objects in memory and serves a PUT endpoint under `${prefix}/mcp/uploads` — registered **before the body parsers** (they would consume the raw stream) and before `allowedMethods(['POST'])`; `makeIsMcpRoute` already claims everything under `/mcp/`, so no routing change was needed. Per-instance, so it warns on the first `createUploadUrl` rather than at startup — uploads being on by default, a boot warning would reach agents that have no file field at all. - **`collectionNames` → `z.enum`.** `fetchCollectionNames()` populates the schema's collection list; tools turn it into a `z.enum` for `collectionName` so the LLM gets autocomplete/validation. If schema fetch fails the server logs a warning and runs "degraded" with `z.string()`. diff --git a/packages/mcp-server/README.md b/packages/mcp-server/README.md index 5d0a7f8389..c0ca93b959 100644 --- a/packages/mcp-server/README.md +++ b/packages/mcp-server/README.md @@ -21,6 +21,14 @@ This MCP server provides HTTP REST API access to Forest Admin operations, enabli | `getActionForm` | Get the form fields for a custom action | | `executeAction` | Execute a custom action | | `requestActionFileUpload` | Get a destination to upload a file to, for an action `File` field (only with `fileUploads`) | +| `listWorkflows` | List the MCP-enabled workflows in the caller's rendering, optionally filtered by collection | +| `triggerWorkflow` | Start a workflow run on a record, and return its `runId` | +| `getWorkflowRun` | Read a run started through MCP: its state and step-by-step history | + +`triggerWorkflow` starts a process with side effects, so it is annotated `destructiveHint: true` and +MCP clients are expected to ask the user before each call. Like every other tool it is **enabled by +default**, but it stays inert until someone turns the MCP trigger on for a given workflow in Forest +— the server refuses a trigger on any workflow that has not opted in. ## Usage diff --git a/packages/mcp-server/src/http-client/index.ts b/packages/mcp-server/src/http-client/index.ts index 5216369941..9aa10a3b22 100644 --- a/packages/mcp-server/src/http-client/index.ts +++ b/packages/mcp-server/src/http-client/index.ts @@ -1,6 +1,11 @@ import type { ForestServerClient } from './types'; -import { ActivityLogsService, ForestHttpApi, SchemaService } from '@forestadmin/forestadmin-client'; +import { + ActivityLogsService, + ForestHttpApi, + SchemaService, + WorkflowsService, +} from '@forestadmin/forestadmin-client'; import ForestServerClientImpl from './mcp-http-client'; @@ -27,8 +32,17 @@ export function createForestServerClient( ...serviceOptions, headers: { 'Forest-Application-Source': 'MCP' }, }); + const workflowsService = new WorkflowsService(forestHttpApi, { + forestServerUrl: options.forestServerUrl, + headers: { 'Forest-Application-Source': 'MCP' }, + }); - return new ForestServerClientImpl(schemaService, activityLogsService, options.forestServerUrl); + return new ForestServerClientImpl( + schemaService, + activityLogsService, + workflowsService, + options.forestServerUrl, + ); } export { ForestServerClientImpl }; @@ -39,9 +53,18 @@ export type { ActivityLogType, CreateActivityLogParams, ForestServerClient, + GetMcpWorkflowByIdParams, + GetMcpWorkflowRunParams, + ListMcpWorkflowsParams, + McpWorkflow, + McpWorkflowLookup, + TriggerMcpWorkflowParams, + HydratedWorkflowRun, + WorkflowRunTriggerResult, UpdateActivityLogStatusParams, ForestSchemaCollection, ForestSchemaField, ForestSchemaAction, SchemaServiceInterface, + WorkflowsServiceInterface, } from './types'; diff --git a/packages/mcp-server/src/http-client/mcp-http-client.ts b/packages/mcp-server/src/http-client/mcp-http-client.ts index 2be7e2a1d7..674c41146e 100644 --- a/packages/mcp-server/src/http-client/mcp-http-client.ts +++ b/packages/mcp-server/src/http-client/mcp-http-client.ts @@ -4,18 +4,28 @@ import type { CreateActivityLogParams, ForestSchemaCollection, ForestServerClient, + GetMcpWorkflowByIdParams, + GetMcpWorkflowRunParams, + HydratedWorkflowRun, + ListMcpWorkflowsParams, + McpWorkflow, + McpWorkflowLookup, SchemaServiceInterface, + TriggerMcpWorkflowParams, UpdateActivityLogStatusParams, + WorkflowRunTriggerResult, + WorkflowsServiceInterface, } from './types'; /** - * Default implementation of ForestServerClient that uses SchemaService and ActivityLogsService. - * This provides a convenient API for MCP server operations. + * Default implementation of ForestServerClient that uses SchemaService, ActivityLogsService + * and WorkflowsService. This provides a convenient API for MCP server operations. */ export default class ForestServerClientImpl implements ForestServerClient { constructor( private readonly schemaService: SchemaServiceInterface, private readonly activityLogsService: ActivityLogsServiceInterface, + private readonly workflowsService: WorkflowsServiceInterface, public readonly forestServerUrl: string, ) {} @@ -34,4 +44,20 @@ export default class ForestServerClientImpl implements ForestServerClient { async updateActivityLogStatus(params: UpdateActivityLogStatusParams): Promise { return this.activityLogsService.updateActivityLogStatus(params); } + + async listMcpEnabledWorkflows(params: ListMcpWorkflowsParams): Promise { + return this.workflowsService.listMcpEnabledWorkflows(params); + } + + async getMcpWorkflowById(params: GetMcpWorkflowByIdParams): Promise { + return this.workflowsService.getMcpWorkflowById(params); + } + + async triggerMcpWorkflow(params: TriggerMcpWorkflowParams): Promise { + return this.workflowsService.triggerMcpWorkflow(params); + } + + async getMcpWorkflowRun(params: GetMcpWorkflowRunParams): Promise { + return this.workflowsService.getMcpWorkflowRun(params); + } } diff --git a/packages/mcp-server/src/http-client/types.ts b/packages/mcp-server/src/http-client/types.ts index 8ee4b10c07..d3a43b2ebb 100644 --- a/packages/mcp-server/src/http-client/types.ts +++ b/packages/mcp-server/src/http-client/types.ts @@ -7,8 +7,17 @@ import type { ForestSchemaAction, ForestSchemaCollection, ForestSchemaField, + GetMcpWorkflowByIdParams, + GetMcpWorkflowRunParams, + HydratedWorkflowRun, + ListMcpWorkflowsParams, + McpWorkflow, + McpWorkflowLookup, SchemaServiceInterface, + TriggerMcpWorkflowParams, UpdateActivityLogStatusParams, + WorkflowRunTriggerResult, + WorkflowsServiceInterface, } from '@forestadmin/forestadmin-client'; // Re-export types from forestadmin-client for convenience @@ -21,8 +30,17 @@ export type { ForestSchemaAction, ForestSchemaCollection, ForestSchemaField, + GetMcpWorkflowByIdParams, + GetMcpWorkflowRunParams, + HydratedWorkflowRun, + ListMcpWorkflowsParams, + McpWorkflow, + McpWorkflowLookup, SchemaServiceInterface, + TriggerMcpWorkflowParams, UpdateActivityLogStatusParams, + WorkflowRunTriggerResult, + WorkflowsServiceInterface, }; /** @@ -54,4 +72,24 @@ export interface ForestServerClient { * Updates an activity log status. */ updateActivityLogStatus(params: UpdateActivityLogStatusParams): Promise; + + /** + * Lists the MCP-enabled workflows the caller can access in a rendering. + */ + listMcpEnabledWorkflows(params: ListMcpWorkflowsParams): Promise; + + /** + * Resolves a single workflow by id (name, collection, mcpEnabled), scoped to the caller. + */ + getMcpWorkflowById(params: GetMcpWorkflowByIdParams): Promise; + + /** + * Starts a run of an MCP-enabled workflow on a record and returns its runId (async). + */ + triggerMcpWorkflow(params: TriggerMcpWorkflowParams): Promise; + + /** + * Reads the full hydrated workflow run, scoped to the caller. + */ + getMcpWorkflowRun(params: GetMcpWorkflowRunParams): Promise; } diff --git a/packages/mcp-server/src/server.ts b/packages/mcp-server/src/server.ts index 6b2cc45a7c..e52040072f 100644 --- a/packages/mcp-server/src/server.ts +++ b/packages/mcp-server/src/server.ts @@ -36,9 +36,12 @@ import declareDescribeCollectionTool from './tools/describe-collection'; import declareDissociateTool from './tools/dissociate'; import declareExecuteActionTool from './tools/execute-action'; import declareGetActionFormTool from './tools/get-action-form'; +import declareGetWorkflowRunTool from './tools/get-workflow-run'; import declareListTool from './tools/list'; import declareListRelatedTool from './tools/list-related'; +import declareListWorkflowsTool from './tools/list-workflows'; import declareRequestActionFileUploadTool from './tools/request-action-file-upload'; +import declareTriggerWorkflowTool from './tools/trigger-workflow'; import declareUpdateTool from './tools/update'; import normalizeAgentUrl from './utils/normalize-agent-url'; import normalizeDomainList from './utils/normalize-domain-list'; @@ -97,6 +100,9 @@ const SAFE_ARGUMENTS_FOR_LOGGING: Record = { requestActionFileUpload: ['mimeType'], associate: ['collectionName', 'relationName', 'parentRecordId', 'targetRecordId'], dissociate: ['collectionName', 'relationName', 'parentRecordId', 'targetRecordIds'], + listWorkflows: ['collectionName'], + triggerWorkflow: ['workflowId', 'recordId'], + getWorkflowRun: ['runId'], }; export type ToolName = @@ -110,6 +116,9 @@ export type ToolName = | 'dissociate' | 'getActionForm' | 'executeAction' + | 'listWorkflows' + | 'triggerWorkflow' + | 'getWorkflowRun' | 'requestActionFileUpload'; /** @@ -283,6 +292,9 @@ export default class ForestMCPServer { { name: 'dissociate', register: () => declareDissociateTool(mcpServer, ctx) }, { name: 'getActionForm', register: () => declareGetActionFormTool(mcpServer, ctx) }, { name: 'executeAction', register: () => declareExecuteActionTool(mcpServer, ctx) }, + { name: 'listWorkflows', register: () => declareListWorkflowsTool(mcpServer, ctx) }, + { name: 'triggerWorkflow', register: () => declareTriggerWorkflowTool(mcpServer, ctx) }, + { name: 'getWorkflowRun', register: () => declareGetWorkflowRunTool(mcpServer, ctx) }, ...(this.fileUploads ? [ { @@ -328,6 +340,9 @@ export default class ForestMCPServer { 'dissociate', 'getActionForm', 'executeAction', + 'listWorkflows', + 'triggerWorkflow', + 'getWorkflowRun', 'requestActionFileUpload', ]; diff --git a/packages/mcp-server/src/tools/get-workflow-run.ts b/packages/mcp-server/src/tools/get-workflow-run.ts new file mode 100644 index 0000000000..65a450f7c7 --- /dev/null +++ b/packages/mcp-server/src/tools/get-workflow-run.ts @@ -0,0 +1,80 @@ +import type { ToolContext } from '../tool-context'; +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; + +import { z } from 'zod'; + +import getAuthContext from '../utils/auth-context'; +import registerToolWithLogging from '../utils/tool-with-logging'; +import toModelSafeError from '../utils/workflow-error'; + +const RUN_ID_DESCRIPTION = 'The id of the workflow run to observe, as returned by triggerWorkflow.'; + +function runUnavailableMessage(runId: string): string { + return ( + `Run "${runId}" could not be read because Forest could not be reached. This is a temporary ` + + 'server-side failure, not a problem with the run id — retry later, and report it to your ' + + 'Forest administrator if it persists.' + ); +} + +interface GetWorkflowRunArgument { + runId: string; +} + +export default function declareGetWorkflowRunTool(mcpServer: McpServer, ctx: ToolContext): string { + const { forestServerClient, logger } = ctx; + + return registerToolWithLogging( + mcpServer, + 'getWorkflowRun', + { + // Spelled out rather than relying on defaults: the MCP spec defaults an omitted + // destructiveHint to true, so a client reading that field alone would treat these + // reads as destructive. + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }, + title: 'Get a workflow run status', + description: + 'Poll a workflow run started with triggerWorkflow. Returns the full hydrated run: its ' + + 'runState (started, pending, loading, aborted or finished) plus the complete ' + + 'workflowHistory — every step with its resolved definition (type, title, prompt, task ' + + 'type, outgoing branches) and its per-step context (completion, selected option, error, ' + + 'escalation state, awaiting-input reason). Use it to see exactly where the run is and ' + + 'what each step does. A run parked on a human-gated step cannot be resumed via MCP in ' + + 'v1 and must be finished from the Forest UI. Poll at a reasonable interval: wait at ' + + 'least a few seconds between calls, and do not busy-loop on a long-running or ' + + 'human-gated run (which never resolves via MCP in v1).', + inputSchema: { + runId: z.string().min(1).describe(RUN_ID_DESCRIPTION), + }, + }, + async (args: GetWorkflowRunArgument, extra) => { + const { forestServerToken, renderingId } = getAuthContext(extra); + + let runStatus; + + try { + runStatus = await forestServerClient.getMcpWorkflowRun({ + forestServerToken, + renderingId, + runId: args.runId, + }); + } catch (error) { + // Same rule as the two sibling workflow tools: Forest's own refusal is worth reading, a + // raw transport failure is transport detail the model must never receive. + throw toModelSafeError(error, { + logger, + context: `Failed to read workflow run "${args.runId}"`, + unavailableMessage: runUnavailableMessage(args.runId), + }); + } + + return { content: [{ type: 'text', text: JSON.stringify(runStatus) }] }; + }, + logger, + ); +} diff --git a/packages/mcp-server/src/tools/list-workflows.ts b/packages/mcp-server/src/tools/list-workflows.ts new file mode 100644 index 0000000000..7389da3ce3 --- /dev/null +++ b/packages/mcp-server/src/tools/list-workflows.ts @@ -0,0 +1,97 @@ +import type { ToolContext } from '../tool-context'; +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; + +import { z } from 'zod'; + +import getAuthContext from '../utils/auth-context'; +import registerToolWithLogging from '../utils/tool-with-logging'; +import toModelSafeError from '../utils/workflow-error'; + +const LISTING_UNAVAILABLE_MESSAGE = + 'The MCP-enabled workflows could not be listed because Forest could not be reached. This is a ' + + 'temporary server-side failure — retry later, and report it to your Forest administrator if it ' + + 'persists.'; + +const COLLECTION_NAME_DESCRIPTION = + 'Optional. Narrow the results to workflows operating on this collection — typically the ' + + 'collection of the record currently in context.'; + +function createListWorkflowsArgumentShape(collectionNames: string[]) { + const collectionName = + collectionNames.length > 0 ? z.enum(collectionNames as [string, ...string[]]) : z.string(); + + return { + collectionName: collectionName.optional().describe(COLLECTION_NAME_DESCRIPTION), + }; +} + +type ListWorkflowsArgument = z.infer< + z.ZodObject> +>; + +export default function declareListWorkflowsTool(mcpServer: McpServer, ctx: ToolContext): string { + const { forestServerClient, logger, collectionNames } = ctx; + + return registerToolWithLogging( + mcpServer, + 'listWorkflows', + { + // Spelled out rather than relying on defaults: the MCP spec defaults an omitted + // destructiveHint to true, so a client reading that field alone would treat these + // reads as destructive. + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }, + title: 'List MCP-enabled workflows', + description: + 'Discover Forest workflows enabled for MCP triggering that you can access. Returns each ' + + "workflow's id, name and the collection it operates on. Optionally filter by collectionName " + + 'to match the record currently in context, then start one with triggerWorkflow.', + inputSchema: createListWorkflowsArgumentShape(collectionNames), + }, + async (args: ListWorkflowsArgument, extra) => { + const { forestServerToken, renderingId } = getAuthContext(extra); + + let workflows; + + try { + workflows = await forestServerClient.listMcpEnabledWorkflows({ + forestServerToken, + renderingId, + collectionName: args.collectionName, + }); + } catch (error) { + // Forest's own refusals still reach the model — they say something actionable. What must + // not is a raw transport failure: its message carries the Forest server URL and an internal + // host and port, and this result is stringified straight into a model's context. + throw toModelSafeError(error, { + logger, + context: 'Failed to list MCP-enabled workflows', + unavailableMessage: LISTING_UNAVAILABLE_MESSAGE, + }); + } + + // A workflow whose collection was renamed or removed cannot be triggered - triggerWorkflow + // rejects a null collectionName up front - so listing it would only send the model round the + // discover, trigger, rejected, discover loop. Surface the drop to the operator instead. + // `!= null` on purpose, to match triggerWorkflow's own guard: an absent key and an explicit + // null must be dropped by the same rule, or the discover/trigger loop reopens. + const triggerable = workflows.filter(workflow => workflow.collectionName != null); + const droppedCount = workflows.length - triggerable.length; + + if (droppedCount > 0) { + logger( + 'Warn', + `listWorkflows hid ${droppedCount} MCP-enabled workflow(s) whose collection is no longer ` + + 'available in this rendering; they cannot be triggered until their configuration is fixed.', + ); + } + + return { content: [{ type: 'text', text: JSON.stringify(triggerable) }] }; + }, + logger, + ); +} diff --git a/packages/mcp-server/src/tools/trigger-workflow.ts b/packages/mcp-server/src/tools/trigger-workflow.ts new file mode 100644 index 0000000000..601fea6b24 --- /dev/null +++ b/packages/mcp-server/src/tools/trigger-workflow.ts @@ -0,0 +1,223 @@ +import type { McpWorkflowLookup } from '../http-client'; +import type { ToolContext } from '../tool-context'; +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; + +import { NotFoundError } from '@forestadmin/forestadmin-client'; +import { z } from 'zod'; + +import getAuthContext from '../utils/auth-context'; +import registerToolWithLogging from '../utils/tool-with-logging'; +import withActivityLog from '../utils/with-activity-log'; +import { RETRY_WILL_NOT_HELP, carriesTransportDetail, isRetryable } from '../utils/workflow-error'; + +const WORKFLOW_ID_DESCRIPTION = + 'The id of the workflow to start, as returned by listWorkflows. The workflow must have the MCP ' + + 'trigger enabled.'; + +const RECORD_ID_DESCRIPTION = + 'The id of the record to run the workflow on. For collections with a composite primary key, ' + + 'use the packed id form (values joined by "|").'; + +interface TriggerWorkflowArgument { + workflowId: string; + recordId: string; +} + +// One message for the unknown, MCP-disabled and trigger-time-404 cases, so the LLM-facing +// contract stays uniform and never leaks whether a given workflow exists. The last sentence exists +// so a model cannot loop: an orchestrator that predates (or was rolled back before) the by-id +// endpoint 404s every lookup while listWorkflows keeps returning the same ids. +function notMcpEnabledMessage(workflowId: string): string { + return ( + `Workflow "${workflowId}" is not an MCP-enabled workflow you can access. ` + + 'Use listWorkflows to discover triggerable workflows. If listWorkflows just returned this id, ' + + 'do not retry — report it to your Forest administrator instead.' + ); +} + +// Distinct from the uniform 404 message: the workflow may well exist and be triggerable, Forest +// just could not be reached. Retrying later is the right advice, and the transport detail stays in +// the operator log rather than in the model's context. Reserved for genuinely retryable failures — +// see isRetryable: a 4xx handed this message would make the model retry an id that can never work. +function lookupUnavailableMessage(workflowId: string): string { + return ( + `Workflow "${workflowId}" could not be resolved because Forest could not be reached. ` + + 'This is a temporary server-side failure, not a problem with the workflow id — retry later, ' + + 'and report it to your Forest administrator if it persists.' + ); +} + +// Forest refused the lookup for a reason that will not change on retry, and said why. Reached only +// for an HttpError — isRetryable treats everything else as retryable — so `detail` is either the +// server's own JSON:API message or a fixed string, never transport detail. Keeping it distinguishes +// a malformed id, a refused identity and a browser-engine environment from one another, instead of +// having all three claim the workflow is not MCP-enabled. +function terminalLookupMessage(workflowId: string, detail: string): string { + const reason = detail.replace(/\.$/, ''); + + return `Workflow "${workflowId}" could not be resolved — ${reason}. ${RETRY_WILL_NOT_HELP}`; +} + +// The trigger is not idempotent and the write may have landed before the transport failed, so this +// deliberately does not tell the model to retry: a blind retry either duplicates intent or hits the +// active-run conflict, and neither is something a model should decide on its own. +function triggerUnavailableMessage(workflowId: string): string { + return ( + `Workflow "${workflowId}" could not be triggered because Forest could not be reached. The run ` + + 'may or may not have started — do not retry blindly; report it to your Forest administrator.' + ); +} + +function unavailableCollectionMessage(workflowId: string): string { + return ( + `Workflow "${workflowId}" cannot be triggered via MCP because its collection is unavailable. ` + + "Check the workflow's configuration in Forest." + ); +} + +export default function declareTriggerWorkflowTool(mcpServer: McpServer, ctx: ToolContext): string { + const { forestServerClient, logger } = ctx; + + return registerToolWithLogging( + mcpServer, + 'triggerWorkflow', + { + annotations: { + readOnlyHint: false, + destructiveHint: true, + idempotentHint: false, + openWorldHint: true, + }, + title: 'Trigger a workflow', + description: + 'Start an MCP-enabled Forest workflow on a specific record. Returns a runId immediately; ' + + 'the run continues asynchronously — poll getWorkflowRun to observe its status. The record ' + + 'is not validated at trigger time: an invalid record surfaces later via getWorkflowRun. ' + + 'Discover triggerable workflows with listWorkflows first.', + inputSchema: { + workflowId: z.string().min(1).describe(WORKFLOW_ID_DESCRIPTION), + // 255 matches the server's selectedRecordId column, so an over-long id is rejected here + // rather than after the pending audit row has been written. + recordId: z.string().min(1).max(255).describe(RECORD_ID_DESCRIPTION), + }, + }, + async (args: TriggerWorkflowArgument, extra) => { + const { forestServerToken, renderingId } = getAuthContext(extra); + + // Resolve the workflow by id up front so the audit log can be labelled BEFORE the trigger + // (fail-closed). Reject unknown or MCP-disabled workflows here — no run, no log. + let workflow: McpWorkflowLookup; + + try { + workflow = await forestServerClient.getMcpWorkflowById({ + forestServerToken, + renderingId, + workflowId: args.workflowId, + }); + } catch (error) { + // Always log: a 404 here is indistinguishable from a missing route, so without this an + // orchestrator too old to serve the lookup is visible only to the model. + logger( + 'Error', + `Failed to resolve workflow "${args.workflowId}" via getMcpWorkflowById: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + + // A 404 keeps the uniform wording on purpose: unknown, MCP-disabled and out-of-rendering + // must stay indistinguishable, or the contract leaks whether a given workflow exists. + if (error instanceof NotFoundError) { + throw new Error(notMcpEnabledMessage(args.workflowId)); + } + + // Any other non-retryable refusal (a 400 on a malformed id, a 401/403 on the identity, a + // 409 on a browser-engine environment) fails identically on every attempt. Sending those + // down the "retry later" path is what made a model loop forever on a request that can + // never succeed — typically a workflow *name* it guessed instead of the id. + if (!isRetryable(error)) { + throw new Error( + terminalLookupMessage( + args.workflowId, + error instanceof Error ? error.message : String(error), + ), + ); + } + + // Retryable (5xx, timeout, ECONNREFUSED). Its message carries transport detail — the Forest + // server URL, an internal host and port — that has no business in a model's context, so + // only the fixed sentence travels. The full error is in the operator log above. + throw new Error(lookupUnavailableMessage(args.workflowId)); + } + + if (!workflow.mcpEnabled) { + throw new Error(notMcpEnabledMessage(args.workflowId)); + } + + // A renamed/deleted collection leaves collectionName null, which the audit route rejects + // (collectionModelName is required). Fail-closed would then block with a misleading message, + // so reject up front with one that names the actual problem. + if (workflow.collectionName == null) { + throw new Error(unavailableCollectionMessage(args.workflowId)); + } + + const result = await withActivityLog({ + forestServerClient, + request: extra, + action: 'triggerWorkflow', + context: { + collectionName: workflow.collectionName, + recordId: args.recordId, + // One trigger writes two Activity Logs rows, and this is the first: it records the + // request, before the run exists. The orchestrator writes the second once the run is + // committed, labelled "triggered ... via MCP". The two deliberately do not share a + // wording — this one cannot carry a runId (that is what makes it fail-closed), and the + // second is best-effort, so a successful trigger can leave only this one. Identical + // labels made a single trigger read as two indistinguishable events, and made "how many + // workflows did assistants start?" answerable only by deduplicating on the runId. + label: `requested the workflow "${workflow.name}" via MCP`, + }, + logger, + operation: () => + forestServerClient.triggerMcpWorkflow({ + forestServerToken, + renderingId, + workflowId: args.workflowId, + recordId: args.recordId, + }), + errorEnhancer: async (parsedMessage, originalError) => { + // Guard the race where the workflow is disabled/deleted between the lookup and the + // trigger. + if (originalError instanceof NotFoundError) { + return notMcpEnabledMessage(args.workflowId); + } + + // withActivityLog rethrows the parsed message as-is, so without this the raw transport + // failure of the start call itself reaches the model — the same leak the lookup above + // guards, one call later. + if (carriesTransportDetail(originalError)) { + logger( + 'Error', + `Failed to trigger workflow "${args.workflowId}": ${ + originalError instanceof Error ? originalError.message : String(originalError) + }`, + ); + + return triggerUnavailableMessage(args.workflowId); + } + + return parsedMessage; + }, + }); + + return { + content: [ + { + type: 'text', + text: JSON.stringify({ runId: result.runId, runState: result.runState }), + }, + ], + }; + }, + logger, + ); +} diff --git a/packages/mcp-server/src/utils/activity-logs-creator.ts b/packages/mcp-server/src/utils/activity-logs-creator.ts index e245c5d28d..3867173802 100644 --- a/packages/mcp-server/src/utils/activity-logs-creator.ts +++ b/packages/mcp-server/src/utils/activity-logs-creator.ts @@ -8,10 +8,22 @@ import type { Logger } from '../server'; import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol.js'; import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types.js'; -import { NotFoundError } from '@forestadmin/forestadmin-client'; +import { HttpError, NotFoundError } from '@forestadmin/forestadmin-client'; + +import getAuthContext from './auth-context'; +import { carriesTransportDetail } from './workflow-error'; export type { ActivityLogAction, ActivityLogResponse }; +/** + * Fail policy for the audit trail, keyed by action type: a write whose activity log cannot be + * created is blocked (no unaudited side effect), while a read proceeds with a warning (an audit + * store outage must not take down the read surface). Both the refusal/outage path and the + * 200-with-no-id path in `createPendingActivityLog` are arbitrated by this map. + * + * One case is arbitrated by the cause instead of the action type: an authorization refusal + * (401/403) propagates for reads too — see `isAuthorizationRefusal`. + */ const ACTION_TO_TYPE: Record = { index: 'read', search: 'read', @@ -22,27 +34,28 @@ const ACTION_TO_TYPE: Record = { delete: 'write', listRelatedData: 'read', describeCollection: 'read', + triggerWorkflow: 'write', }; -function getAuthContext(request: RequestHandlerExtra): { - forestServerToken: string; - renderingId: string; -} { - const forestServerToken = request.authInfo?.extra?.forestServerToken; - const renderingId = request.authInfo?.extra?.renderingId; - - if (!forestServerToken || typeof forestServerToken !== 'string') { - throw new Error('Invalid or missing forestServerToken in authentication context'); - } - - // renderingId can be number (from JWT) or string - convert to string for API calls - if (renderingId === undefined || renderingId === null) { - throw new Error('Invalid or missing renderingId in authentication context'); - } - - return { forestServerToken, renderingId: String(renderingId) }; +/** + * The caller's identity was rejected (401/403), which is not an audit outage: the read the caller + * is about to perform is not authorized either. Fail-open exists so a broken audit store cannot + * take down the read surface — it must not turn an authorization refusal into a silent warning. + */ +function isAuthorizationRefusal(error: unknown): boolean { + return error instanceof HttpError && (error.status === 401 || error.status === 403); } +/** + * Sent instead of the cause when a write is blocked by an audit failure whose message carries + * transport detail. Retrying is safe advice here, unlike a failure of the operation itself: this + * throws before the operation runs, so nothing has happened yet. + */ +export const AUDIT_UNAVAILABLE_MESSAGE = + 'Forest could not be reached to write the audit trail, so the operation was not performed. ' + + 'This is a temporary server-side failure — retry later, and report it to your Forest ' + + 'administrator if it persists.'; + export default async function createPendingActivityLog( forestServerClient: ForestServerClient, request: RequestHandlerExtra, @@ -52,21 +65,83 @@ export default async function createPendingActivityLog( recordId?: string | number; recordIds?: string[] | number[]; label?: string; + /** Used only to report why a read proceeded unaudited; never sent to the server. */ + logger?: Logger; }, ) { const type = ACTION_TO_TYPE[action]; + // Outside the fail policy below: a missing auth context is a caller bug, not an audit outage. const { forestServerToken, renderingId } = getAuthContext(request); - return forestServerClient.createMcpActivityLog({ - forestServerToken, - renderingId, - action, - type, - collectionName: extra?.collectionName, - recordId: extra?.recordId, - recordIds: extra?.recordIds, - label: extra?.label, - }); + let activityLog: ActivityLogResponse | undefined; + + try { + activityLog = await forestServerClient.createMcpActivityLog({ + forestServerToken, + renderingId, + action, + type, + collectionName: extra?.collectionName, + recordId: extra?.recordId, + recordIds: extra?.recordIds, + label: extra?.label, + }); + } catch (error) { + // The audit log was refused (400/404 — e.g. an unresolvable collection) or the store is + // unreachable (5xx, timeout, connection error). Both land here, and both are far more likely + // than the 200-with-null-id case below. + if (type === 'write' || isAuthorizationRefusal(error)) { + // A blocked write is reported to the caller, and for an MCP tool the caller is a model. An + // HttpError message is the server's own detail or a fixed string, both meant to be read; a + // timeout or a raw Node/superagent error instead carries the Forest server URL, an internal + // host:port or a TLS chain. Only the sanitized sentence may travel — the same split the + // workflow tools apply to their own calls, and what the 200-with-no-id branch below has + // always done. Without this the cause reaches the model verbatim. + if (carriesTransportDetail(error)) { + extra?.logger?.( + 'Error', + `Activity log for '${action}' could not be created: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + + throw new Error(AUDIT_UNAVAILABLE_MESSAGE); + } + + throw error; + } + + // Report the cause: without it every fail-open read logs the same sentence, and an operator + // cannot tell a validation refusal (act now) from a transient outage (wait). + extra?.logger?.( + 'Error', + `Activity log for '${action}' could not be created: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + + return null; + } + + // 200 with no id: the route answered but the audit store dropped the write. + if (activityLog?.id === null || activityLog?.id === undefined) { + if (type === 'write') { + throw new Error( + 'Failed to create activity log: the server returned no activity log id. ' + + 'Blocking the operation to preserve the audit trail.', + ); + } + + extra?.logger?.( + 'Error', + `Activity log for '${action}' could not be created: the server answered 200 with no ` + + 'activity log id, so the audit store dropped the write.', + ); + + return null; + } + + return activityLog; } interface UpdateActivityLogOptions { @@ -86,9 +161,19 @@ async function updateActivityLogStatus( ): Promise { const { forestServerClient, request, activityLog, status, logger } = options; - // Use optional chaining with fallback since we're in error handling context - // and don't want to throw a different error if auth context is missing - const forestServerToken = (request.authInfo?.extra?.forestServerToken as string) ?? ''; + // Read directly rather than through getAuthContext: this runs in a fire-and-forget path and must + // not throw a second, unrelated error. A missing token means the request cannot be authenticated, + // so skip the call rather than spend a round trip on a request that can only be refused. + const forestServerToken = request.authInfo?.extra?.forestServerToken; + + if (typeof forestServerToken !== 'string' || !forestServerToken) { + logger( + 'Error', + `Cannot update activity log status to '${status}': no forestServerToken in the auth context.`, + ); + + return; + } try { await forestServerClient.updateActivityLogStatus({ diff --git a/packages/mcp-server/src/utils/auth-context.ts b/packages/mcp-server/src/utils/auth-context.ts new file mode 100644 index 0000000000..941856e11b --- /dev/null +++ b/packages/mcp-server/src/utils/auth-context.ts @@ -0,0 +1,24 @@ +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol.js'; +import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types.js'; + +/** + * Extracts the caller's Forest identity from the MCP request auth context. + * Populated by the OAuth provider's `verifyAccessToken` (see `forest-oauth-provider.ts`). + */ +export default function getAuthContext( + request: RequestHandlerExtra, +): { forestServerToken: string; renderingId: string } { + const forestServerToken = request.authInfo?.extra?.forestServerToken; + const renderingId = request.authInfo?.extra?.renderingId; + + if (!forestServerToken || typeof forestServerToken !== 'string') { + throw new Error('Invalid or missing forestServerToken in authentication context'); + } + + // renderingId can be number (from JWT) or string - convert to string for API calls + if (renderingId === undefined || renderingId === null) { + throw new Error('Invalid or missing renderingId in authentication context'); + } + + return { forestServerToken, renderingId: String(renderingId) }; +} diff --git a/packages/mcp-server/src/utils/with-activity-log.ts b/packages/mcp-server/src/utils/with-activity-log.ts index 20eb05beb3..9a1e74bc14 100644 --- a/packages/mcp-server/src/utils/with-activity-log.ts +++ b/packages/mcp-server/src/utils/with-activity-log.ts @@ -40,20 +40,36 @@ export default async function withActivityLog(options: WithActivityLogOptions const { forestServerClient, request, action, context, logger, operation, errorEnhancer } = options; - // We want to create the activity log before executing the operation - // If activity log creation fails, we must prevent the execution of the operation + // The activity log is created before the operation runs, so intent is captured even when the + // operation itself fails. `createPendingActivityLog` owns the fail policy: it throws for a write + // whose log could not be created (blocking the operation) and for an authorization refusal on + // any action, and resolves to null for a read whose audit write was lost. + const activityLog = await createPendingActivityLog(forestServerClient, request, action, { + ...context, + logger, + }); - const activityLog = await createPendingActivityLog(forestServerClient, request, action, context); + // Read whose audit log could not be created (fail-open): proceed without status tracking. The + // cause was already logged by `createPendingActivityLog`. + if (!activityLog) { + logger( + 'Warn', + `Activity log for '${action}' was not persisted by the server; ` + + 'proceeding without audit trail for this read operation.', + ); + } try { const result = await operation(); - markActivityLogAsSucceeded({ - forestServerClient, - request, - activityLog, - logger, - }); + if (activityLog) { + markActivityLogAsSucceeded({ + forestServerClient, + request, + activityLog, + logger, + }); + } return result; } catch (error) { @@ -74,12 +90,14 @@ export default async function withActivityLog(options: WithActivityLogOptions } } - markActivityLogAsFailed({ - forestServerClient, - request, - activityLog, - logger, - }); + if (activityLog) { + markActivityLogAsFailed({ + forestServerClient, + request, + activityLog, + logger, + }); + } throw new Error(errorMessage); } diff --git a/packages/mcp-server/src/utils/workflow-error.ts b/packages/mcp-server/src/utils/workflow-error.ts new file mode 100644 index 0000000000..85d1b2496e --- /dev/null +++ b/packages/mcp-server/src/utils/workflow-error.ts @@ -0,0 +1,70 @@ +import type { Logger } from '../server'; + +import { HttpError } from '@forestadmin/forestadmin-client'; + +// A timeout is the one HttpError whose message is built client-side: ServerUtils interpolates the +// full Forest server URL into it. Every other HttpError message is either a fixed string or the +// server's own JSON:API detail, both of which are meant to be read. +const TIMEOUT_STATUS = 408; +const RATE_LIMITED_STATUS = 429; + +/** + * True when the error's message carries transport detail — the Forest server URL, an internal + * host:port, a TLS chain. Anything that is not an `HttpError` reached us as a raw Node/superagent + * error, so its message is unfit for a model's context. + */ +export function carriesTransportDetail(error: unknown): boolean { + return !(error instanceof HttpError) || error.status === TIMEOUT_STATUS; +} + +/** + * True when retrying the same call could plausibly succeed. A 4xx that is neither a timeout nor a + * rate-limit is the caller's own fault and will fail identically forever — telling a model to + * retry one of those is what turns a bad argument into an endless loop. + */ +export function isRetryable(error: unknown): boolean { + if (!(error instanceof HttpError)) return true; + + return ( + error.status < 400 || + error.status >= 500 || + error.status === TIMEOUT_STATUS || + error.status === RATE_LIMITED_STATUS + ); +} + +/** + * Closing advice for a refusal that will repeat. Shared so the three workflow tools tell a model + * the same thing — the wording is the only signal it has that retrying is pointless. + */ +export const RETRY_WILL_NOT_HELP = + 'Retrying will not help: fix the request, or report it to your Forest administrator.'; + +/** + * Turns a workflow transport failure into an error the model may see: the full cause always goes + * to the operator log, a message carrying transport detail is replaced by `unavailableMessage`, + * and a refusal that will repeat is told to stop. + */ +export default function toModelSafeError( + error: unknown, + { + logger, + context, + unavailableMessage, + }: { logger: Logger; context: string; unavailableMessage: string }, +): Error { + const detail = error instanceof Error ? error.message : String(error); + + logger('Error', `${context}: ${detail}`); + + if (carriesTransportDetail(error)) return new Error(unavailableMessage); + + // A 4xx that is neither a timeout nor a rate limit fails identically on every attempt. Handing a + // model the bare reason leaves nothing saying so — and these are the tools it is told to poll, so + // the loop it produces is the documented usage rather than a mistake. + if (!isRetryable(error)) { + return new Error(`${detail.replace(/\.$/, '')}. ${RETRY_WILL_NOT_HELP}`); + } + + return new Error(detail); +} diff --git a/packages/mcp-server/test/helpers/forest-server-client.ts b/packages/mcp-server/test/helpers/forest-server-client.ts index 74bc697dac..3809292984 100644 --- a/packages/mcp-server/test/helpers/forest-server-client.ts +++ b/packages/mcp-server/test/helpers/forest-server-client.ts @@ -14,6 +14,30 @@ export default function createMockForestServerClient( attributes: { index: 'mock-index' }, }), updateActivityLogStatus: jest.fn().mockResolvedValue(undefined), + listMcpEnabledWorkflows: jest.fn().mockResolvedValue([]), + getMcpWorkflowById: jest.fn().mockResolvedValue({ + workflowId: 'wf-1', + name: 'Refund order', + collectionName: 'orders', + mcpEnabled: true, + }), + triggerMcpWorkflow: jest.fn().mockResolvedValue({ runId: '1', runState: 'loading' }), + getMcpWorkflowRun: jest.fn().mockResolvedValue({ + id: 1, + userId: 1, + renderingId: 1, + collectionId: 'orders', + workflowId: 'wf-1', + bpmnVersion: '1', + selectedRecordId: '1', + runState: 'started', + engine: 'orchestrator', + triggerType: 'mcp', + lockedAt: null, + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + workflowHistory: [], + }), ...overrides, } as jest.Mocked; } diff --git a/packages/mcp-server/test/helpers/registered-tool-config.ts b/packages/mcp-server/test/helpers/registered-tool-config.ts index a1f6b25061..39f10cd1f8 100644 --- a/packages/mcp-server/test/helpers/registered-tool-config.ts +++ b/packages/mcp-server/test/helpers/registered-tool-config.ts @@ -2,5 +2,10 @@ export type RegisteredToolConfig = { title: string; description: string; inputSchema: unknown; - annotations?: { readOnlyHint?: boolean }; + annotations?: { + readOnlyHint?: boolean; + destructiveHint?: boolean; + idempotentHint?: boolean; + openWorldHint?: boolean; + }; }; diff --git a/packages/mcp-server/test/http-client/mcp-http-client.test.ts b/packages/mcp-server/test/http-client/mcp-http-client.test.ts index 9b904e1918..5ef51e983f 100644 --- a/packages/mcp-server/test/http-client/mcp-http-client.test.ts +++ b/packages/mcp-server/test/http-client/mcp-http-client.test.ts @@ -2,6 +2,7 @@ import type { ActivityLogsServiceInterface, ForestSchemaCollection, SchemaServiceInterface, + WorkflowsServiceInterface, } from '../../src/http-client/types'; import { createForestServerClient } from '../../src/http-client'; @@ -10,6 +11,7 @@ import ForestServerClientImpl from '../../src/http-client/mcp-http-client'; describe('ForestServerClientImpl', () => { let mockSchemaService: jest.Mocked; let mockActivityLogsService: jest.Mocked; + let mockWorkflowsService: jest.Mocked; let client: ForestServerClientImpl; beforeEach(() => { @@ -21,9 +23,16 @@ describe('ForestServerClientImpl', () => { createMcpActivityLog: jest.fn(), updateActivityLogStatus: jest.fn(), }; + mockWorkflowsService = { + listMcpEnabledWorkflows: jest.fn(), + getMcpWorkflowById: jest.fn(), + triggerMcpWorkflow: jest.fn(), + getMcpWorkflowRun: jest.fn(), + }; client = new ForestServerClientImpl( mockSchemaService, mockActivityLogsService, + mockWorkflowsService, 'https://api.forestadmin.com', ); }); @@ -102,6 +111,99 @@ describe('ForestServerClientImpl', () => { expect(mockActivityLogsService.updateActivityLogStatus).toHaveBeenCalledWith(params); }); }); + + describe('listMcpEnabledWorkflows', () => { + it('should delegate to workflowsService.listMcpEnabledWorkflows()', async () => { + const workflows = [{ workflowId: 'wf-1', name: 'Refund order', collectionName: 'orders' }]; + mockWorkflowsService.listMcpEnabledWorkflows.mockResolvedValue(workflows); + + const params = { + forestServerToken: 'test-token', + renderingId: '12345', + collectionName: 'orders', + }; + + const result = await client.listMcpEnabledWorkflows(params); + + expect(mockWorkflowsService.listMcpEnabledWorkflows).toHaveBeenCalledWith(params); + expect(result).toBe(workflows); + }); + }); + + describe('getMcpWorkflowById', () => { + it('should delegate to workflowsService.getMcpWorkflowById()', async () => { + const workflow = { + workflowId: 'wf-1', + name: 'Refund order', + collectionName: 'orders', + mcpEnabled: true, + }; + mockWorkflowsService.getMcpWorkflowById.mockResolvedValue(workflow); + + const params = { + forestServerToken: 'test-token', + renderingId: '12345', + workflowId: 'wf-1', + }; + + const result = await client.getMcpWorkflowById(params); + + expect(mockWorkflowsService.getMcpWorkflowById).toHaveBeenCalledWith(params); + expect(result).toBe(workflow); + }); + }); + + describe('triggerMcpWorkflow', () => { + it('should delegate to workflowsService.triggerMcpWorkflow()', async () => { + const run = { runId: '7', runState: 'loading' as const }; + mockWorkflowsService.triggerMcpWorkflow.mockResolvedValue(run); + + const params = { + forestServerToken: 'test-token', + renderingId: '12345', + workflowId: 'wf-1', + recordId: '42', + }; + + const result = await client.triggerMcpWorkflow(params); + + expect(mockWorkflowsService.triggerMcpWorkflow).toHaveBeenCalledWith(params); + expect(result).toBe(run); + }); + }); + + describe('getMcpWorkflowRun', () => { + it('should delegate to workflowsService.getMcpWorkflowRun()', async () => { + const hydratedRun = { + id: 7, + userId: 42, + renderingId: 12345, + collectionId: 'orders', + workflowId: 'wf-1', + bpmnVersion: '3', + selectedRecordId: '99', + runState: 'finished' as const, + engine: 'orchestrator' as const, + triggerType: 'mcp' as const, + lockedAt: null, + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:05:00.000Z', + workflowHistory: [], + }; + mockWorkflowsService.getMcpWorkflowRun.mockResolvedValue(hydratedRun); + + const params = { + forestServerToken: 'test-token', + renderingId: '12345', + runId: '7', + }; + + const result = await client.getMcpWorkflowRun(params); + + expect(mockWorkflowsService.getMcpWorkflowRun).toHaveBeenCalledWith(params); + expect(result).toBe(hydratedRun); + }); + }); }); describe('createForestServerClient', () => { @@ -133,5 +235,9 @@ describe('createForestServerClient', () => { expect(client.createActivityLog).toBeDefined(); expect(client.createMcpActivityLog).toBeDefined(); expect(client.updateActivityLogStatus).toBeDefined(); + expect(client.listMcpEnabledWorkflows).toBeDefined(); + expect(client.getMcpWorkflowById).toBeDefined(); + expect(client.triggerMcpWorkflow).toBeDefined(); + expect(client.getMcpWorkflowRun).toBeDefined(); }); }); diff --git a/packages/mcp-server/test/server.test.ts b/packages/mcp-server/test/server.test.ts index a31c3bf077..d84bc98b79 100644 --- a/packages/mcp-server/test/server.test.ts +++ b/packages/mcp-server/test/server.test.ts @@ -3642,6 +3642,9 @@ describe('enabledTools', () => { 'dissociate', 'getActionForm', 'executeAction', + 'listWorkflows', + 'triggerWorkflow', + 'getWorkflowRun', 'requestActionFileUpload', ], }); @@ -3671,6 +3674,102 @@ describe('enabledTools', () => { ); }); + // Registration, the ToolName union and allToolNames are three separate edits in server.ts; the + // union and the list are type-checked, the registration call is not. Without this, dropping the + // registerTool line leaves a tool that is advertised as available, never registered, and green. + it('should register the three workflow tools by default, with their annotations', async () => { + const savedFetch3 = global.fetch; + const savedPort3 = process.env.MCP_SERVER_PORT; + process.env.MCP_SERVER_PORT = (await getAvailablePort()).toString(); + const mockServer3 = new MockServer(); + mockServer3 + .get('/liana/environment', { + data: { id: '12345', attributes: { api_endpoint: 'https://api.example.com' } }, + }) + .get('/liana/forest-schema', { + data: [ + { + id: 'users', + type: 'collections', + attributes: { name: 'users', fields: [{ field: 'id', type: 'Number' }] }, + }, + ], + included: [], + meta: { liana: 'forest-express-sequelize', liana_version: '9.0.0', liana_features: null }, + }) + .get(/\/oauth\/register\//, { error: 'Client not found' }, 404); + + global.fetch = mockServer3.fetch; + + // No enabledTools: exercises the default-on path the release notes describe. + const defaultServer = new ForestMCPServer({ + envSecret: 'test-env-secret', + authSecret: 'test-auth-secret', + }); + + const defaultApp = await defaultServer.buildExpressApp(); + const defaultHttpServer = defaultApp.listen(Number(process.env.MCP_SERVER_PORT)) as http.Server; + + await new Promise(resolve => { + defaultHttpServer.on('listening', resolve); + }); + + const validToken = jsonwebtoken.sign( + { id: 123, email: 'user@example.com', renderingId: 456 }, + 'test-auth-secret', + { expiresIn: '1h' }, + ); + + const response = await request(defaultHttpServer) + .post('/mcp') + .set('Authorization', `Bearer ${validToken}`) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json, text/event-stream') + .send({ jsonrpc: '2.0', method: 'tools/list', id: 1 }); + + expect(response.status).toBe(200); + + let responseData: { + result: { tools: Array<{ name: string; annotations?: Record }> }; + }; + + if (response.body && Object.keys(response.body).length > 0) { + responseData = response.body; + } else { + const dataLine = response.text.split('\n').find((line: string) => line.startsWith('data: ')); + if (!dataLine) throw new Error('Expected SSE data line not found in response'); + responseData = JSON.parse(dataLine.replace('data: ', '')); + } + + const { tools } = responseData.result; + const toolNames = tools.map(t => t.name); + + expect(toolNames).toContain('listWorkflows'); + expect(toolNames).toContain('triggerWorkflow'); + expect(toolNames).toContain('getWorkflowRun'); + + // The annotations are what lets a client tell the side-effectful tool from the two reads, so + // they travel over the wire or they do not exist. + expect(tools.find(t => t.name === 'triggerWorkflow')?.annotations).toMatchObject({ + readOnlyHint: false, + destructiveHint: true, + idempotentHint: false, + openWorldHint: true, + }); + expect(tools.find(t => t.name === 'listWorkflows')?.annotations).toMatchObject({ + readOnlyHint: true, + }); + expect(tools.find(t => t.name === 'getWorkflowRun')?.annotations).toMatchObject({ + readOnlyHint: true, + }); + + await new Promise(resolve => { + defaultHttpServer.close(() => resolve()); + }); + global.fetch = savedFetch3; + process.env.MCP_SERVER_PORT = savedPort3; + }); + it('should only expose describeCollection when enabledTools is empty', async () => { const savedFetch2 = global.fetch; const savedPort2 = process.env.MCP_SERVER_PORT; diff --git a/packages/mcp-server/test/tools/execute-action.test.ts b/packages/mcp-server/test/tools/execute-action.test.ts index 9b8bab74d0..8708a2768f 100644 --- a/packages/mcp-server/test/tools/execute-action.test.ts +++ b/packages/mcp-server/test/tools/execute-action.test.ts @@ -22,6 +22,10 @@ const mockForestServerClient: ForestServerClient = { createActivityLog: jest.fn(), createMcpActivityLog: jest.fn(), updateActivityLogStatus: jest.fn(), + listMcpEnabledWorkflows: jest.fn(), + getMcpWorkflowById: jest.fn(), + triggerMcpWorkflow: jest.fn(), + getMcpWorkflowRun: jest.fn(), }; const mockBuildClientWithActions = buildClientWithActions as jest.MockedFunction< diff --git a/packages/mcp-server/test/tools/get-action-form.test.ts b/packages/mcp-server/test/tools/get-action-form.test.ts index 9102810b26..275470f39a 100644 --- a/packages/mcp-server/test/tools/get-action-form.test.ts +++ b/packages/mcp-server/test/tools/get-action-form.test.ts @@ -17,6 +17,10 @@ const mockForestServerClient: ForestServerClient = { createActivityLog: jest.fn(), createMcpActivityLog: jest.fn(), updateActivityLogStatus: jest.fn(), + listMcpEnabledWorkflows: jest.fn(), + getMcpWorkflowById: jest.fn(), + triggerMcpWorkflow: jest.fn(), + getMcpWorkflowRun: jest.fn(), }; const mockBuildClientWithActions = buildClientWithActions as jest.MockedFunction< diff --git a/packages/mcp-server/test/tools/get-workflow-run.test.ts b/packages/mcp-server/test/tools/get-workflow-run.test.ts new file mode 100644 index 0000000000..143633a47b --- /dev/null +++ b/packages/mcp-server/test/tools/get-workflow-run.test.ts @@ -0,0 +1,307 @@ +import type { ForestServerClient } from '../../src/http-client'; +import type { Logger } from '../../src/server'; +import type { RegisteredToolConfig } from '../helpers/registered-tool-config'; +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; +import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; + +import { ForbiddenError, HttpError, NotFoundError } from '@forestadmin/forestadmin-client'; + +import declareGetWorkflowRunTool from '../../src/tools/get-workflow-run'; +import createMockForestServerClient from '../helpers/forest-server-client'; + +const mockLogger: Logger = jest.fn(); + +describe('declareGetWorkflowRunTool', () => { + let mcpServer: McpServer; + let mockForestServerClient: jest.Mocked; + let registeredToolHandler: (args: unknown, extra: unknown) => Promise; + let registeredToolConfig: RegisteredToolConfig; + + beforeEach(() => { + jest.clearAllMocks(); + + mockForestServerClient = createMockForestServerClient(); + + mcpServer = { + registerTool: jest.fn((name, config, handler) => { + registeredToolConfig = config; + registeredToolHandler = handler; + }), + } as unknown as McpServer; + }); + + describe('tool registration', () => { + it('should register a tool named "getWorkflowRun"', () => { + declareGetWorkflowRunTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + expect(mcpServer.registerTool).toHaveBeenCalledWith( + 'getWorkflowRun', + expect.any(Object), + expect.any(Function), + ); + }); + + it('should register tool with correct title and description', () => { + declareGetWorkflowRunTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + expect(registeredToolConfig.title).toBe('Get a workflow run status'); + expect(registeredToolConfig.description).toContain('workflowHistory'); + expect(registeredToolConfig.description).toContain('cannot be resumed via MCP'); + expect(registeredToolConfig.description).toContain('wait at least a few seconds'); + }); + + it('should spell out every annotation, not just readOnlyHint', () => { + declareGetWorkflowRunTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + // An omitted destructiveHint defaults to true in the MCP spec, so a client reading + // that field alone would treat this read as destructive. + expect(registeredToolConfig.annotations).toEqual({ + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }); + }); + + it('should require a string runId argument', () => { + declareGetWorkflowRunTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + const schema = registeredToolConfig.inputSchema as Record< + string, + { parse: (value: unknown) => unknown } + >; + + expect(() => schema.runId.parse('7')).not.toThrow(); + expect(() => schema.runId.parse(undefined)).toThrow(); + expect(() => schema.runId.parse(7)).toThrow(); + expect(() => schema.runId.parse('')).toThrow(); + }); + }); + + describe('tool execution', () => { + const mockExtra = { + authInfo: { + token: 'test-token', + extra: { + forestServerToken: 'forest-token', + renderingId: 123, + environmentApiEndpoint: 'https://api.example.com', + }, + }, + } as unknown as RequestHandlerExtra; + + const hydratedRun = { + id: 7, + userId: 42, + renderingId: 123, + collectionId: 'orders', + workflowId: 'wf-1', + bpmnVersion: '3', + selectedRecordId: '99', + runState: 'finished' as const, + engine: 'orchestrator' as const, + triggerType: 'mcp' as const, + lockedAt: null, + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:05:00.000Z', + workflowHistory: [ + { + stepName: 'Refund order', + stepIndex: 0, + done: true, + isCardStep: false, + context: { manuallyCompleted: true as const, completedBy: 42 }, + stepDefinition: { + type: 'task' as const, + title: 'Refund order', + executionType: 'fully-automated' as const, + automaticCompletion: true, + outgoing: [], + taskType: 'trigger-action' as const, + }, + }, + ], + }; + + beforeEach(() => { + declareGetWorkflowRunTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + mockForestServerClient.getMcpWorkflowRun.mockResolvedValue(hydratedRun); + }); + + it('should call getWorkflowRun with the identity from the auth context and the runId', async () => { + await registeredToolHandler({ runId: '7' }, mockExtra); + + expect(mockForestServerClient.getMcpWorkflowRun).toHaveBeenCalledWith({ + forestServerToken: 'forest-token', + renderingId: '123', + runId: '7', + }); + }); + + it('should return the full hydrated run as JSON text content, unchanged', async () => { + const result = await registeredToolHandler({ runId: '7' }, mockExtra); + + expect(result).toEqual({ + content: [{ type: 'text', text: JSON.stringify(hydratedRun) }], + }); + }); + + it('should surface a human-gated step and its context verbatim', async () => { + const gatedRun = { + ...hydratedRun, + runState: 'started' as const, + workflowHistory: [ + { + stepName: 'Manager approval', + stepIndex: 0, + done: false, + isCardStep: true, + context: { escalationState: 'escalated' as const }, + stepDefinition: { + type: 'escalation' as const, + title: 'Manager approval', + executionType: 'manual' as const, + automaticCompletion: false, + outgoing: [{ stepId: 'end', buttonText: 'Approve' }], + }, + }, + ], + }; + mockForestServerClient.getMcpWorkflowRun.mockResolvedValue(gatedRun); + + const result = await registeredToolHandler({ runId: '7' }, mockExtra); + + expect(result).toEqual({ + content: [{ type: 'text', text: JSON.stringify(gatedRun) }], + }); + }); + + it('should return an error result when the auth context is missing the token', async () => { + const extraWithoutToken = { + authInfo: { extra: { renderingId: 123 } }, + } as unknown as RequestHandlerExtra; + + const result = await registeredToolHandler({ runId: '7' }, extraWithoutToken); + + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('forestServerToken') }], + isError: true, + }); + expect(mockForestServerClient.getMcpWorkflowRun).not.toHaveBeenCalled(); + }); + + it('should map an unknown runId 404 to an error tool result', async () => { + mockForestServerClient.getMcpWorkflowRun.mockRejectedValue( + new NotFoundError('Workflow run not found'), + ); + + const result = await registeredToolHandler({ runId: '7' }, mockExtra); + + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('not found') }], + isError: true, + }); + }); + + it('should map a forbidden runId 403 to an error tool result', async () => { + mockForestServerClient.getMcpWorkflowRun.mockRejectedValue( + new ForbiddenError('You are not allowed to access this workflow run'), + ); + + const result = await registeredToolHandler({ runId: '7' }, mockExtra); + + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('not allowed') }], + isError: true, + }); + }); + + it.each([ + ['a raw connection error', new Error('connect ECONNREFUSED 10.0.4.17:3310')], + [ + 'a timeout naming the server URL', + new HttpError( + 'The request to Forest Admin server has timed out while trying to reach ' + + 'https://api.internal.acme.corp/api/workflow-orchestrator/mcp-workflows/runs/7 at ' + + '2026-08-19T13:00:00.000Z. Message: Timeout of 10000ms exceeded', + 408, + ), + ], + ])('should keep transport detail out of the tool result on %s', async (_, error) => { + mockForestServerClient.getMcpWorkflowRun.mockRejectedValue(error); + + const result = await registeredToolHandler({ runId: '7' }, mockExtra); + const { text } = (result as { content: [{ text: string }] }).content[0]; + + expect(result).toMatchObject({ isError: true }); + expect(text).toBe( + 'Run "7" could not be read because Forest could not be reached. This is a temporary ' + + 'server-side failure, not a problem with the run id — retry later, and report it to ' + + 'your Forest administrator if it persists.', + ); + expect(text).not.toContain('10.0.4.17'); + expect(text).not.toContain('acme.corp'); + expect(mockLogger).toHaveBeenCalledWith( + 'Error', + `Failed to read workflow run "7": ${error.message}`, + ); + }); + + // This is the tool the model is told to poll, so a refusal it cannot fix has to say so or the + // documented usage becomes an endless loop. + it.each([ + ['an unknown runId', new NotFoundError('Workflow run not found')], + ['a refused identity', new ForbiddenError('You are not allowed to access this workflow run')], + ['a malformed runId', new HttpError('Validation failed', 400)], + [ + 'a browser-engine environment', + new HttpError('This environment runs workflows in the browser', 409), + ], + ])('should tell the model not to retry %s', async (_, error) => { + mockForestServerClient.getMcpWorkflowRun.mockRejectedValue(error); + + const result = await registeredToolHandler({ runId: '7' }, mockExtra); + const { text } = (result as { content: [{ text: string }] }).content[0]; + + expect(result).toMatchObject({ isError: true }); + expect(text).toBe( + `${error.message.replace(/\.$/, '')}. Retrying will not help: fix the request, or report ` + + 'it to your Forest administrator.', + ); + }); + + it.each([ + ['a rate limit', new HttpError('Too many requests', 429)], + ['a server error', new HttpError('Internal server error', 500)], + ])('should leave %s retryable, with no advice to stop', async (_, error) => { + mockForestServerClient.getMcpWorkflowRun.mockRejectedValue(error); + + const result = await registeredToolHandler({ runId: '7' }, mockExtra); + const { text } = (result as { content: [{ text: string }] }).content[0]; + + expect(text).toBe(error.message); + expect(text).not.toContain('Retrying will not help'); + }); + }); +}); diff --git a/packages/mcp-server/test/tools/list-workflows.test.ts b/packages/mcp-server/test/tools/list-workflows.test.ts new file mode 100644 index 0000000000..5f630c6b03 --- /dev/null +++ b/packages/mcp-server/test/tools/list-workflows.test.ts @@ -0,0 +1,320 @@ +import type { ForestServerClient } from '../../src/http-client'; +import type { Logger } from '../../src/server'; +import type { RegisteredToolConfig } from '../helpers/registered-tool-config'; +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; +import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; + +import { HttpError, NotFoundError } from '@forestadmin/forestadmin-client'; + +import declareListWorkflowsTool from '../../src/tools/list-workflows'; +import createMockForestServerClient from '../helpers/forest-server-client'; + +const mockLogger: Logger = jest.fn(); + +describe('declareListWorkflowsTool', () => { + let mcpServer: McpServer; + let mockForestServerClient: jest.Mocked; + let registeredToolHandler: (args: unknown, extra: unknown) => Promise; + let registeredToolConfig: RegisteredToolConfig; + + beforeEach(() => { + jest.clearAllMocks(); + + mockForestServerClient = createMockForestServerClient(); + + mcpServer = { + registerTool: jest.fn((name, config, handler) => { + registeredToolConfig = config; + registeredToolHandler = handler; + }), + } as unknown as McpServer; + }); + + describe('tool registration', () => { + it('should register a tool named "listWorkflows"', () => { + declareListWorkflowsTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + expect(mcpServer.registerTool).toHaveBeenCalledWith( + 'listWorkflows', + expect.any(Object), + expect.any(Function), + ); + }); + + it('should register tool with correct title and description', () => { + declareListWorkflowsTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + expect(registeredToolConfig.title).toBe('List MCP-enabled workflows'); + expect(registeredToolConfig.description).toContain('MCP triggering'); + }); + + it('should spell out every annotation, not just readOnlyHint', () => { + declareListWorkflowsTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + // An omitted destructiveHint defaults to true in the MCP spec, so a client reading + // that field alone would treat this read as destructive. + expect(registeredToolConfig.annotations).toEqual({ + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }); + }); + + it('should expose an optional collectionName argument', () => { + declareListWorkflowsTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + const schema = registeredToolConfig.inputSchema as Record< + string, + { parse: (value: unknown) => unknown } + >; + expect(registeredToolConfig.inputSchema).toHaveProperty('collectionName'); + expect(schema.collectionName.parse(undefined)).toBeUndefined(); + }); + + it('should accept any string for collectionName when no collection names provided', () => { + declareListWorkflowsTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + const schema = registeredToolConfig.inputSchema as Record< + string, + { parse: (value: unknown) => unknown } + >; + expect(() => schema.collectionName.parse('any-collection')).not.toThrow(); + expect(() => schema.collectionName.parse(undefined)).not.toThrow(); + expect(() => schema.collectionName.parse(123)).toThrow(); + }); + + it('should restrict collectionName to the known collections when provided', () => { + declareListWorkflowsTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: ['orders', 'users'], + }); + + const schema = registeredToolConfig.inputSchema as Record< + string, + { parse: (value: unknown) => unknown } + >; + expect(() => schema.collectionName.parse('orders')).not.toThrow(); + expect(() => schema.collectionName.parse(undefined)).not.toThrow(); + expect(() => schema.collectionName.parse('invalid-collection')).toThrow(); + }); + }); + + describe('tool execution', () => { + const mockExtra = { + authInfo: { + token: 'test-token', + extra: { + forestServerToken: 'forest-token', + renderingId: 123, + environmentApiEndpoint: 'https://api.example.com', + }, + }, + } as unknown as RequestHandlerExtra; + + const workflows = [ + { workflowId: 'wf-1', name: 'Refund order', collectionName: 'orders' }, + { workflowId: 'wf-2', name: 'Notify customer', collectionName: 'orders' }, + ]; + + beforeEach(() => { + declareListWorkflowsTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + mockForestServerClient.listMcpEnabledWorkflows.mockResolvedValue(workflows); + }); + + it('should call listMcpEnabledWorkflows with the identity from the auth context', async () => { + await registeredToolHandler({}, mockExtra); + + expect(mockForestServerClient.listMcpEnabledWorkflows).toHaveBeenCalledWith({ + forestServerToken: 'forest-token', + renderingId: '123', + collectionName: undefined, + }); + }); + + it('should forward the collectionName filter to listMcpEnabledWorkflows', async () => { + await registeredToolHandler({ collectionName: 'orders' }, mockExtra); + + expect(mockForestServerClient.listMcpEnabledWorkflows).toHaveBeenCalledWith({ + forestServerToken: 'forest-token', + renderingId: '123', + collectionName: 'orders', + }); + }); + + it('should return the workflows as JSON text content', async () => { + const result = await registeredToolHandler({}, mockExtra); + + expect(result).toEqual({ + content: [{ type: 'text', text: JSON.stringify(workflows) }], + }); + }); + + it('should hide workflows whose collection is unavailable, since triggerWorkflow rejects them', async () => { + mockForestServerClient.listMcpEnabledWorkflows.mockResolvedValue([ + { workflowId: 'wf-1', name: 'Refund order', collectionName: 'orders' }, + { workflowId: 'wf-2', name: 'Retired process', collectionName: null }, + ]); + + const result = await registeredToolHandler({}, mockExtra); + + expect(result).toEqual({ + content: [ + { + type: 'text', + text: JSON.stringify([ + { workflowId: 'wf-1', name: 'Refund order', collectionName: 'orders' }, + ]), + }, + ], + }); + }); + + it('should warn the operator about each hidden workflow', async () => { + mockForestServerClient.listMcpEnabledWorkflows.mockResolvedValue([ + { workflowId: 'wf-1', name: 'Refund order', collectionName: 'orders' }, + { workflowId: 'wf-2', name: 'Retired process', collectionName: null }, + { workflowId: 'wf-3', name: 'Other retired', collectionName: null }, + ]); + + await registeredToolHandler({}, mockExtra); + + expect(mockLogger).toHaveBeenCalledWith( + 'Warn', + 'listWorkflows hid 2 MCP-enabled workflow(s) whose collection is no longer available in ' + + 'this rendering; they cannot be triggered until their configuration is fixed.', + ); + }); + + it('should not warn when every workflow is triggerable', async () => { + mockForestServerClient.listMcpEnabledWorkflows.mockResolvedValue([ + { workflowId: 'wf-1', name: 'Refund order', collectionName: 'orders' }, + ]); + + await registeredToolHandler({}, mockExtra); + + expect(mockLogger).not.toHaveBeenCalledWith( + 'Warn', + expect.stringContaining('listWorkflows hid'), + ); + }); + + it('should return an error result when the auth context is missing the token', async () => { + const extraWithoutToken = { + authInfo: { extra: { renderingId: 123 } }, + } as unknown as RequestHandlerExtra; + + const result = await registeredToolHandler({}, extraWithoutToken); + + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('forestServerToken') }], + isError: true, + }); + expect(mockForestServerClient.listMcpEnabledWorkflows).not.toHaveBeenCalled(); + }); + + it('should map server errors to an error tool result', async () => { + mockForestServerClient.listMcpEnabledWorkflows.mockRejectedValue( + new NotFoundError('No active workflow for the rendering'), + ); + + const result = await registeredToolHandler({}, mockExtra); + + expect(result).toEqual({ + content: [ + { type: 'text', text: expect.stringContaining('No active workflow for the rendering') }, + ], + isError: true, + }); + }); + + // This result is stringified straight into a model's context, so it is the one place a raw + // connection error must not reach: the model vendor would receive the private Forest endpoint. + it.each([ + ['a raw connection error', new Error('connect ECONNREFUSED 10.0.4.17:3310')], + [ + 'a timeout naming the server URL', + new HttpError( + 'The request to Forest Admin server has timed out while trying to reach ' + + 'https://api.internal.acme.corp/api/workflow-orchestrator/mcp-workflows at ' + + '2026-08-19T13:00:00.000Z. Message: Timeout of 10000ms exceeded', + 408, + ), + ], + ])('should keep transport detail out of the tool result on %s', async (_, error) => { + mockForestServerClient.listMcpEnabledWorkflows.mockRejectedValue(error); + + const result = await registeredToolHandler({}, mockExtra); + const { text } = (result as { content: [{ text: string }] }).content[0]; + + expect(result).toMatchObject({ isError: true }); + expect(text).toBe( + 'The MCP-enabled workflows could not be listed because Forest could not be reached. This ' + + 'is a temporary server-side failure — retry later, and report it to your Forest ' + + 'administrator if it persists.', + ); + expect(text).not.toContain('10.0.4.17'); + expect(text).not.toContain('acme.corp'); + // The operator keeps the whole cause. + expect(mockLogger).toHaveBeenCalledWith( + 'Error', + `Failed to list MCP-enabled workflows: ${error.message}`, + ); + }); + + // Discovery is the step a model repeats when anything downstream rejects it, so a refusal that + // will not change has to close the loop rather than invite another round. + it.each([ + ['an unknown rendering', new NotFoundError('Rendering not found')], + ['a malformed collectionName', new HttpError('Validation failed', 400)], + ])('should tell the model not to retry %s', async (_, error) => { + mockForestServerClient.listMcpEnabledWorkflows.mockRejectedValue(error); + + const result = await registeredToolHandler({}, mockExtra); + const { text } = (result as { content: [{ text: string }] }).content[0]; + + expect(result).toMatchObject({ isError: true }); + expect(text).toBe( + `${error.message.replace(/\.$/, '')}. Retrying will not help: fix the request, or report ` + + 'it to your Forest administrator.', + ); + }); + + it('should leave a server error retryable, with no advice to stop', async () => { + const error = new HttpError('Internal server error', 500); + mockForestServerClient.listMcpEnabledWorkflows.mockRejectedValue(error); + + const result = await registeredToolHandler({}, mockExtra); + const { text } = (result as { content: [{ text: string }] }).content[0]; + + expect(text).toBe('Internal server error'); + expect(text).not.toContain('Retrying will not help'); + }); + }); +}); diff --git a/packages/mcp-server/test/tools/trigger-workflow.test.ts b/packages/mcp-server/test/tools/trigger-workflow.test.ts new file mode 100644 index 0000000000..7afa1e1b50 --- /dev/null +++ b/packages/mcp-server/test/tools/trigger-workflow.test.ts @@ -0,0 +1,524 @@ +import type { ForestServerClient } from '../../src/http-client'; +import type { Logger } from '../../src/server'; +import type { RegisteredToolConfig } from '../helpers/registered-tool-config'; +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; +import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; + +import { ForbiddenError, HttpError, NotFoundError } from '@forestadmin/forestadmin-client'; + +import declareTriggerWorkflowTool from '../../src/tools/trigger-workflow'; +import createMockForestServerClient from '../helpers/forest-server-client'; + +const mockLogger: Logger = jest.fn(); + +describe('declareTriggerWorkflowTool', () => { + let mcpServer: McpServer; + let mockForestServerClient: jest.Mocked; + let registeredToolHandler: (args: unknown, extra: unknown) => Promise; + let registeredToolConfig: RegisteredToolConfig; + + beforeEach(() => { + jest.clearAllMocks(); + + mockForestServerClient = createMockForestServerClient(); + + mcpServer = { + registerTool: jest.fn((name, config, handler) => { + registeredToolConfig = config; + registeredToolHandler = handler; + }), + } as unknown as McpServer; + }); + + describe('tool registration', () => { + it('should register a tool named "triggerWorkflow"', () => { + declareTriggerWorkflowTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + expect(mcpServer.registerTool).toHaveBeenCalledWith( + 'triggerWorkflow', + expect.any(Object), + expect.any(Function), + ); + }); + + it('should register tool with correct title and description', () => { + declareTriggerWorkflowTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + expect(registeredToolConfig.title).toBe('Trigger a workflow'); + expect(registeredToolConfig.description).toContain('getWorkflowRun'); + }); + + it('should annotate the tool as a non-read-only, destructive, non-idempotent write', () => { + declareTriggerWorkflowTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + expect(registeredToolConfig.annotations).toEqual({ + readOnlyHint: false, + destructiveHint: true, + idempotentHint: false, + openWorldHint: true, + }); + }); + + it('should require string workflowId and recordId arguments', () => { + declareTriggerWorkflowTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + const schema = registeredToolConfig.inputSchema as Record< + string, + { parse: (value: unknown) => unknown } + >; + + expect(() => schema.workflowId.parse('wf-1')).not.toThrow(); + expect(() => schema.workflowId.parse(undefined)).toThrow(); + expect(() => schema.workflowId.parse('')).toThrow(); + expect(() => schema.recordId.parse('42')).not.toThrow(); + expect(() => schema.recordId.parse(123)).toThrow(); + expect(() => schema.recordId.parse('')).toThrow(); + }); + }); + + describe('tool execution', () => { + const mockExtra = { + authInfo: { + token: 'test-token', + extra: { + forestServerToken: 'forest-token', + renderingId: 123, + environmentApiEndpoint: 'https://api.example.com', + }, + }, + } as unknown as RequestHandlerExtra; + + beforeEach(() => { + declareTriggerWorkflowTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + mockForestServerClient.getMcpWorkflowById.mockResolvedValue({ + workflowId: 'wf-1', + name: 'Refund order', + collectionName: 'orders', + mcpEnabled: true, + }); + mockForestServerClient.triggerMcpWorkflow.mockResolvedValue({ + runId: '7', + runState: 'loading', + }); + }); + + it('should resolve the workflow by id with the identity from the auth context', async () => { + await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(mockForestServerClient.getMcpWorkflowById).toHaveBeenCalledWith({ + forestServerToken: 'forest-token', + renderingId: '123', + workflowId: 'wf-1', + }); + }); + + it('should trigger the workflow with the identity from the auth context and the args', async () => { + await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(mockForestServerClient.triggerMcpWorkflow).toHaveBeenCalledWith({ + forestServerToken: 'forest-token', + renderingId: '123', + workflowId: 'wf-1', + recordId: '42', + }); + }); + + it('should not list workflows before triggering', async () => { + await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(mockForestServerClient.listMcpEnabledWorkflows).not.toHaveBeenCalled(); + }); + + it('should return only the runId and runState as JSON text content', async () => { + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(result).toEqual({ + content: [{ type: 'text', text: JSON.stringify({ runId: '7', runState: 'loading' }) }], + }); + }); + + // "requested", not "triggered": the orchestrator writes a second row with the latter once the + // run is committed. Aligning the two wordings — which an earlier round did — made one trigger + // read as two identical events in Activity Logs, and since the orchestrator's row is + // best-effort while this one is fail-closed, the number of rows per trigger is not even + // stable. Do not "fix" this back to match its sibling. + it('should record the activity log before triggering, labelled as a request', async () => { + await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(mockForestServerClient.createMcpActivityLog).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'triggerWorkflow', + type: 'write', + collectionName: 'orders', + recordId: '42', + label: 'requested the workflow "Refund order" via MCP', + }), + ); + + const logOrder = mockForestServerClient.createMcpActivityLog.mock.invocationCallOrder[0]; + const triggerOrder = mockForestServerClient.triggerMcpWorkflow.mock.invocationCallOrder[0]; + expect(logOrder).toBeLessThan(triggerOrder); + }); + + it('should mark the activity log as completed after a successful trigger', async () => { + await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(mockForestServerClient.updateActivityLogStatus).toHaveBeenCalledWith( + expect.objectContaining({ status: 'completed' }), + ); + }); + + it('should not start the run when the pending activity log cannot be created (fail-closed)', async () => { + mockForestServerClient.createMcpActivityLog.mockRejectedValue( + new Error('connect ECONNREFUSED 10.0.0.4:5432'), + ); + + const result = (await registeredToolHandler( + { workflowId: 'wf-1', recordId: '42' }, + mockExtra, + )) as { content: [{ type: string; text: string }]; isError: boolean }; + + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('was not performed') }], + isError: true, + }); + // The cause never reaches the model: it names an internal host and port. + expect(result.content[0].text).not.toContain('ECONNREFUSED'); + expect(mockForestServerClient.triggerMcpWorkflow).not.toHaveBeenCalled(); + }); + + it('should not start the run when the audit store returns a 200 with a null id (fail-closed)', async () => { + mockForestServerClient.createMcpActivityLog.mockResolvedValue({ + id: null, + attributes: {}, + } as never); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(result).toEqual({ + content: [ + { type: 'text', text: expect.stringContaining('the server returned no activity log id') }, + ], + isError: true, + }); + expect(mockForestServerClient.triggerMcpWorkflow).not.toHaveBeenCalled(); + expect(mockForestServerClient.updateActivityLogStatus).not.toHaveBeenCalled(); + }); + + it('should return an error result when the auth context is missing the token', async () => { + const extraWithoutToken = { + authInfo: { extra: { renderingId: 123 } }, + } as unknown as RequestHandlerExtra; + + const result = await registeredToolHandler( + { workflowId: 'wf-1', recordId: '42' }, + extraWithoutToken, + ); + + expect(result).toEqual({ + content: [{ type: 'text', text: expect.stringContaining('forestServerToken') }], + isError: true, + }); + expect(mockForestServerClient.getMcpWorkflowById).not.toHaveBeenCalled(); + expect(mockForestServerClient.triggerMcpWorkflow).not.toHaveBeenCalled(); + }); + + it('should reject an unknown workflow (lookup 404) without triggering or auditing', async () => { + mockForestServerClient.getMcpWorkflowById.mockRejectedValue( + new NotFoundError('Workflow not found'), + ); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(result).toEqual({ + content: [ + { type: 'text', text: expect.stringContaining('is not an MCP-enabled workflow') }, + ], + isError: true, + }); + expect(mockForestServerClient.triggerMcpWorkflow).not.toHaveBeenCalled(); + expect(mockForestServerClient.createMcpActivityLog).not.toHaveBeenCalled(); + }); + + it('should reject a recordId longer than the server column, before any call', async () => { + const schema = registeredToolConfig.inputSchema as { + recordId: { safeParse: (value: unknown) => { success: boolean } }; + }; + + expect(schema.recordId.safeParse('x'.repeat(255)).success).toBe(true); + expect(schema.recordId.safeParse('x'.repeat(256)).success).toBe(false); + }); + + it('should tell the caller not to retry when the lookup 404s, so a missing route cannot loop', async () => { + mockForestServerClient.getMcpWorkflowById.mockRejectedValue( + new NotFoundError('Workflow not found'), + ); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(result).toEqual({ + content: [ + { + type: 'text', + text: + 'Workflow "wf-1" is not an MCP-enabled workflow you can access. Use listWorkflows to ' + + 'discover triggerable workflows. If listWorkflows just returned this id, do not ' + + 'retry — report it to your Forest administrator instead.', + }, + ], + isError: true, + }); + }); + + it('should log the underlying lookup failure so an outage is visible to an operator', async () => { + mockForestServerClient.getMcpWorkflowById.mockRejectedValue( + new NotFoundError('Cannot GET /api/workflow-orchestrator/mcp-workflows/wf-1'), + ); + + await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(mockLogger).toHaveBeenCalledWith( + 'Error', + 'Failed to resolve workflow "wf-1" via getMcpWorkflowById: ' + + 'Cannot GET /api/workflow-orchestrator/mcp-workflows/wf-1', + ); + }); + + it('should not leak transport detail to the model when the lookup fails for any other reason', async () => { + mockForestServerClient.getMcpWorkflowById.mockRejectedValue( + new Error( + 'Failed to reach Forest: connect ECONNREFUSED 10.1.2.3:3310 ' + + '(https://api.internal.forestadmin.com)', + ), + ); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(result).toEqual({ + content: [ + { + type: 'text', + text: + 'Workflow "wf-1" could not be resolved because Forest could not be reached. This is ' + + 'a temporary server-side failure, not a problem with the workflow id — retry later, ' + + 'and report it to your Forest administrator if it persists.', + }, + ], + isError: true, + }); + // The detail stays operator-side. + expect(JSON.stringify(result)).not.toContain('ECONNREFUSED'); + expect(JSON.stringify(result)).not.toContain('api.internal.forestadmin.com'); + expect(mockLogger).toHaveBeenCalledWith( + 'Error', + expect.stringContaining('connect ECONNREFUSED 10.1.2.3:3310'), + ); + expect(mockForestServerClient.createMcpActivityLog).not.toHaveBeenCalled(); + expect(mockForestServerClient.triggerMcpWorkflow).not.toHaveBeenCalled(); + }); + + // The server validates workflowId as a UUID, the identity, and the workflow engine on every + // lookup, so these fail identically on every attempt. Sending them down the "retry later" path + // is what let a model loop forever on a request — typically a workflow *name* it guessed + // instead of the id — that can never succeed. Each keeps Forest's own reason: unlike the 404, + // none of them leaks whether the workflow exists. + it.each([ + ['a non-UUID workflowId (400)', new HttpError('"workflowId" must be a valid GUID', 400)], + ['an expired token (401)', new HttpError('Unauthorized', 401)], + ['a refused identity (403)', new ForbiddenError('Forbidden')], + [ + 'a browser-engine environment (409)', + new HttpError('This environment still runs workflows in the browser', 409), + ], + ])('should treat %s as terminal rather than retryable', async (_, error) => { + mockForestServerClient.getMcpWorkflowById.mockRejectedValue(error); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + const { text } = (result as { content: [{ text: string }] }).content[0]; + + expect(result).toMatchObject({ isError: true }); + expect(text).toBe( + `Workflow "wf-1" could not be resolved — ${error.message.replace(/\.$/, '')}. Retrying ` + + 'will not help: fix the request, or report it to your Forest administrator.', + ); + expect(text).not.toContain('retry later'); + expect(mockForestServerClient.triggerMcpWorkflow).not.toHaveBeenCalled(); + }); + + // The 404 is the one that must stay uniform: it covers unknown, MCP-disabled and + // out-of-rendering alike, so a caller cannot use it to probe which workflows exist. + it('should keep the uniform wording on a 404 so it cannot be used to probe', async () => { + mockForestServerClient.getMcpWorkflowById.mockRejectedValue( + new NotFoundError('Workflow 3f7c not found in rendering 42'), + ); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + const { text } = (result as { content: [{ text: string }] }).content[0]; + + expect(text).toBe( + 'Workflow "wf-1" is not an MCP-enabled workflow you can access. Use listWorkflows to ' + + 'discover triggerable workflows. If listWorkflows just returned this id, do not retry ' + + '— report it to your Forest administrator instead.', + ); + expect(text).not.toContain('rendering 42'); + }); + + it('should still tell the caller to retry when the lookup fails with a 5xx', async () => { + mockForestServerClient.getMcpWorkflowById.mockRejectedValue( + new HttpError('Internal Server Error', 503), + ); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect((result as { content: [{ text: string }] }).content[0].text).toContain('retry later'); + }); + + // The lookup's guard is one call early: the start itself goes through withActivityLog, which + // rethrows the parsed message as-is. + it('should keep transport detail out of the model when the trigger call itself fails', async () => { + mockForestServerClient.triggerMcpWorkflow.mockRejectedValue( + new Error('connect ECONNREFUSED 10.0.4.17:3310'), + ); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + const { text } = (result as { content: [{ text: string }] }).content[0]; + + expect(result).toMatchObject({ isError: true }); + expect(text).toBe( + 'Workflow "wf-1" could not be triggered because Forest could not be reached. The run may ' + + 'or may not have started — do not retry blindly; report it to your Forest administrator.', + ); + expect(text).not.toContain('ECONNREFUSED'); + expect(mockLogger).toHaveBeenCalledWith( + 'Error', + 'Failed to trigger workflow "wf-1": connect ECONNREFUSED 10.0.4.17:3310', + ); + // The audit row was written before the trigger and must still be closed out. + expect(mockForestServerClient.updateActivityLogStatus).toHaveBeenCalledWith( + expect.objectContaining({ status: 'failed' }), + ); + }); + + it('should reject an MCP-disabled workflow without triggering or auditing', async () => { + mockForestServerClient.getMcpWorkflowById.mockResolvedValue({ + workflowId: 'wf-1', + name: 'Refund order', + collectionName: 'orders', + mcpEnabled: false, + }); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(result).toEqual({ + content: [ + { type: 'text', text: expect.stringContaining('is not an MCP-enabled workflow') }, + ], + isError: true, + }); + expect(mockForestServerClient.triggerMcpWorkflow).not.toHaveBeenCalled(); + expect(mockForestServerClient.createMcpActivityLog).not.toHaveBeenCalled(); + }); + + it('should reject a workflow whose collection is unavailable without auditing or triggering', async () => { + mockForestServerClient.getMcpWorkflowById.mockResolvedValue({ + workflowId: 'wf-1', + name: 'Refund order', + collectionName: null, + mcpEnabled: true, + }); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(result).toEqual({ + content: [ + { + type: 'text', + text: + 'Workflow "wf-1" cannot be triggered via MCP because its collection is unavailable. ' + + "Check the workflow's configuration in Forest.", + }, + ], + isError: true, + }); + expect(mockForestServerClient.createMcpActivityLog).not.toHaveBeenCalled(); + expect(mockForestServerClient.triggerMcpWorkflow).not.toHaveBeenCalled(); + expect(mockForestServerClient.updateActivityLogStatus).not.toHaveBeenCalled(); + }); + + it('should map a trigger-time 404 race to the friendly error and mark the log failed', async () => { + mockForestServerClient.triggerMcpWorkflow.mockRejectedValue( + new NotFoundError('Workflow MCP trigger not found or disabled'), + ); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(result).toEqual({ + content: [ + { type: 'text', text: expect.stringContaining('is not an MCP-enabled workflow') }, + ], + isError: true, + }); + expect(mockForestServerClient.createMcpActivityLog).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'triggerWorkflow', + collectionName: 'orders', + recordId: '42', + label: 'requested the workflow "Refund order" via MCP', + }), + ); + expect(mockForestServerClient.updateActivityLogStatus).toHaveBeenCalledWith( + expect.objectContaining({ status: 'failed' }), + ); + }); + + it('should pass a 409 already-ongoing run through as an error and mark the log failed', async () => { + // An HttpError, not a bare Error: ServerUtils maps a 409 carrying a JSON:API detail to + // `new HttpError(detail, 409)`. The distinction is load-bearing — a bare Error is what a raw + // transport failure looks like, and those are the ones whose message must not reach a model. + mockForestServerClient.triggerMcpWorkflow.mockRejectedValue( + new HttpError('A run is already ongoing on this record', 409), + ); + + const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra); + + expect(result).toEqual({ + content: [ + { type: 'text', text: expect.stringContaining('already ongoing on this record') }, + ], + isError: true, + }); + expect(mockForestServerClient.createMcpActivityLog).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'triggerWorkflow', + collectionName: 'orders', + recordId: '42', + label: 'requested the workflow "Refund order" via MCP', + }), + ); + expect(mockForestServerClient.updateActivityLogStatus).toHaveBeenCalledWith( + expect.objectContaining({ status: 'failed' }), + ); + }); + }); +}); diff --git a/packages/mcp-server/test/utils/activity-logs-creator.test.ts b/packages/mcp-server/test/utils/activity-logs-creator.test.ts index bbb79e8b70..5de9d42720 100644 --- a/packages/mcp-server/test/utils/activity-logs-creator.test.ts +++ b/packages/mcp-server/test/utils/activity-logs-creator.test.ts @@ -3,9 +3,10 @@ import type { ActivityLogAction } from '../../src/utils/activity-logs-creator'; import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; -import { NotFoundError } from '@forestadmin/forestadmin-client'; +import { ForbiddenError, HttpError, NotFoundError } from '@forestadmin/forestadmin-client'; import createPendingActivityLog, { + AUDIT_UNAVAILABLE_MESSAGE, markActivityLogAsFailed, markActivityLogAsSucceeded, } from '../../src/utils/activity-logs-creator'; @@ -236,16 +237,155 @@ describe('createPendingActivityLog', () => { }); describe('error handling', () => { - it('should propagate error when createMcpActivityLog fails', async () => { + it.each(['action', 'create', 'update', 'delete', 'triggerWorkflow'])( + 'should block write action "%s" and keep the server reason when createMcpActivityLog rejects (fail-closed)', + async action => { + mockForestServerClient.createMcpActivityLog.mockRejectedValue( + new HttpError('collectionModelName is required', 400), + ); + + const request = createMockRequest(); + + await expect( + createPendingActivityLog(mockForestServerClient, request, action), + ).rejects.toThrow('collectionModelName is required'); + }, + ); + + // The blocked write is reported to a model, so a cause carrying the Forest server URL, an + // internal host:port or a TLS chain must not travel with it - only the fixed sentence does, + // and the cause goes to the operator log instead. + it.each([ + ['a transport failure', new Error('connect ECONNREFUSED 10.0.0.4:5432')], + [ + 'a timeout', + new HttpError('Timeout of 10000ms exceeded on https://api.forestadmin.com/liana', 408), + ], + ])('should replace %s with a sanitized message on a write', async (_label, error) => { + mockForestServerClient.createMcpActivityLog.mockRejectedValue(error); + + const logger = jest.fn(); + const request = createMockRequest(); + + await expect( + createPendingActivityLog(mockForestServerClient, request, 'triggerWorkflow', { logger }), + ).rejects.toThrow(AUDIT_UNAVAILABLE_MESSAGE); + + expect(logger).toHaveBeenCalledWith( + 'Error', + `Activity log for 'triggerWorkflow' could not be created: ${(error as Error).message}`, + ); + }); + + it.each([ + 'index', + 'search', + 'filter', + 'listRelatedData', + 'describeCollection', + ])( + 'should resolve to null when createMcpActivityLog rejects for read action "%s" (fail-open)', + async action => { + mockForestServerClient.createMcpActivityLog.mockRejectedValue( + new Error('Failed to create activity log: Server error message'), + ); + + const request = createMockRequest(); + + await expect( + createPendingActivityLog(mockForestServerClient, request, action), + ).resolves.toBeNull(); + }, + ); + + // Real HttpErrors rather than bare Errors: the status is what the fail policy now reads, so a + // generic Error would make these labels describe a mode the code never sees. + it.each([ + ['a 400 rejection (unresolvable collection)', new HttpError('Validation failed', 400)], + ['a 5xx rejection (audit store unreachable)', new HttpError('Internal Server Error', 500)], + ['a transport failure', new Error('connect ECONNREFUSED')], + ])('should let a read through on %s', async (_label, error) => { + mockForestServerClient.createMcpActivityLog.mockRejectedValue(error); + + const request = createMockRequest(); + + await expect( + createPendingActivityLog(mockForestServerClient, request, 'index'), + ).resolves.toBeNull(); + }); + + // An authorization refusal is not an audit outage: the caller's identity was rejected, so the + // read it is about to perform is not authorized either. Fail-open must not swallow it. + it.each([ + ['a 401 rejection', new HttpError('Unauthorized', 401)], + ['a 403 rejection', new ForbiddenError('Forbidden')], + ])('should propagate %s even for a read action', async (_label, error) => { + mockForestServerClient.createMcpActivityLog.mockRejectedValue(error); + + const request = createMockRequest(); + + await expect( + createPendingActivityLog(mockForestServerClient, request, 'index'), + ).rejects.toThrow(error); + }); + + it('should report the cause when a read proceeds unaudited', async () => { mockForestServerClient.createMcpActivityLog.mockRejectedValue( - new Error('Failed to create activity log: Server error message'), + new HttpError('collectionModelName is required', 400), ); + const logger = jest.fn(); + const request = createMockRequest(); + + await expect( + createPendingActivityLog(mockForestServerClient, request, 'index', { logger }), + ).resolves.toBeNull(); + + expect(logger).toHaveBeenCalledWith( + 'Error', + "Activity log for 'index' could not be created: collectionModelName is required", + ); + }); + + it('should report the cause when a read gets a 200 with no activity log id', async () => { + mockForestServerClient.createMcpActivityLog.mockResolvedValue({ id: null } as never); + + const logger = jest.fn(); + const request = createMockRequest(); + + await expect( + createPendingActivityLog(mockForestServerClient, request, 'index', { logger }), + ).resolves.toBeNull(); + + expect(logger).toHaveBeenCalledWith( + 'Error', + "Activity log for 'index' could not be created: the server answered 200 with no " + + 'activity log id, so the audit store dropped the write.', + ); + }); + + it('should never forward the logger to the server', async () => { + mockForestServerClient.createMcpActivityLog.mockResolvedValue({ id: 'log-1' } as never); + + const logger = jest.fn(); const request = createMockRequest(); + await createPendingActivityLog(mockForestServerClient, request, 'index', { logger }); + + expect(mockForestServerClient.createMcpActivityLog).toHaveBeenCalledWith( + expect.not.objectContaining({ logger: expect.anything() }), + ); + }); + + it('should still throw an invalid-auth-context error for a read action', async () => { + const request = { + authInfo: { extra: { renderingId: '12345' } }, + } as unknown as RequestHandlerExtra; + await expect( createPendingActivityLog(mockForestServerClient, request, 'index'), - ).rejects.toThrow('Failed to create activity log: Server error message'); + ).rejects.toThrow('Invalid or missing forestServerToken in authentication context'); + expect(mockForestServerClient.createMcpActivityLog).not.toHaveBeenCalled(); }); it('should not throw when createMcpActivityLog succeeds', async () => { @@ -255,6 +395,59 @@ describe('createPendingActivityLog', () => { createPendingActivityLog(mockForestServerClient, request, 'index'), ).resolves.not.toThrow(); }); + + it.each(['action', 'create', 'update', 'delete', 'triggerWorkflow'])( + 'should reject when the server returns a 200 with a null id for write action "%s" (fail-closed)', + async action => { + mockForestServerClient.createMcpActivityLog.mockResolvedValue({ + id: null, + attributes: {}, + } as never); + + const request = createMockRequest(); + + await expect( + createPendingActivityLog(mockForestServerClient, request, action), + ).rejects.toThrow( + 'Failed to create activity log: the server returned no activity log id. ' + + 'Blocking the operation to preserve the audit trail.', + ); + }, + ); + + it('should reject when the server returns a response with an undefined id for a write action (fail-closed)', async () => { + mockForestServerClient.createMcpActivityLog.mockResolvedValue({ + attributes: {}, + } as never); + + const request = createMockRequest(); + + await expect( + createPendingActivityLog(mockForestServerClient, request, 'triggerWorkflow'), + ).rejects.toThrow('the server returned no activity log id'); + }); + + it.each([ + 'index', + 'search', + 'filter', + 'listRelatedData', + 'describeCollection', + ])( + 'should resolve to null when the server returns a 200 with a null id for read action "%s" (fail-open)', + async action => { + mockForestServerClient.createMcpActivityLog.mockResolvedValue({ + id: null, + attributes: {}, + } as never); + + const request = createMockRequest(); + + await expect( + createPendingActivityLog(mockForestServerClient, request, action), + ).resolves.toBeNull(); + }, + ); }); }); @@ -279,6 +472,38 @@ describe('markActivityLogAsFailed', () => { } as unknown as RequestHandlerExtra; } + it.each([ + ['a missing token', {}], + ['a non-string token', { forestServerToken: 42 }], + ['an empty token', { forestServerToken: '' }], + ])('should skip the status update on %s rather than send an empty Bearer', async (_l, extra) => { + const request = { + authInfo: { token: 'mock-token', extra }, + } as unknown as RequestHandlerExtra; + const activityLog = { id: 'log-123', attributes: { index: 'idx-456' } }; + const mockLogger = jest.fn(); + + markActivityLogAsFailed({ + forestServerClient: mockForestServerClient, + request, + activityLog, + logger: mockLogger, + }); + + await new Promise(resolve => { + setTimeout(resolve, 0); + }); + + // An empty Bearer can only be refused, so the round trip is skipped entirely. (The retry + // branch below is 404-only, so a 401 would not have been repeated — it would just have cost + // one pointless call and an error log naming an auth failure rather than the missing token.) + expect(mockForestServerClient.updateActivityLogStatus).not.toHaveBeenCalled(); + expect(mockLogger).toHaveBeenCalledWith( + 'Error', + "Cannot update activity log status to 'failed': no forestServerToken in the auth context.", + ); + }); + it('should call updateActivityLogStatus with failed status', async () => { const request = createMockRequest(); const activityLog = { id: 'log-123', attributes: { index: 'idx-456' } }; diff --git a/packages/mcp-server/test/utils/auth-context.test.ts b/packages/mcp-server/test/utils/auth-context.test.ts new file mode 100644 index 0000000000..b1ca6f1331 --- /dev/null +++ b/packages/mcp-server/test/utils/auth-context.test.ts @@ -0,0 +1,69 @@ +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; +import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; + +import getAuthContext from '../../src/utils/auth-context'; + +describe('getAuthContext', () => { + const createRequest = (extra: Record | undefined) => + ({ + authInfo: extra === undefined ? undefined : { extra }, + } as unknown as RequestHandlerExtra); + + it('should return the forestServerToken and renderingId from authInfo.extra', () => { + const request = createRequest({ forestServerToken: 'forest-token', renderingId: '123' }); + + expect(getAuthContext(request)).toEqual({ + forestServerToken: 'forest-token', + renderingId: '123', + }); + }); + + it('should convert a numeric renderingId (from the JWT) to a string', () => { + const request = createRequest({ forestServerToken: 'forest-token', renderingId: 456 }); + + expect(getAuthContext(request)).toEqual({ + forestServerToken: 'forest-token', + renderingId: '456', + }); + }); + + it('should throw when forestServerToken is missing', () => { + const request = createRequest({ renderingId: '123' }); + + expect(() => getAuthContext(request)).toThrow( + 'Invalid or missing forestServerToken in authentication context', + ); + }); + + it('should throw when forestServerToken is not a string', () => { + const request = createRequest({ forestServerToken: 42, renderingId: '123' }); + + expect(() => getAuthContext(request)).toThrow( + 'Invalid or missing forestServerToken in authentication context', + ); + }); + + it('should throw when renderingId is missing', () => { + const request = createRequest({ forestServerToken: 'forest-token' }); + + expect(() => getAuthContext(request)).toThrow( + 'Invalid or missing renderingId in authentication context', + ); + }); + + it('should throw when renderingId is null', () => { + const request = createRequest({ forestServerToken: 'forest-token', renderingId: null }); + + expect(() => getAuthContext(request)).toThrow( + 'Invalid or missing renderingId in authentication context', + ); + }); + + it('should throw when authInfo is absent entirely', () => { + const request = createRequest(undefined); + + expect(() => getAuthContext(request)).toThrow( + 'Invalid or missing forestServerToken in authentication context', + ); + }); +}); diff --git a/packages/mcp-server/test/utils/with-activity-log.test.ts b/packages/mcp-server/test/utils/with-activity-log.test.ts index 338325ce12..b15af30814 100644 --- a/packages/mcp-server/test/utils/with-activity-log.test.ts +++ b/packages/mcp-server/test/utils/with-activity-log.test.ts @@ -60,7 +60,8 @@ describe('withActivityLog', () => { mockForestServerClient, mockRequest, 'index', - { collectionName: 'users' }, + // The logger travels with the context so the creator can report why a read went unaudited. + { collectionName: 'users', logger: expect.any(Function) }, ); expect(mockCreatePendingActivityLog).toHaveBeenCalledBefore(operation); }); @@ -200,7 +201,7 @@ describe('withActivityLog', () => { mockForestServerClient, mockRequest, 'index', - undefined, + { logger: expect.any(Function) }, ); }); @@ -228,10 +229,101 @@ describe('withActivityLog', () => { collectionName: 'orders', recordIds: [1, 2, 3], label: 'Bulk delete orders', + logger: expect.any(Function), }, ); }); + describe('when the activity log was not persisted (read action, fail-open)', () => { + beforeEach(() => { + mockCreatePendingActivityLog.mockResolvedValue(null); + }); + + it('should run the operation and return its result', async () => { + const expectedResult = { content: [{ type: 'text', text: 'data' }] }; + const operation = jest.fn().mockResolvedValue(expectedResult); + + const result = await withActivityLog({ + forestServerClient: mockForestServerClient, + request: mockRequest, + action: 'index', + logger: mockLogger, + operation, + }); + + expect(result).toEqual(expectedResult); + }); + + it('should log a warning about the missing audit trail', async () => { + const operation = jest.fn().mockResolvedValue({ result: 'success' }); + + await withActivityLog({ + forestServerClient: mockForestServerClient, + request: mockRequest, + action: 'index', + logger: mockLogger, + operation, + }); + + expect(mockLogger).toHaveBeenCalledWith( + 'Warn', + "Activity log for 'index' was not persisted by the server; " + + 'proceeding without audit trail for this read operation.', + ); + }); + + it('should not track the activity log status on success', async () => { + const operation = jest.fn().mockResolvedValue({ result: 'success' }); + + await withActivityLog({ + forestServerClient: mockForestServerClient, + request: mockRequest, + action: 'index', + logger: mockLogger, + operation, + }); + + expect(mockMarkActivityLogAsSucceeded).not.toHaveBeenCalled(); + }); + + it('should not track the activity log status on failure, and still throw', async () => { + const operation = jest.fn().mockRejectedValue(new Error('Operation failed')); + + await expect( + withActivityLog({ + forestServerClient: mockForestServerClient, + request: mockRequest, + action: 'index', + logger: mockLogger, + operation, + }), + ).rejects.toThrow('Operation failed'); + + expect(mockMarkActivityLogAsFailed).not.toHaveBeenCalled(); + }); + }); + + describe('when the activity log could not be created (write action, fail-closed)', () => { + it('should not run the operation and should propagate the error', async () => { + mockCreatePendingActivityLog.mockRejectedValue(new Error('Internal Server Error')); + const operation = jest.fn().mockResolvedValue({ result: 'success' }); + + await expect( + withActivityLog({ + forestServerClient: mockForestServerClient, + request: mockRequest, + action: 'triggerWorkflow', + logger: mockLogger, + operation, + }), + ).rejects.toThrow('Internal Server Error'); + + expect(operation).not.toHaveBeenCalled(); + expect(mockMarkActivityLogAsSucceeded).not.toHaveBeenCalled(); + expect(mockMarkActivityLogAsFailed).not.toHaveBeenCalled(); + }); + }); + describe('errorEnhancer', () => { it('should apply errorEnhancer to error message when provided', async () => { const operation = jest.fn().mockRejectedValue(new Error('Original error')); diff --git a/packages/workflow-executor/src/adapters/server-types.ts b/packages/workflow-executor/src/adapters/server-types.ts index 7295b73ca3..101fbd67c3 100644 --- a/packages/workflow-executor/src/adapters/server-types.ts +++ b/packages/workflow-executor/src/adapters/server-types.ts @@ -191,6 +191,7 @@ export type ServerWorkflowRunState = 'started' | 'pending' | 'loading' | 'aborte export enum ServerWorkflowTriggerType { manual = 'manual', webhook = 'webhook', + mcp = 'mcp', } export interface ServerHydratedWorkflowRun { diff --git a/packages/workflow-executor/src/types/validated/execution.ts b/packages/workflow-executor/src/types/validated/execution.ts index fa4ca7c265..1b80e70963 100644 --- a/packages/workflow-executor/src/types/validated/execution.ts +++ b/packages/workflow-executor/src/types/validated/execution.ts @@ -34,6 +34,7 @@ export type Step = z.infer; export enum TriggerType { Manual = 'manual', Webhook = 'webhook', + Mcp = 'mcp', } export const TriggerTypeSchema = z.nativeEnum(TriggerType); diff --git a/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts b/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts index 4397c00e91..327f0af681 100644 --- a/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts +++ b/packages/workflow-executor/test/adapters/run-to-available-step-mapper.test.ts @@ -135,6 +135,14 @@ describe('toAvailableStepExecution', () => { expect(result?.triggerType).toBe(TriggerType.Webhook); }); + it('should map an mcp-triggered run without failing validation', () => { + const run = makeRun({ triggerType: ServerWorkflowTriggerType.mcp }); + + const result = toAvailableStepExecution(run); + + expect(result?.triggerType).toBe(TriggerType.Mcp); + }); + it('should default triggerType to manual when the orchestrator omits it', () => { const run = makeRun(); delete run.triggerType;