perf(mcp-server): trigger workflow by id instead of listing all workflows (PRD-831)

packages/forestadmin-client/src/permissions/forest-http-api.ts

@@ -177,12 +177,12 @@ export default class ForestHttpApi implements ForestAdminServerInterface {
     workflowId: string,
     recordId: string,
   ): Promise<WorkflowRunTriggerResult> {
-    // The orchestrator returns a numeric runId; normalize it to the string form
-    // expected by getMcpWorkflowRun.
-    const result = await ServerUtils.queryWithBearerToken<{
-      runId: number | string;
-      runState: WorkflowRunTriggerResult['runState'];
-    }>({
+    // The orchestrator returns a numeric runId; normalize it to the string form expected by
+    // getMcpWorkflowRun. workflowName/collectionName are passed through when present so the
+    // caller can label the audit log without listing every workflow first.
+    const result = await ServerUtils.queryWithBearerToken<
+      Omit<WorkflowRunTriggerResult, 'runId'> & { runId: number | string }
+    >({
       forestServerUrl: options.forestServerUrl,
       method: 'post',
       path: `/api/workflow-orchestrator/mcp-workflows/${encodeURIComponent(workflowId)}/start`,
@@ -191,7 +191,7 @@ export default class ForestHttpApi implements ForestAdminServerInterface {
       headers: { 'forest-rendering-id': renderingId, ...options.headers },
     });
 
-    return { runId: String(result.runId), runState: result.runState };
+    return { ...result, runId: String(result.runId) };
   }
 
   async getMcpWorkflowRun(

packages/forestadmin-client/src/types.ts

@@ -307,10 +307,14 @@ export type WorkflowRunState = 'started' | 'pending' | 'loading' | 'aborted' | '
 /**
  * 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.
+ * `workflowName`/`collectionName` are echoed by the start endpoint to build the audit label
+ * without a second round-trip; they are optional so older servers degrade gracefully.
  */
 export interface WorkflowRunTriggerResult {
   runId: string;
   runState: WorkflowRunState;
+  workflowName?: string;
+  collectionName?: string | null;
 }
 
 export interface TriggerMcpWorkflowParams {

packages/mcp-server/src/tools/trigger-workflow.ts

@@ -1,11 +1,15 @@
+import type { WorkflowRunTriggerResult } 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 createPendingActivityLog, {
+  markActivityLogAsSucceeded,
+} from '../utils/activity-logs-creator';
 import getAuthContext from '../utils/auth-context';
 import registerToolWithLogging from '../utils/tool-with-logging';
-import withActivityLog from '../utils/with-activity-log';
 
 const WORKFLOW_ID_DESCRIPTION =
   'The id of the workflow to start, as returned by listWorkflows. The workflow must have the MCP ' +
@@ -41,48 +45,62 @@ export default function declareTriggerWorkflowTool(mcpServer: McpServer, ctx: To
     async (args: TriggerWorkflowArgument, extra) => {
       const { forestServerToken, renderingId } = getAuthContext(extra);
 
-      // We list workflows first to resolve the name/collection needed for the activity-log label
-      // (the trigger endpoint returns neither). The server also validates access at trigger time,
-      // so this lookup is primarily for enrichment; targeting the workflow by id directly would
-      // save a round-trip — tracked in PRD-831.
-      const workflows = await forestServerClient.listMcpWorkflows({
-        forestServerToken,
-        renderingId,
-      });
-      const workflow = workflows.find(candidate => candidate.workflowId === args.workflowId);
+      let result: WorkflowRunTriggerResult;
 
-      // Rejected before withActivityLog: with no resolved workflow there is no collection to
-      // attach, and the server drops MCP activity logs that carry no resource (see PRD-49), so a
-      // pre-trigger rejection cannot be audited. Only real triggers are logged (incl. server-side
-      // 403/409, which fail inside withActivityLog below).
-      if (!workflow) {
-        throw new Error(
-          `Workflow "${args.workflowId}" is not an MCP-enabled workflow you can access. ` +
-            'Use listWorkflows to discover triggerable workflows.',
-        );
+      try {
+        result = await forestServerClient.triggerWorkflow({
+          forestServerToken,
+          renderingId,
+          workflowId: args.workflowId,
+          recordId: args.recordId,
+        });
+      } catch (error) {
+        // The server answers 404 both for an unknown workflow and for one whose MCP trigger is
+        // disabled (indistinguishable on purpose). Surface the same guidance the tool gave when
+        // it validated the id client-side, so the LLM-facing contract stays identical.
+        if (error instanceof NotFoundError) {
+          throw new Error(
+            `Workflow "${args.workflowId}" is not an MCP-enabled workflow you can access. ` +
+              'Use listWorkflows to discover triggerable workflows.',
+          );
+        }
+
+        throw error;
       }
 
-      return withActivityLog({
-        forestServerClient,
-        request: extra,
-        action: 'triggerWorkflow',
-        context: {
-          collectionName: workflow.collectionName ?? undefined,
-          recordId: args.recordId,
-          label: `triggered the workflow "${workflow.name}"`,
-        },
-        logger,
-        operation: async () => {
-          const result = await forestServerClient.triggerWorkflow({
-            forestServerToken,
-            renderingId,
-            workflowId: args.workflowId,
+      // Audit the successful trigger. The start endpoint echoes the workflow name/collection so we
+      // can label the log without a prior listing; fall back to the id when an older server omits
+      // them. The run is already started, so a logging hiccup must not fail the tool.
+      try {
+        const activityLog = await createPendingActivityLog(
+          forestServerClient,
+          extra,
+          'triggerWorkflow',
+          {
+            collectionName: result.collectionName ?? undefined,
             recordId: args.recordId,
-          });
+            label: `triggered the workflow "${result.workflowName ?? args.workflowId}"`,
+          },
+        );
+
+        markActivityLogAsSucceeded({ forestServerClient, request: extra, activityLog, logger });
+      } catch (error) {
+        logger(
+          'Warn',
+          `Failed to record triggerWorkflow activity log: ${
+            error instanceof Error ? error.message : String(error)
+          }`,
+        );
+      }
 
-          return { content: [{ type: 'text', text: JSON.stringify(result) }] };
-        },
-      });
+      return {
+        content: [
+          {
+            type: 'text',
+            text: JSON.stringify({ runId: result.runId, runState: result.runState }),
+          },
+        ],
+      };
     },
     logger,
   );

packages/mcp-server/test/tools/trigger-workflow.test.ts

@@ -8,13 +8,9 @@ import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sd
 import { NotFoundError } from '@forestadmin/forestadmin-client';
 
 import declareTriggerWorkflowTool from '../../src/tools/trigger-workflow';
-import withActivityLog from '../../src/utils/with-activity-log';
 import createMockForestServerClient from '../helpers/forest-server-client';
 
-jest.mock('../../src/utils/with-activity-log');
-
 const mockLogger: Logger = jest.fn();
-const mockWithActivityLog = withActivityLog as jest.MockedFunction<typeof withActivityLog>;
 
 describe('declareTriggerWorkflowTool', () => {
   let mcpServer: McpServer;
@@ -33,9 +29,6 @@ describe('declareTriggerWorkflowTool', () => {
         registeredToolHandler = handler;
       }),
     } as unknown as McpServer;
-
-    // By default, withActivityLog executes the operation and returns its result
-    mockWithActivityLog.mockImplementation(async options => options.operation());
   });
 
   describe('tool registration', () => {
@@ -111,13 +104,15 @@ describe('declareTriggerWorkflowTool', () => {
         logger: mockLogger,
         collectionNames: [],
       });
-      mockForestServerClient.listMcpWorkflows.mockResolvedValue([
-        { workflowId: 'wf-1', name: 'Refund order', collectionName: 'orders' },
-      ]);
-      mockForestServerClient.triggerWorkflow.mockResolvedValue({ runId: '7', runState: 'loading' });
+      mockForestServerClient.triggerWorkflow.mockResolvedValue({
+        runId: '7',
+        runState: 'loading',
+        workflowName: 'Refund order',
+        collectionName: 'orders',
+      });
     });
 
-    it('should call triggerWorkflow with the identity from the auth context and the args', async () => {
+    it('should start the workflow directly with the identity from the auth context and the args', async () => {
       await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra);
 
       expect(mockForestServerClient.triggerWorkflow).toHaveBeenCalledWith({
@@ -128,46 +123,60 @@ describe('declareTriggerWorkflowTool', () => {
       });
     });
 
-    it('should return the runId and runState as JSON text content', async () => {
+    it('should not list workflows before triggering', async () => {
+      await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra);
+
+      expect(mockForestServerClient.listMcpWorkflows).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' }) }],
       });
     });
 
-    it('should wrap the trigger in an activity log carrying the resolved collection and record', async () => {
+    it('should record an activity log labelled from the response name and collection', async () => {
       await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra);
 
-      expect(mockWithActivityLog).toHaveBeenCalledWith({
-        forestServerClient: mockForestServerClient,
-        request: mockExtra,
-        action: 'triggerWorkflow',
-        context: {
+      expect(mockForestServerClient.createMcpActivityLog).toHaveBeenCalledWith(
+        expect.objectContaining({
+          action: 'triggerWorkflow',
+          type: 'write',
           collectionName: 'orders',
           recordId: '42',
           label: 'triggered the workflow "Refund order"',
-        },
-        logger: mockLogger,
-        operation: expect.any(Function),
+        }),
+      );
+    });
+
+    it('should fall back to the workflowId in the label when the response omits name/collection', async () => {
+      mockForestServerClient.triggerWorkflow.mockResolvedValue({
+        runId: '7',
+        runState: 'loading',
       });
+
+      await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra);
+
+      expect(mockForestServerClient.createMcpActivityLog).toHaveBeenCalledWith(
+        expect.objectContaining({
+          collectionName: undefined,
+          recordId: '42',
+          label: 'triggered the workflow "wf-1"',
+        }),
+      );
     });
 
-    it('should error without triggering when the workflow is not among accessible workflows', async () => {
-      mockForestServerClient.listMcpWorkflows.mockResolvedValue([
-        { workflowId: 'other-wf', name: 'Other', collectionName: 'orders' },
-      ]);
+    it('should still return the run when recording the activity log fails', async () => {
+      mockForestServerClient.createMcpActivityLog.mockRejectedValue(new Error('audit down'));
 
       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,
+        content: [{ type: 'text', text: JSON.stringify({ runId: '7', runState: 'loading' }) }],
       });
-      expect(mockForestServerClient.triggerWorkflow).not.toHaveBeenCalled();
-      expect(mockWithActivityLog).not.toHaveBeenCalled();
+      expect(mockLogger).toHaveBeenCalledWith('Warn', expect.stringContaining('audit down'));
     });
 
     it('should return an error result when the auth context is missing the token', async () => {
@@ -187,32 +196,36 @@ describe('declareTriggerWorkflowTool', () => {
       expect(mockForestServerClient.triggerWorkflow).not.toHaveBeenCalled();
     });
 
-    it('should map a 409 already-ongoing run to an error tool result', async () => {
+    it('should map a server 404 to the "is not an MCP-enabled workflow" tool error', async () => {
       mockForestServerClient.triggerWorkflow.mockRejectedValue(
-        new Error('A run is already ongoing on this record'),
+        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('already ongoing on this record') },
+          { type: 'text', text: expect.stringContaining('is not an MCP-enabled workflow') },
         ],
         isError: true,
       });
+      expect(mockForestServerClient.createMcpActivityLog).not.toHaveBeenCalled();
     });
 
-    it('should map a 404 non-mcp-enabled workflow to an error tool result', async () => {
+    it('should pass a 409 already-ongoing run through as an error tool result', async () => {
       mockForestServerClient.triggerWorkflow.mockRejectedValue(
-        new NotFoundError('Workflow MCP trigger not found or disabled'),
+        new Error('A run is already ongoing on this record'),
       );
 
       const result = await registeredToolHandler({ workflowId: 'wf-1', recordId: '42' }, mockExtra);
 
       expect(result).toEqual({
-        content: [{ type: 'text', text: expect.stringContaining('not found or disabled') }],
+        content: [
+          { type: 'text', text: expect.stringContaining('already ongoing on this record') },
+        ],
         isError: true,
       });
+      expect(mockForestServerClient.createMcpActivityLog).not.toHaveBeenCalled();
     });
   });
 });