diff --git a/packages/agents-a365-tooling/src/McpToolServerConfigurationService.ts b/packages/agents-a365-tooling/src/McpToolServerConfigurationService.ts index 3fd16f9..5a693a2 100644 --- a/packages/agents-a365-tooling/src/McpToolServerConfigurationService.ts +++ b/packages/agents-a365-tooling/src/McpToolServerConfigurationService.ts @@ -4,7 +4,7 @@ import fs from 'fs'; import path from 'path'; import axios from 'axios'; -import { MCPServerConfig, McpClientTool, ToolOptions } from './contracts'; +import { MCPServerConfig, MCPServerManifestEntry, McpClientTool, ToolOptions } from './contracts'; import { Utility } from './Utility'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; @@ -129,9 +129,17 @@ export class McpToolServerConfigurationService { * { * "mcpServerName": "sharePointMCPServerConfig", * "mcpServerUniqueName": "mcp_SharePointTools" + * }, + * { + * "mcpServerName": "customMCPServer", + * "url": "http://localhost:3000/mcp" * } * ] * } + * + * Each server entry can optionally include a "url" field to specify a custom MCP server URL. + * If the "url" field is not provided, the URL will be automatically constructed using the server name. + * The server name is determined by using "mcpServerName" if present, otherwise "mcpServerUniqueName". */ private async getMCPServerConfigsFromManifest(): Promise { let manifestPath = path.join(process.cwd(), 'ToolingManifest.json'); @@ -150,10 +158,16 @@ export class McpToolServerConfigurationService { const manifestData = JSON.parse(jsonContent); const mcpServers = manifestData.mcpServers || []; - return mcpServers.map((s: MCPServerConfig) => { + return mcpServers.map((s: MCPServerManifestEntry) => { + // Use mcpServerName if available, otherwise fall back to mcpServerUniqueName + const serverName = s.mcpServerName || s.mcpServerUniqueName; + if (!serverName) { + throw new Error('Either mcpServerName or mcpServerUniqueName must be provided in manifest entry'); + } return { - mcpServerName: s.mcpServerName, - url: Utility.BuildMcpServerUrl(s.mcpServerName) + mcpServerName: serverName, + url: s.url || Utility.BuildMcpServerUrl(serverName), + headers: s.headers }; }); } catch (err: unknown) { diff --git a/packages/agents-a365-tooling/src/contracts.ts b/packages/agents-a365-tooling/src/contracts.ts index 79831a5..fe18889 100644 --- a/packages/agents-a365-tooling/src/contracts.ts +++ b/packages/agents-a365-tooling/src/contracts.ts @@ -7,6 +7,13 @@ export interface MCPServerConfig { headers?: Record; } +export interface MCPServerManifestEntry { + mcpServerName?: string; + mcpServerUniqueName?: string; + url?: string; + headers?: Record; +} + export interface McpClientTool { name: string; description?: string; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3bb19b9..9b8bbb7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -87,6 +87,9 @@ catalogs: '@typescript-eslint/parser': specifier: ^8.47.0 version: 8.47.0 + cross-env: + specifier: ^7.0.3 + version: 7.0.3 dotenv: specifier: ^17.2.2 version: 17.2.3 @@ -699,6 +702,9 @@ importers: '@typescript-eslint/parser': specifier: 'catalog:' version: 8.47.0(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3) + cross-env: + specifier: 'catalog:' + version: 7.0.3 eslint: specifier: 'catalog:' version: 9.39.1(jiti@2.6.1) @@ -2322,6 +2328,11 @@ packages: create-require@1.1.1: resolution: {integrity: sha512-dcKFX3jn0MpIaXjisoRvexIJVEKzaq7z2rZKxf+MSr9TkdmHmsU4m2lcLojrj/FHl8mk5VxMmYA+ftRkP/3oKQ==} + cross-env@7.0.3: + resolution: {integrity: sha512-+/HKd6EgcQCJGh2PSjZuUitQBQynKor4wrFbRg4DtAgS1aWO+gU52xpH7M9ScGgXSYmAVS9bIJ8EzuaGw0oNAw==} + engines: {node: '>=10.14', npm: '>=6', yarn: '>=1'} + hasBin: true + cross-spawn@7.0.6: resolution: {integrity: sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==} engines: {node: '>= 8'} @@ -5807,6 +5818,10 @@ snapshots: create-require@1.1.1: {} + cross-env@7.0.3: + dependencies: + cross-spawn: 7.0.6 + cross-spawn@7.0.6: dependencies: path-key: 3.1.1 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index a78ab54..ed0ad2a 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -56,6 +56,7 @@ catalog: "@types/node": "^20.17.0" "@typescript-eslint/eslint-plugin": "^8.47.0" "@typescript-eslint/parser": "^8.47.0" + "cross-env": "^7.0.3" "dotenv": "^17.2.2" "eslint": "^9.39.1" "jest": "^30.2.0" diff --git a/tests/package.json b/tests/package.json index 90f8870..065306e 100644 --- a/tests/package.json +++ b/tests/package.json @@ -6,11 +6,11 @@ "main": "dist/index.js", "types": "dist/index.d.ts", "scripts": { - "test": "jest --config jest.config.cjs --passWithNoTests --testPathIgnorePatterns=/integration/", - "test:watch": "jest --config jest.config.cjs --watch --testPathIgnorePatterns=/integration/", - "test:coverage": "jest --config jest.config.cjs --coverage --passWithNoTests --testPathIgnorePatterns=/integration/", - "test:verbose": "jest --config jest.config.cjs --verbose --passWithNoTests --testPathIgnorePatterns=/integration/", - "test:ci": "jest --config jest.config.cjs --coverage --ci --maxWorkers=2 --passWithNoTests --testPathIgnorePatterns=/integration/" + "test": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --config jest.config.cjs --passWithNoTests --testPathIgnorePatterns=/integration/", + "test:watch": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --config jest.config.cjs --watch --testPathIgnorePatterns=/integration/", + "test:coverage": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --config jest.config.cjs --coverage --passWithNoTests --testPathIgnorePatterns=/integration/", + "test:verbose": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --config jest.config.cjs --verbose --passWithNoTests --testPathIgnorePatterns=/integration/", + "test:ci": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --config jest.config.cjs --coverage --ci --maxWorkers=2 --passWithNoTests --testPathIgnorePatterns=/integration/" }, "keywords": [ "agents", @@ -50,6 +50,7 @@ "@types/node": "catalog:", "@typescript-eslint/eslint-plugin": "catalog:", "@typescript-eslint/parser": "catalog:", + "cross-env": "catalog:", "eslint": "catalog:", "jest": "catalog:", "js-yaml": "catalog:", diff --git a/tests/tooling/McpToolServerConfigurationService.test.ts b/tests/tooling/McpToolServerConfigurationService.test.ts new file mode 100644 index 0000000..ce9b20c --- /dev/null +++ b/tests/tooling/McpToolServerConfigurationService.test.ts @@ -0,0 +1,266 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, it, expect, jest, beforeEach, afterEach } from '@jest/globals'; +import { McpToolServerConfigurationService } from '../../packages/agents-a365-tooling/src/McpToolServerConfigurationService'; +import { Utility } from '../../packages/agents-a365-tooling/src/Utility'; +import fs from 'fs'; + +describe('McpToolServerConfigurationService', () => { + let service: McpToolServerConfigurationService; + const originalEnv = process.env; + + beforeEach(() => { + service = new McpToolServerConfigurationService(); + process.env = { ...originalEnv }; + // Set to development mode to read from manifest + process.env.NODE_ENV = 'Development'; + }); + + afterEach(() => { + process.env = originalEnv; + jest.restoreAllMocks(); + }); + + describe('listToolServers with custom URLs', () => { + it('should use custom URL when provided in manifest', async () => { + // Arrange + const manifestContent = { + mcpServers: [ + { + mcpServerName: 'customServer', + url: 'http://localhost:3000/custom-mcp' + } + ] + }; + + jest.spyOn(fs, 'existsSync').mockReturnValue(true); + jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent)); + + // Act + const servers = await service.listToolServers('test-agent-id', 'mock-auth-token'); + + // Assert + expect(servers).toHaveLength(1); + expect(servers[0].mcpServerName).toBe('customServer'); + expect(servers[0].url).toBe('http://localhost:3000/custom-mcp'); + }); + + it('should build URL when not provided in manifest', async () => { + // Arrange + const manifestContent = { + mcpServers: [ + { + mcpServerName: 'mcp_MailTools' + } + ] + }; + + jest.spyOn(fs, 'existsSync').mockReturnValue(true); + jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent)); + + // Act + const servers = await service.listToolServers('test-agent-id', 'mock-auth-token'); + + // Assert + expect(servers).toHaveLength(1); + expect(servers[0].mcpServerName).toBe('mcp_MailTools'); + // In development mode, should use mock server URL + expect(servers[0].url).toBe(Utility.BuildMcpServerUrl('mcp_MailTools')); + }); + + it('should handle mix of custom and default URLs in manifest', async () => { + // Arrange + const manifestContent = { + mcpServers: [ + { + mcpServerName: 'customServer', + url: 'https://custom.example.com/mcp' + }, + { + mcpServerName: 'mcp_MailTools' + }, + { + mcpServerName: 'anotherCustom', + url: 'http://localhost:5000/mcp-server' + } + ] + }; + + jest.spyOn(fs, 'existsSync').mockReturnValue(true); + jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent)); + + // Act + const servers = await service.listToolServers('test-agent-id', 'mock-auth-token'); + + // Assert + expect(servers).toHaveLength(3); + + // First server has custom URL + expect(servers[0].mcpServerName).toBe('customServer'); + expect(servers[0].url).toBe('https://custom.example.com/mcp'); + + // Second server uses default URL building + expect(servers[1].mcpServerName).toBe('mcp_MailTools'); + expect(servers[1].url).toBe(Utility.BuildMcpServerUrl('mcp_MailTools')); + + // Third server has custom URL + expect(servers[2].mcpServerName).toBe('anotherCustom'); + expect(servers[2].url).toBe('http://localhost:5000/mcp-server'); + }); + + it('should return empty array when manifest file does not exist', async () => { + // Arrange + jest.spyOn(fs, 'existsSync').mockReturnValue(false); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + // Act + const servers = await service.listToolServers('test-agent-id', 'mock-auth-token'); + + // Assert + expect(servers).toHaveLength(0); + expect(consoleWarnSpy).toHaveBeenCalled(); + }); + + it('should handle empty mcpServers array in manifest', async () => { + // Arrange + const manifestContent = { + mcpServers: [] + }; + + jest.spyOn(fs, 'existsSync').mockReturnValue(true); + jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent)); + + // Act + const servers = await service.listToolServers('test-agent-id', 'mock-auth-token'); + + // Assert + expect(servers).toHaveLength(0); + }); + + it('should handle missing mcpServers property in manifest', async () => { + // Arrange + const manifestContent = {}; + + jest.spyOn(fs, 'existsSync').mockReturnValue(true); + jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent)); + + // Act + const servers = await service.listToolServers('test-agent-id', 'mock-auth-token'); + + // Assert + expect(servers).toHaveLength(0); + }); + + it('should use mcpServerUniqueName as fallback when mcpServerName is not provided', async () => { + // Arrange + const manifestContent = { + mcpServers: [ + { + mcpServerUniqueName: 'mcp_UniqueServer' + } + ] + }; + + jest.spyOn(fs, 'existsSync').mockReturnValue(true); + jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent)); + + // Act + const servers = await service.listToolServers('test-agent-id', 'mock-auth-token'); + + // Assert + expect(servers).toHaveLength(1); + expect(servers[0].mcpServerName).toBe('mcp_UniqueServer'); + expect(servers[0].url).toBe(Utility.BuildMcpServerUrl('mcp_UniqueServer')); + }); + + it('should prefer mcpServerName over mcpServerUniqueName when both are provided', async () => { + // Arrange + const manifestContent = { + mcpServers: [ + { + mcpServerName: 'mcp_PreferredName', + mcpServerUniqueName: 'mcp_FallbackName' + } + ] + }; + + jest.spyOn(fs, 'existsSync').mockReturnValue(true); + jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent)); + + // Act + const servers = await service.listToolServers('test-agent-id', 'mock-auth-token'); + + // Assert + expect(servers).toHaveLength(1); + expect(servers[0].mcpServerName).toBe('mcp_PreferredName'); + expect(servers[0].url).toBe(Utility.BuildMcpServerUrl('mcp_PreferredName')); + }); + + it('should return empty array and log error when neither mcpServerName nor mcpServerUniqueName is provided', async () => { + // Arrange + const manifestContent = { + mcpServers: [ + { + url: 'http://localhost:3000/custom-mcp' + } + ] + }; + + jest.spyOn(fs, 'existsSync').mockReturnValue(true); + jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent)); + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + // Act + const servers = await service.listToolServers('test-agent-id', 'mock-auth-token'); + + // Assert + expect(servers).toHaveLength(0); + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('Either mcpServerName or mcpServerUniqueName must be provided') + ); + }); + + it('should preserve custom headers when provided in manifest', async () => { + // Arrange + const manifestContent = { + mcpServers: [ + { + mcpServerName: 'serverWithHeaders', + url: 'http://localhost:3000/custom-mcp', + headers: { + 'Authorization': 'Bearer token123', + 'X-Custom-Header': 'custom-value' + } + }, + { + mcpServerName: 'serverWithoutHeaders', + url: 'http://localhost:4000/another-mcp' + } + ] + }; + + jest.spyOn(fs, 'existsSync').mockReturnValue(true); + jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent)); + + // Act + const servers = await service.listToolServers('test-agent-id', 'mock-auth-token'); + + // Assert + expect(servers).toHaveLength(2); + + // First server should have headers preserved + expect(servers[0].mcpServerName).toBe('serverWithHeaders'); + expect(servers[0].url).toBe('http://localhost:3000/custom-mcp'); + expect(servers[0].headers).toEqual({ + 'Authorization': 'Bearer token123', + 'X-Custom-Header': 'custom-value' + }); + + // Second server should have undefined headers + expect(servers[1].mcpServerName).toBe('serverWithoutHeaders'); + expect(servers[1].url).toBe('http://localhost:4000/another-mcp'); + expect(servers[1].headers).toBeUndefined(); + }); + }); +});