diff --git a/packages/ai-proxy/src/forest-integration-client.ts b/packages/ai-proxy/src/forest-integration-client.ts index 449b3d41e0..dff89c8e82 100644 --- a/packages/ai-proxy/src/forest-integration-client.ts +++ b/packages/ai-proxy/src/forest-integration-client.ts @@ -14,6 +14,7 @@ export type CustomConfig = ZendeskConfig | KolarConfig | SnowflakeConfig; export type ForestIntegrationName = 'Zendesk' | 'Kolar' | 'Snowflake'; export interface ForestIntegrationConfig { + id?: string; integrationName: ForestIntegrationName; config: CustomConfig; isForestConnector: true; @@ -39,16 +40,16 @@ export default class ForestIntegrationClient implements ToolProvider { async loadTools(): Promise { const tools: RemoteTool[] = []; - this.configs.forEach(({ integrationName, config }) => { + this.configs.forEach(({ id: mcpServerId, integrationName, config }) => { switch (integrationName) { case 'Zendesk': - tools.push(...getZendeskTools(config as ZendeskConfig)); + tools.push(...getZendeskTools(config as ZendeskConfig, mcpServerId)); break; case 'Kolar': - tools.push(...getKolarTools(config as KolarConfig)); + tools.push(...getKolarTools(config as KolarConfig, mcpServerId)); break; case 'Snowflake': - tools.push(...getSnowflakeTools(config as SnowflakeConfig)); + tools.push(...getSnowflakeTools(config as SnowflakeConfig, mcpServerId)); break; default: this.logger?.('Warn', `Unsupported integration: ${integrationName}`); diff --git a/packages/ai-proxy/src/integrations/kolar/tools.ts b/packages/ai-proxy/src/integrations/kolar/tools.ts index e3ae18701f..57f8bb730c 100644 --- a/packages/ai-proxy/src/integrations/kolar/tools.ts +++ b/packages/ai-proxy/src/integrations/kolar/tools.ts @@ -11,7 +11,7 @@ export interface KolarConfig { apiKey: string; } -export default function getKolarTools(config: KolarConfig): RemoteTool[] { +export default function getKolarTools(config: KolarConfig, mcpServerId?: string): RemoteTool[] { const { baseUrl, headers } = getKolarConfig(config); return [ @@ -23,6 +23,7 @@ export default function getKolarTools(config: KolarConfig): RemoteTool[] { tool => new ServerRemoteTool({ sourceId: 'kolar', + mcpServerId, tool, }), ); diff --git a/packages/ai-proxy/src/integrations/snowflake/tools.ts b/packages/ai-proxy/src/integrations/snowflake/tools.ts index 3b0bad7818..35cbc277ea 100644 --- a/packages/ai-proxy/src/integrations/snowflake/tools.ts +++ b/packages/ai-proxy/src/integrations/snowflake/tools.ts @@ -15,7 +15,10 @@ export interface SnowflakeConfig { defaultRole?: string; } -export default function getSnowflakeTools(config: SnowflakeConfig): RemoteTool[] { +export default function getSnowflakeTools( + config: SnowflakeConfig, + mcpServerId?: string, +): RemoteTool[] { const headers = getSnowflakeAuthHeaders(config); const baseUrl = getSnowflakeBaseUrl(config.accountIdentifier); @@ -27,6 +30,7 @@ export default function getSnowflakeTools(config: SnowflakeConfig): RemoteTool[] tool => new ServerRemoteTool({ sourceId: 'snowflake', + mcpServerId, tool, }), ); diff --git a/packages/ai-proxy/src/integrations/zendesk/tools.ts b/packages/ai-proxy/src/integrations/zendesk/tools.ts index 9886f86c49..d106a69b16 100644 --- a/packages/ai-proxy/src/integrations/zendesk/tools.ts +++ b/packages/ai-proxy/src/integrations/zendesk/tools.ts @@ -15,7 +15,7 @@ export interface ZendeskConfig { apiToken: string; } -export default function getZendeskTools(config: ZendeskConfig): RemoteTool[] { +export default function getZendeskTools(config: ZendeskConfig, mcpServerId?: string): RemoteTool[] { const { baseUrl, headers } = getZendeskConfig(config); return [ @@ -29,6 +29,7 @@ export default function getZendeskTools(config: ZendeskConfig): RemoteTool[] { tool => new ServerRemoteTool({ sourceId: 'zendesk', + mcpServerId, tool, }), ); diff --git a/packages/ai-proxy/src/mcp-client.ts b/packages/ai-proxy/src/mcp-client.ts index 71f2868fc6..bb834c576e 100644 --- a/packages/ai-proxy/src/mcp-client.ts +++ b/packages/ai-proxy/src/mcp-client.ts @@ -8,14 +8,17 @@ import McpServerRemoteTool from './mcp-server-remote-tool'; export type McpServers = MultiServerMCPClient['config']['mcpServers']; -export type McpServerConfig = MultiServerMCPClient['config']['mcpServers'][string]; +export type McpServerConfig = MultiServerMCPClient['config']['mcpServers'][string] & { + id?: string; +}; export type McpConfiguration = { - configs: McpServers; + configs: Record; } & Omit; export default class McpClient implements ToolProvider { private readonly mcpClients: Record = {}; + private readonly mcpServerIdsByName: Record = {}; private readonly logger?: Logger; constructor(config: McpConfiguration, logger?: Logger) { @@ -23,8 +26,11 @@ export default class McpClient implements ToolProvider { // split the config into several clients to be more resilient // if a mcp server is down, the others will still work Object.entries(config.configs).forEach(([name, serverConfig]) => { + const { id: mcpServerId, ...rest } = serverConfig as McpServerConfig & + Record; + this.mcpServerIdsByName[name] = mcpServerId; this.mcpClients[name] = new MultiServerMCPClient({ - mcpServers: { [name]: serverConfig }, + mcpServers: { [name]: rest as McpServerConfig }, ...config, }); }); @@ -39,7 +45,12 @@ export default class McpClient implements ToolProvider { try { const loadedTools = (await client.getTools()) ?? []; const extendedTools = loadedTools.map( - tool => new McpServerRemoteTool({ tool, sourceId: name }), + tool => + new McpServerRemoteTool({ + tool, + sourceId: name, + mcpServerId: this.mcpServerIdsByName[name], + }), ); tools.push(...extendedTools); } catch (error) { diff --git a/packages/ai-proxy/src/mcp-server-remote-tool.ts b/packages/ai-proxy/src/mcp-server-remote-tool.ts index 8bc4f4e7bf..85791d895e 100644 --- a/packages/ai-proxy/src/mcp-server-remote-tool.ts +++ b/packages/ai-proxy/src/mcp-server-remote-tool.ts @@ -3,7 +3,11 @@ import type { StructuredToolInterface } from '@langchain/core/tools'; import RemoteTool from './remote-tool'; export default class McpServerRemoteTool extends RemoteTool { - constructor(options: { tool: StructuredToolInterface; sourceId?: string }) { + constructor(options: { + tool: StructuredToolInterface; + sourceId?: string; + mcpServerId?: string; + }) { super({ ...options, sourceType: 'mcp-server' }); } } diff --git a/packages/ai-proxy/src/remote-tool.ts b/packages/ai-proxy/src/remote-tool.ts index 9c31f3f1d1..a59911664a 100644 --- a/packages/ai-proxy/src/remote-tool.ts +++ b/packages/ai-proxy/src/remote-tool.ts @@ -4,15 +4,20 @@ export default abstract class RemoteTool { base: StructuredToolInterface; sourceId: string; sourceType: string; + // Shared across McpServerRemoteTool and ServerRemoteTool because the orchestrator + // stores Forest connectors alongside user MCP servers in the same ai_mcp_configs table. + mcpServerId?: string; constructor(options: { tool: StructuredToolInterface; sourceId?: string; sourceType?: string; + mcpServerId?: string; }) { this.base = options.tool; this.sourceId = options.sourceId; this.sourceType = options.sourceType; + this.mcpServerId = options.mcpServerId; } get sanitizedName() { diff --git a/packages/ai-proxy/src/server-remote-tool.ts b/packages/ai-proxy/src/server-remote-tool.ts index 0ed5dcf487..2341411972 100644 --- a/packages/ai-proxy/src/server-remote-tool.ts +++ b/packages/ai-proxy/src/server-remote-tool.ts @@ -3,7 +3,11 @@ import type { StructuredToolInterface } from '@langchain/core/tools'; import RemoteTool from './remote-tool'; export default class ServerRemoteTool extends RemoteTool { - constructor(options: { tool: StructuredToolInterface; sourceId?: string }) { + constructor(options: { + tool: StructuredToolInterface; + sourceId?: string; + mcpServerId?: string; + }) { super({ ...options, sourceType: 'server' }); } } diff --git a/packages/ai-proxy/test/forest-integration-client.test.ts b/packages/ai-proxy/test/forest-integration-client.test.ts index 84427975cb..104fb6f80d 100644 --- a/packages/ai-proxy/test/forest-integration-client.test.ts +++ b/packages/ai-proxy/test/forest-integration-client.test.ts @@ -1,6 +1,9 @@ import ForestIntegrationClient from '../src/forest-integration-client'; +import getKolarTools from '../src/integrations/kolar/tools'; import { validateKolarConfig } from '../src/integrations/kolar/utils'; +import getSnowflakeTools from '../src/integrations/snowflake/tools'; import { validateSnowflakeConfig } from '../src/integrations/snowflake/utils'; +import getZendeskTools from '../src/integrations/zendesk/tools'; import { validateZendeskConfig } from '../src/integrations/zendesk/utils'; const mockZendeskTools = [{ name: 'zendesk_get_tickets' }, { name: 'zendesk_get_ticket' }]; @@ -37,6 +40,7 @@ describe('ForestIntegrationClient', () => { it('should load zendesk tools when integration is zendesk', async () => { const client = new ForestIntegrationClient([ { + id: '1', integrationName: 'Zendesk', config: { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' }, isForestConnector: true, @@ -52,7 +56,7 @@ describe('ForestIntegrationClient', () => { const logger = jest.fn(); const client = new ForestIntegrationClient( // @ts-expect-error Testing unsupported integration - [{ integrationName: 'unknown', config: {} as any, isForestConnector: true }], + [{ id: '1', integrationName: 'unknown', config: {} as any, isForestConnector: true }], logger, ); @@ -64,6 +68,7 @@ describe('ForestIntegrationClient', () => { it('should load kolar tools when integration is Kolar', async () => { const client = new ForestIntegrationClient([ { + id: '1', integrationName: 'Kolar', config: { apiKey: 'key' }, isForestConnector: true, @@ -78,6 +83,7 @@ describe('ForestIntegrationClient', () => { it('should load snowflake tools when integration is Snowflake', async () => { const client = new ForestIntegrationClient([ { + id: '1', integrationName: 'Snowflake', config: { accountIdentifier: 'a', @@ -101,11 +107,13 @@ describe('ForestIntegrationClient', () => { it('should load tools from multiple configs', async () => { const client = new ForestIntegrationClient([ { + id: '1', integrationName: 'Zendesk', config: { subdomain: 'a', email: 'a@b.com', apiToken: 'tok' }, isForestConnector: true, }, { + id: '2', integrationName: 'Zendesk', config: { subdomain: 'b', email: 'c@d.com', apiToken: 'tok2' }, isForestConnector: true, @@ -122,7 +130,7 @@ describe('ForestIntegrationClient', () => { it('should call validateZendeskConfig for Zendesk integration', async () => { const zendeskConfig = { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' }; const client = new ForestIntegrationClient([ - { integrationName: 'Zendesk', config: zendeskConfig, isForestConnector: true }, + { id: '1', integrationName: 'Zendesk', config: zendeskConfig, isForestConnector: true }, ]); await client.checkConnection(); @@ -133,6 +141,7 @@ describe('ForestIntegrationClient', () => { it('should return true on success', async () => { const client = new ForestIntegrationClient([ { + id: '1', integrationName: 'Zendesk', config: { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' }, isForestConnector: true, @@ -147,7 +156,7 @@ describe('ForestIntegrationClient', () => { it('should call validateKolarConfig for Kolar integration', async () => { const kolarConfig = { apiKey: 'key' }; const client = new ForestIntegrationClient([ - { integrationName: 'Kolar', config: kolarConfig, isForestConnector: true }, + { id: '1', integrationName: 'Kolar', config: kolarConfig, isForestConnector: true }, ]); await client.checkConnection(); @@ -161,7 +170,7 @@ describe('ForestIntegrationClient', () => { programmaticAccessToken: 'tok', }; const client = new ForestIntegrationClient([ - { integrationName: 'Snowflake', config: snowflakeConfig, isForestConnector: true }, + { id: '1', integrationName: 'Snowflake', config: snowflakeConfig, isForestConnector: true }, ]); await client.checkConnection(); @@ -172,12 +181,10 @@ describe('ForestIntegrationClient', () => { it('should throw for unsupported integration', async () => { const client = new ForestIntegrationClient([ // @ts-expect-error Testing unsupported integration - { integrationName: 'Unknown', config: {}, isForestConnector: true }, + { id: '1', integrationName: 'Unknown', config: {}, isForestConnector: true }, ]); - await expect(client.checkConnection()).rejects.toThrow( - 'Unsupported integration: Unknown', - ); + await expect(client.checkConnection()).rejects.toThrow('Unsupported integration: Unknown'); }); }); @@ -188,4 +195,77 @@ describe('ForestIntegrationClient', () => { await expect(client.dispose()).resolves.toBeUndefined(); }); }); + + describe('ForestIntegrationConfig.id is threaded as mcpServerId into integration tool factories', () => { + it('passes the config id to getZendeskTools so produced tools can be matched by step.mcpServerId', async () => { + const client = new ForestIntegrationClient([ + { + id: 'forest-zendesk-42', + integrationName: 'Zendesk', + config: { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' }, + isForestConnector: true, + }, + ]); + + await client.loadTools(); + + expect(getZendeskTools).toHaveBeenCalledWith( + expect.objectContaining({ subdomain: 'test' }), + 'forest-zendesk-42', + ); + }); + + it('passes the config id to getKolarTools', async () => { + const client = new ForestIntegrationClient([ + { + id: 'forest-kolar-7', + integrationName: 'Kolar', + config: { apiKey: 'key' }, + isForestConnector: true, + }, + ]); + + await client.loadTools(); + + expect(getKolarTools).toHaveBeenCalledWith( + expect.objectContaining({ apiKey: 'key' }), + 'forest-kolar-7', + ); + }); + + it('passes the config id to getSnowflakeTools', async () => { + const client = new ForestIntegrationClient([ + { + id: 'forest-snowflake-99', + integrationName: 'Snowflake', + config: { accountIdentifier: 'a', programmaticAccessToken: 'tok' }, + isForestConnector: true, + }, + ]); + + await client.loadTools(); + + expect(getSnowflakeTools).toHaveBeenCalledWith( + expect.objectContaining({ accountIdentifier: 'a' }), + 'forest-snowflake-99', + ); + }); + + it('passes undefined to the factory when the config entry has no id', async () => { + const client = new ForestIntegrationClient([ + { + integrationName: 'Zendesk', + config: { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' }, + isForestConnector: true, + }, + ]); + + await client.loadTools(); + + expect(getZendeskTools).toHaveBeenCalledWith( + expect.objectContaining({ subdomain: 'test' }), + undefined, + ); + }); + }); }); diff --git a/packages/ai-proxy/test/integrations/kolar/tools.test.ts b/packages/ai-proxy/test/integrations/kolar/tools.test.ts index df670e3c72..c4428e1161 100644 --- a/packages/ai-proxy/test/integrations/kolar/tools.test.ts +++ b/packages/ai-proxy/test/integrations/kolar/tools.test.ts @@ -26,4 +26,23 @@ describe('getKolarTools', () => { 'kolar_get_screening_result', ]); }); + + describe('mcpServerId propagation', () => { + it('sets RemoteTool.mcpServerId on every produced tool when mcpServerId is provided', () => { + const tools = getKolarTools(config, 'forest-kolar-7'); + + expect(tools).not.toHaveLength(0); + tools.forEach(tool => { + expect(tool.mcpServerId).toBe('forest-kolar-7'); + }); + }); + + it('leaves RemoteTool.mcpServerId undefined when no mcpServerId is provided', () => { + const tools = getKolarTools(config); + + tools.forEach(tool => { + expect(tool.mcpServerId).toBeUndefined(); + }); + }); + }); }); diff --git a/packages/ai-proxy/test/integrations/snowflake/tools.test.ts b/packages/ai-proxy/test/integrations/snowflake/tools.test.ts index 5e8336c23a..be75a9cb8c 100644 --- a/packages/ai-proxy/test/integrations/snowflake/tools.test.ts +++ b/packages/ai-proxy/test/integrations/snowflake/tools.test.ts @@ -28,4 +28,23 @@ describe('getSnowflakeTools', () => { 'snowflake_execute_query', ]); }); + + describe('mcpServerId propagation', () => { + it('sets RemoteTool.mcpServerId on every produced tool when mcpServerId is provided', () => { + const tools = getSnowflakeTools(config, 'forest-snowflake-99'); + + expect(tools).not.toHaveLength(0); + tools.forEach(tool => { + expect(tool.mcpServerId).toBe('forest-snowflake-99'); + }); + }); + + it('leaves RemoteTool.mcpServerId undefined when no mcpServerId is provided', () => { + const tools = getSnowflakeTools(config); + + tools.forEach(tool => { + expect(tool.mcpServerId).toBeUndefined(); + }); + }); + }); }); diff --git a/packages/ai-proxy/test/integrations/zendesk/tools.test.ts b/packages/ai-proxy/test/integrations/zendesk/tools.test.ts index b446bcb7ef..24bd2e0fc9 100644 --- a/packages/ai-proxy/test/integrations/zendesk/tools.test.ts +++ b/packages/ai-proxy/test/integrations/zendesk/tools.test.ts @@ -28,4 +28,23 @@ describe('getZendeskTools', () => { 'zendesk_update_ticket', ]); }); + + describe('mcpServerId propagation', () => { + it('sets RemoteTool.mcpServerId on every produced tool when mcpServerId is provided', () => { + const tools = getZendeskTools(config, 'forest-zendesk-42'); + + expect(tools).not.toHaveLength(0); + tools.forEach(tool => { + expect(tool.mcpServerId).toBe('forest-zendesk-42'); + }); + }); + + it('leaves RemoteTool.mcpServerId undefined when no mcpServerId is provided', () => { + const tools = getZendeskTools(config); + + tools.forEach(tool => { + expect(tool.mcpServerId).toBeUndefined(); + }); + }); + }); }); diff --git a/packages/ai-proxy/test/mcp-client.test.ts b/packages/ai-proxy/test/mcp-client.test.ts index cab68489be..ad128c2fbd 100644 --- a/packages/ai-proxy/test/mcp-client.test.ts +++ b/packages/ai-proxy/test/mcp-client.test.ts @@ -80,6 +80,51 @@ describe('McpClient', () => { }); }); + describe('mcpServerId threaded from config entry into McpServerRemoteTool', () => { + it('sets RemoteTool.mcpServerId from the config entry id alongside sourceId from the map key', async () => { + const tool1 = tool(() => {}, { + name: 'tool1', + description: 'description1', + schema: undefined, + responseFormat: 'content', + }); + const configWithId = { + configs: { + slack: { + transport: 'stdio' as const, + command: 'npx', + args: ['-y', '@modelcontextprotocol/server-slack'], + env: {}, + id: 'config-id-42', + }, + }, + } as unknown as McpConfiguration; + const mcpClient = new McpClient(configWithId); + getToolsMock.mockResolvedValue([tool1]); + + const tools = await mcpClient.loadTools(); + + expect(tools).toHaveLength(1); + expect(tools[0].sourceId).toBe('slack'); + expect(tools[0].mcpServerId).toBe('config-id-42'); + }); + + it('leaves RemoteTool.mcpServerId undefined when the config entry has no id (legacy / unenriched payload)', async () => { + const tool1 = tool(() => {}, { + name: 'tool1', + description: 'description1', + schema: undefined, + responseFormat: 'content', + }); + const mcpClient = new McpClient(aConfig); + getToolsMock.mockResolvedValue([tool1]); + + const tools = await mcpClient.loadTools(); + + expect(tools[0].mcpServerId).toBeUndefined(); + }); + }); + describe('when there is an error while loading the tools', () => { it('should not throw an error and try to load every mcp tools', async () => { const mcpClient = new McpClient({ @@ -396,6 +441,7 @@ describe('McpClient', () => { const configs = { server1: { type: 'http' as const, url: 'https://server1.com' }, zendesk: { + id: '1', isForestConnector: true as const, integrationName: 'Zendesk' as const, config: { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' }, diff --git a/packages/ai-proxy/test/tool-provider-factory.test.ts b/packages/ai-proxy/test/tool-provider-factory.test.ts index a3fbe44d7e..e56b436577 100644 --- a/packages/ai-proxy/test/tool-provider-factory.test.ts +++ b/packages/ai-proxy/test/tool-provider-factory.test.ts @@ -35,14 +35,12 @@ describe('createToolProviders', () => { const providers = createToolProviders(configs as any); expect(providers).toHaveLength(1); - expect(McpClient).toHaveBeenCalledWith( - { configs: { slack: configs.slack } }, - undefined, - ); + expect(McpClient).toHaveBeenCalledWith({ configs: { slack: configs.slack } }, undefined); }); it('should create ForestIntegrationClient for ForestIntegration configs', () => { const zendeskConfig = { + id: '1', isForestConnector: true as const, integrationName: 'Zendesk' as const, config: { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' }, @@ -58,6 +56,7 @@ describe('createToolProviders', () => { const configs = { slack: { command: 'npx', args: [] }, zendesk: { + id: '1', isForestConnector: true as const, integrationName: 'Zendesk' as const, config: { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' }, @@ -67,10 +66,7 @@ describe('createToolProviders', () => { const providers = createToolProviders(configs as any); expect(providers).toHaveLength(2); - expect(McpClient).toHaveBeenCalledWith( - { configs: { slack: configs.slack } }, - undefined, - ); + expect(McpClient).toHaveBeenCalledWith({ configs: { slack: configs.slack } }, undefined); expect(ForestIntegrationClient).toHaveBeenCalledWith([configs.zendesk], undefined); }); @@ -85,6 +81,7 @@ describe('createToolProviders', () => { const configs = { slack: { command: 'npx', args: [] }, zendesk: { + id: '1', isForestConnector: true as const, integrationName: 'Zendesk' as const, config: { subdomain: 'test', email: 'a@b.com', apiToken: 'tok' }, diff --git a/packages/workflow-executor/src/errors.ts b/packages/workflow-executor/src/errors.ts index 6c1090d5d8..4aeb7f4618 100644 --- a/packages/workflow-executor/src/errors.ts +++ b/packages/workflow-executor/src/errors.ts @@ -203,8 +203,13 @@ export class StepTimeoutError extends WorkflowExecutorError { } export class NoMcpToolsError extends WorkflowExecutorError { - constructor() { - super('No MCP tools available', 'No tools are available to execute this step.'); + constructor(requestedMcpServerId?: string, loadedMcpServerIds?: readonly string[]) { + const technical = requestedMcpServerId + ? `No MCP tools available for mcpServerId="${requestedMcpServerId}". Loaded MCP server ids: [${( + loadedMcpServerIds ?? [] + ).join(', ')}]` + : 'No MCP tools available'; + super(technical, 'No tools are available to execute this step.'); } } diff --git a/packages/workflow-executor/src/executors/mcp-step-executor.ts b/packages/workflow-executor/src/executors/mcp-step-executor.ts index fca94387a5..2723976888 100644 --- a/packages/workflow-executor/src/executors/mcp-step-executor.ts +++ b/packages/workflow-executor/src/executors/mcp-step-executor.ts @@ -224,13 +224,26 @@ export default class McpStepExecutor extends BaseStepExecutor ); } - /** Returns tools filtered by mcpServerId (if specified). Throws NoMcpToolsError if empty. */ private getFilteredTools(): RemoteTool[] { const { mcpServerId } = this.context.stepDefinition; const tools = mcpServerId - ? this.remoteTools.filter(t => t.sourceId === mcpServerId) + ? this.remoteTools.filter(t => t.mcpServerId === mcpServerId) : [...this.remoteTools]; - if (tools.length === 0) throw new NoMcpToolsError(); + + if (tools.length === 0) { + const loadedMcpServerIds = this.remoteTools + .map(t => t.mcpServerId) + .filter((value): value is string => !!value); + const error = new NoMcpToolsError(mcpServerId, loadedMcpServerIds); + this.context.logger.error(error.message, { + runId: this.context.runId, + stepId: this.context.stepId, + stepIndex: this.context.stepIndex, + requestedMcpServerId: mcpServerId, + loadedMcpServerIds, + }); + throw error; + } return tools; } diff --git a/packages/workflow-executor/test/errors.test.ts b/packages/workflow-executor/test/errors.test.ts index a936567012..369d99f46a 100644 --- a/packages/workflow-executor/test/errors.test.ts +++ b/packages/workflow-executor/test/errors.test.ts @@ -1,4 +1,4 @@ -import { extractErrorMessage } from '../src/errors'; +import { NoMcpToolsError, extractErrorMessage } from '../src/errors'; describe('extractErrorMessage', () => { it('returns err.message when non-empty', () => { @@ -58,3 +58,39 @@ describe('extractErrorMessage', () => { expect(extractErrorMessage(err)).toBe('from cause'); }); }); + +describe('NoMcpToolsError', () => { + it('produces a fully generic technical message when no mcpServerId was requested (no filter case)', () => { + const err = new NoMcpToolsError(); + + expect(err.message).toBe('No MCP tools available'); + expect(err.userMessage).toBe('No tools are available to execute this step.'); + }); + + it('includes the requested mcpServerId in the technical message when a filter was active', () => { + const err = new NoMcpToolsError('id-missing', ['id-A', 'id-B']); + + expect(err.message).toMatch(/id-missing/); + }); + + it('lists the loaded mcpServerIds in the technical message so misconfigurations are diagnosable', () => { + const err = new NoMcpToolsError('id-missing', ['id-A', 'id-B']); + + expect(err.message).toMatch(/id-A/); + expect(err.message).toMatch(/id-B/); + }); + + it('handles an empty loaded-id list without producing a malformed message', () => { + const err = new NoMcpToolsError('id-missing', []); + + expect(err.message).toMatch(/id-missing/); + expect(err.message).not.toMatch(/undefined|null|\[object/i); + }); + + it('keeps the user-facing message generic — no internal ids must leak', () => { + const err = new NoMcpToolsError('id-missing', ['id-A', 'id-B']); + + expect(err.userMessage).toBe('No tools are available to execute this step.'); + expect(err.userMessage).not.toMatch(/id-missing|id-A|id-B/); + }); +}); diff --git a/packages/workflow-executor/test/executors/mcp-step-executor.test.ts b/packages/workflow-executor/test/executors/mcp-step-executor.test.ts index e58aa8346e..96c3d393a9 100644 --- a/packages/workflow-executor/test/executors/mcp-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/mcp-step-executor.test.ts @@ -16,7 +16,12 @@ import { StepType } from '../../src/types/validated/step-definition'; // --------------------------------------------------------------------------- class MockRemoteTool extends RemoteTool { - constructor(options: { name: string; sourceId?: string; invoke?: jest.Mock }) { + constructor(options: { + name: string; + sourceId?: string; + mcpServerId?: string; + invoke?: jest.Mock; + }) { const invokeFn = options.invoke ?? jest.fn().mockResolvedValue('tool-result'); super({ tool: { @@ -27,6 +32,7 @@ class MockRemoteTool extends RemoteTool { } as unknown as RemoteTool['base'], sourceId: options.sourceId ?? 'mcp-server-1', sourceType: 'mcp', + mcpServerId: options.mcpServerId, }); } } @@ -439,14 +445,23 @@ describe('McpStepExecutor', () => { }); }); - describe('mcpServerId filter', () => { - it('passes only tools from the specified server to the AI', async () => { - const toolA = new MockRemoteTool({ name: 'tool_a', sourceId: 'server-A' }); - const toolB = new MockRemoteTool({ name: 'tool_b', sourceId: 'server-B' }); + describe('mcpServerId filter (matches by tool.mcpServerId, not tool.sourceId)', () => { + it('passes only tools whose mcpServerId matches step.mcpServerId to the AI', async () => { + const toolA = new MockRemoteTool({ + name: 'tool_a', + sourceId: 'zendesk', + mcpServerId: 'id-A', + }); + const toolB = new MockRemoteTool({ + name: 'tool_b', + sourceId: 'zendesk', + mcpServerId: 'id-B', + }); const invokeFn = jest.fn().mockResolvedValue('ok'); const toolB2 = new MockRemoteTool({ name: 'tool_b2', - sourceId: 'server-B', + sourceId: 'zendesk', + mcpServerId: 'id-B', invoke: invokeFn, }); @@ -455,19 +470,86 @@ describe('McpStepExecutor', () => { const context = makeContext({ model, runStore, - stepDefinition: makeStep({ mcpServerId: 'server-B', automaticExecution: true }), + stepDefinition: makeStep({ mcpServerId: 'id-B', automaticExecution: true }), }); const executor = new McpStepExecutor(context, [toolA, toolB, toolB2]); await executor.execute(); - // bindTools should only receive server-B tools const boundTools = bindTools.mock.calls[0][0] as Array<{ name: string }>; const boundNames = boundTools.map(t => t.name); expect(boundNames).not.toContain('tool_a'); expect(boundNames).toContain('tool_b'); expect(boundNames).toContain('tool_b2'); }); + + it('does not match by sourceId — server-name collisions must not leak tools across configs', async () => { + const tool = new MockRemoteTool({ + name: 'tool_a', + sourceId: 'server-B', + mcpServerId: 'id-99', + }); + const context = makeContext({ + stepDefinition: makeStep({ mcpServerId: 'server-B' }), + }); + const executor = new McpStepExecutor(context, [tool]); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome.error).toBe('No tools are available to execute this step.'); + }); + + it('returns all tools (no filter) when step.mcpServerId is absent', async () => { + const toolA = new MockRemoteTool({ + name: 'tool_a', + sourceId: 'server-A', + mcpServerId: 'id-A', + }); + const toolB = new MockRemoteTool({ + name: 'tool_b', + sourceId: 'server-B', + mcpServerId: 'id-B', + }); + const { model, bindTools } = makeMockModel('tool_a', {}); + const context = makeContext({ + model, + stepDefinition: makeStep({ automaticExecution: true }), + }); + const executor = new McpStepExecutor(context, [toolA, toolB]); + + await executor.execute(); + + const boundTools = bindTools.mock.calls[0][0] as Array<{ name: string }>; + const boundNames = boundTools.map(t => t.name); + expect(boundNames).toEqual(expect.arrayContaining(['tool_a', 'tool_b'])); + }); + + it('resolves a Forest-connector-backed tool when its mcpServerId is threaded through', async () => { + const invokeFn = jest.fn().mockResolvedValue('done'); + const forestTool = new MockRemoteTool({ + name: 'zendesk_get_tickets', + sourceId: 'zendesk', + mcpServerId: 'forest-connector-42', + invoke: invokeFn, + }); + const { model, bindTools } = makeMockModel('zendesk_get_tickets', {}); + const context = makeContext({ + model, + stepDefinition: makeStep({ + mcpServerId: 'forest-connector-42', + automaticExecution: true, + }), + }); + const executor = new McpStepExecutor(context, [forestTool]); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('success'); + const boundTools = bindTools.mock.calls[0][0] as Array<{ name: string }>; + expect(boundTools.map(t => t.name)).toEqual(['zendesk_get_tickets']); + expect(invokeFn).toHaveBeenCalled(); + }); }); describe('NoMcpToolsError', () => { @@ -482,9 +564,13 @@ describe('McpStepExecutor', () => { }); it('returns error when mcpServerId filter yields no tools', async () => { - const tool = new MockRemoteTool({ name: 'tool_a', sourceId: 'server-A' }); + const tool = new MockRemoteTool({ + name: 'tool_a', + sourceId: 'server-A', + mcpServerId: 'id-A', + }); const context = makeContext({ - stepDefinition: makeStep({ mcpServerId: 'server-B' }), + stepDefinition: makeStep({ mcpServerId: 'id-B' }), }); const executor = new McpStepExecutor(context, [tool]); @@ -493,6 +579,50 @@ describe('McpStepExecutor', () => { expect(result.stepOutcome.status).toBe('error'); expect(result.stepOutcome.error).toBe('No tools are available to execute this step.'); }); + + it('keeps the user-facing error message generic regardless of the misconfigured mcpServerId', async () => { + const tool = new MockRemoteTool({ + name: 'tool_a', + sourceId: 'server-A', + mcpServerId: 'id-A', + }); + const context = makeContext({ stepDefinition: makeStep({ mcpServerId: 'id-B' }) }); + const executor = new McpStepExecutor(context, [tool]); + + const result = await executor.execute(); + + expect(result.stepOutcome.error).toBe('No tools are available to execute this step.'); + expect(result.stepOutcome.error).not.toMatch(/id-B/); + }); + + it('logs the technical message with the requested mcpServerId and loaded mcpServerIds when filter misses', async () => { + const logger = { info: jest.fn(), error: jest.fn() }; + const toolA = new MockRemoteTool({ + name: 'tool_a', + sourceId: 'server-A', + mcpServerId: 'id-A', + }); + const toolB = new MockRemoteTool({ + name: 'tool_b', + sourceId: 'server-B', + mcpServerId: 'id-B', + }); + const context = makeContext({ + logger, + stepDefinition: makeStep({ mcpServerId: 'id-missing' }), + }); + const executor = new McpStepExecutor(context, [toolA, toolB]); + + await executor.execute(); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringMatching(/id-missing/), + expect.objectContaining({ + requestedMcpServerId: 'id-missing', + loadedMcpServerIds: expect.arrayContaining(['id-A', 'id-B']), + }), + ); + }); }); describe('McpToolNotFoundError', () => {