From 1f8c410e41aa0b1904b97b05542a1756c9b3514c Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 2 Mar 2026 11:25:53 +0000 Subject: [PATCH] feat: add serverInstructions support to MCP server config Adds an optional serverInstructions field to the MCP server configuration schema, allowing users to define custom instructions per server in mcp_settings.json. Config-defined instructions take precedence and are combined with protocol-provided instructions when both exist. Closes #11825 --- src/services/mcp/McpHub.ts | 13 +- src/services/mcp/__tests__/McpHub.spec.ts | 167 ++++++++++++++++++++++ 2 files changed, 179 insertions(+), 1 deletion(-) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index ea38ee02d6d..1836c4a7930 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -70,6 +70,7 @@ const BaseConfigSchema = z.object({ alwaysAllow: z.array(z.string()).default([]), watchPaths: z.array(z.string()).optional(), // paths to watch for changes and restart server disabledTools: z.array(z.string()).default([]), + serverInstructions: z.string().optional(), // user-defined instructions that supplement or override protocol-provided instructions }) // Custom error messages for better user feedback @@ -632,6 +633,7 @@ export class McpHub { disabled: reason === DisableReason.SERVER_DISABLED ? true : config.disabled, source, projectPath: source === "project" ? vscode.workspace.workspaceFolders?.[0]?.uri.fsPath : undefined, + instructions: config.serverInstructions, errorHistory: [], }, client: null, @@ -878,7 +880,16 @@ export class McpHub { await client.connect(transport) connection.server.status = "connected" connection.server.error = "" - connection.server.instructions = client.getInstructions() + + // Merge user-defined serverInstructions from config with protocol-provided instructions. + // Config-defined serverInstructions take precedence; if both exist, they are combined. + const protocolInstructions = client.getInstructions() + const configInstructions = configInjected.serverInstructions + if (configInstructions && protocolInstructions) { + connection.server.instructions = `${configInstructions}\n\n${protocolInstructions}` + } else { + connection.server.instructions = configInstructions || protocolInstructions + } // Initial fetch of tools and resources connection.server.tools = await this.fetchToolsList(name, source) diff --git a/src/services/mcp/__tests__/McpHub.spec.ts b/src/services/mcp/__tests__/McpHub.spec.ts index 3f06627cc17..e43da38e0f1 100644 --- a/src/services/mcp/__tests__/McpHub.spec.ts +++ b/src/services/mcp/__tests__/McpHub.spec.ts @@ -2019,6 +2019,173 @@ describe("McpHub", () => { }) }) + describe("serverInstructions config support", () => { + it("should accept serverInstructions in config schema", () => { + const result = ServerConfigSchema.safeParse({ + command: "node", + args: ["test.js"], + serverInstructions: "Custom instructions for this server", + }) + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.serverInstructions).toBe("Custom instructions for this server") + } + }) + + it("should accept config without serverInstructions (optional field)", () => { + const result = ServerConfigSchema.safeParse({ + command: "node", + args: ["test.js"], + }) + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.serverInstructions).toBeUndefined() + } + }) + + it("should accept serverInstructions for SSE config", () => { + const result = ServerConfigSchema.safeParse({ + type: "sse", + url: "http://localhost:3000/sse", + serverInstructions: "SSE server custom instructions", + }) + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.serverInstructions).toBe("SSE server custom instructions") + } + }) + + it("should accept serverInstructions for streamable-http config", () => { + const result = ServerConfigSchema.safeParse({ + type: "streamable-http", + url: "http://localhost:3000/mcp", + serverInstructions: "Streamable HTTP server custom instructions", + }) + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.serverInstructions).toBe("Streamable HTTP server custom instructions") + } + }) + + it("should use config serverInstructions when protocol provides none", async () => { + // Mock StdioClientTransport + const stdioModule = await import("@modelcontextprotocol/sdk/client/stdio.js") + const StdioClientTransport = stdioModule.StdioClientTransport as ReturnType + + const mockTransport = { + start: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + stderr: { on: vi.fn() }, + onerror: null, + onclose: null, + } + + StdioClientTransport.mockImplementation(() => mockTransport) + + // Mock Client - returns undefined for getInstructions (no protocol instructions) + const clientModule = await import("@modelcontextprotocol/sdk/client/index.js") + const Client = clientModule.Client as ReturnType + + Client.mockImplementation(() => ({ + connect: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + getInstructions: vi.fn().mockReturnValue(undefined), + request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }), + })) + + vi.mocked(fs.readFile).mockResolvedValue( + JSON.stringify({ + mcpServers: { + "instructions-test-server": { + command: "node", + args: ["test.js"], + serverInstructions: "Config-defined instructions", + }, + }, + }), + ) + + const mcpHub = new McpHub(mockProvider as ClineProvider) + await new Promise((resolve) => setTimeout(resolve, 100)) + + const connection = mcpHub.connections.find((conn) => conn.server.name === "instructions-test-server") + expect(connection).toBeDefined() + expect(connection?.server.instructions).toBe("Config-defined instructions") + }) + + it("should combine config serverInstructions with protocol instructions", async () => { + // Mock StdioClientTransport + const stdioModule = await import("@modelcontextprotocol/sdk/client/stdio.js") + const StdioClientTransport = stdioModule.StdioClientTransport as ReturnType + + const mockTransport = { + start: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + stderr: { on: vi.fn() }, + onerror: null, + onclose: null, + } + + StdioClientTransport.mockImplementation(() => mockTransport) + + // Mock Client - returns protocol instructions + const clientModule = await import("@modelcontextprotocol/sdk/client/index.js") + const Client = clientModule.Client as ReturnType + + Client.mockImplementation(() => ({ + connect: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + getInstructions: vi.fn().mockReturnValue("Protocol instructions from server"), + request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }), + })) + + vi.mocked(fs.readFile).mockResolvedValue( + JSON.stringify({ + mcpServers: { + "combined-instructions-server": { + command: "node", + args: ["test.js"], + serverInstructions: "Config-defined instructions", + }, + }, + }), + ) + + const mcpHub = new McpHub(mockProvider as ClineProvider) + await new Promise((resolve) => setTimeout(resolve, 100)) + + const connection = mcpHub.connections.find((conn) => conn.server.name === "combined-instructions-server") + expect(connection).toBeDefined() + // Config instructions should come first, followed by protocol instructions + expect(connection?.server.instructions).toBe( + "Config-defined instructions\n\nProtocol instructions from server", + ) + }) + + it("should set serverInstructions on placeholder connections for disabled servers", async () => { + vi.mocked(fs.readFile).mockResolvedValue( + JSON.stringify({ + mcpServers: { + "disabled-instructions-server": { + command: "node", + args: ["test.js"], + disabled: true, + serverInstructions: "Instructions for disabled server", + }, + }, + }), + ) + + const mcpHub = new McpHub(mockProvider as ClineProvider) + await new Promise((resolve) => setTimeout(resolve, 100)) + + const connection = mcpHub.connections.find((conn) => conn.server.name === "disabled-instructions-server") + expect(connection).toBeDefined() + expect(connection?.type).toBe("disconnected") + expect(connection?.server.instructions).toBe("Instructions for disabled server") + }) + }) + describe("Windows command wrapping", () => { let StdioClientTransport: ReturnType let Client: ReturnType