From 6413af3ca06ea331e04ec43f1fb2cfd49f693a23 Mon Sep 17 00:00:00 2001 From: "A.K.M. Adib" Date: Tue, 3 Feb 2026 15:33:59 -0500 Subject: [PATCH 1/8] complete --- .../cli/src/ui/commands/mcpCommand.test.ts | 1 + packages/core/src/core/prompts.test.ts | 30 ++++ packages/core/src/prompts/promptProvider.ts | 28 +++- packages/core/src/telemetry/loggers.test.ts | 1 + packages/core/src/tools/mcp-client.test.ts | 152 ++++++++++++++++++ packages/core/src/tools/mcp-client.ts | 18 +++ packages/core/src/tools/mcp-tool.test.ts | 5 + packages/core/src/tools/mcp-tool.ts | 2 + 8 files changed, 232 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/ui/commands/mcpCommand.test.ts b/packages/cli/src/ui/commands/mcpCommand.test.ts index 83b5dbb1799..ecce5c9cd5d 100644 --- a/packages/cli/src/ui/commands/mcpCommand.test.ts +++ b/packages/cli/src/ui/commands/mcpCommand.test.ts @@ -60,6 +60,7 @@ const createMockMCPTool = ( { type: 'object', properties: {} }, mockMessageBus, undefined, // trust + undefined, // isReadOnly undefined, // nameOverride undefined, // cliConfig undefined, // extensionName diff --git a/packages/core/src/core/prompts.test.ts b/packages/core/src/core/prompts.test.ts index 591d63dec70..c4af0986ed3 100644 --- a/packages/core/src/core/prompts.test.ts +++ b/packages/core/src/core/prompts.test.ts @@ -22,6 +22,9 @@ import { DEFAULT_GEMINI_MODEL, } from '../config/models.js'; import { ApprovalMode } from '../policy/types.js'; +import { DiscoveredMCPTool } from '../tools/mcp-tool.js'; +import type { CallableTool } from '@google/genai'; +import type { MessageBus } from '../confirmation-bus/message-bus.js'; // Mock tool names if they are dynamically generated or complex vi.mock('../tools/ls', () => ({ LSTool: { Name: 'list_directory' } })); @@ -62,6 +65,7 @@ describe('Core System Prompt (prompts.ts)', () => { mockConfig = { getToolRegistry: vi.fn().mockReturnValue({ getAllToolNames: vi.fn().mockReturnValue([]), + getAllTools: vi.fn().mockReturnValue([]), }), getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true), storage: { @@ -272,6 +276,32 @@ describe('Core System Prompt (prompts.ts)', () => { expect(prompt).toMatchSnapshot(); }); + it('should include read-only MCP tools in PLAN mode', () => { + vi.mocked(mockConfig.getApprovalMode).mockReturnValue(ApprovalMode.PLAN); + + const mcpTool = new DiscoveredMCPTool( + {} as CallableTool, + 'readonly-server', + 'read_static_value', + 'A read-only tool', + {}, + {} as MessageBus, + false, + true, // isReadOnly + ); + + vi.mocked(mockConfig.getToolRegistry().getAllTools).mockReturnValue([ + mcpTool, + ]); + vi.mocked(mockConfig.getToolRegistry().getAllToolNames).mockReturnValue([ + mcpTool.name, + ]); + + const prompt = getCoreSystemPrompt(mockConfig); + + expect(prompt).toContain('`read_static_value` (readonly-server)'); + }); + it('should only list available tools in PLAN mode', () => { vi.mocked(mockConfig.getApprovalMode).mockReturnValue(ApprovalMode.PLAN); // Only enable a subset of tools, including ask_user diff --git a/packages/core/src/prompts/promptProvider.ts b/packages/core/src/prompts/promptProvider.ts index aa02b70a4a4..20889b06635 100644 --- a/packages/core/src/prompts/promptProvider.ts +++ b/packages/core/src/prompts/promptProvider.ts @@ -25,6 +25,7 @@ import { READ_FILE_TOOL_NAME, } from '../tools/tool-names.js'; import { resolveModel, isPreviewModel } from '../config/models.js'; +import { DiscoveredMCPTool } from '../tools/mcp-tool.js'; /** * Orchestrates prompt generation by gathering context and building options. @@ -55,13 +56,30 @@ export class PromptProvider { const isGemini3 = isPreviewModel(desiredModel); // --- Context Gathering --- + let planModeToolsList = PLAN_MODE_TOOLS.filter((t) => + new Set(toolNames).has(t), + ) + .map((t) => `- \`${t}\``) + .join('\n'); + + // Add read-only MCP tools to the list + if (isPlanMode) { + const allTools = config.getToolRegistry().getAllTools(); + const readOnlyMcpTools = allTools.filter( + (t): t is DiscoveredMCPTool => + t instanceof DiscoveredMCPTool && !!t.isReadOnly, + ); + if (readOnlyMcpTools.length > 0) { + const mcpToolsList = readOnlyMcpTools + .map((t) => `- \`${t.name}\` (${t.serverName})`) + .join('\n'); + planModeToolsList += `\n${mcpToolsList}`; + } + } + const planOptions: snippets.ApprovalModePlanOptions | undefined = isPlanMode ? { - planModeToolsList: PLAN_MODE_TOOLS.filter((t) => - new Set(toolNames).has(t), - ) - .map((t) => `- \`${t}\``) - .join('\n'), + planModeToolsList, plansDir: config.storage.getProjectTempPlansDir(), } : undefined; diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index 43d8faeeea4..0fe51a7120c 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -1494,6 +1494,7 @@ describe('loggers', () => { false, undefined, undefined, + undefined, 'test-extension', 'test-extension-id', ); diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index e4bbd7d756b..86263fc587a 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -19,6 +19,7 @@ import { MCPOAuthTokenStorage } from '../mcp/oauth-token-storage.js'; import { OAuthUtils } from '../mcp/oauth-utils.js'; import type { PromptRegistry } from '../prompts/prompt-registry.js'; import { ToolListChangedNotificationSchema } from '@modelcontextprotocol/sdk/types.js'; +import { ApprovalMode, PolicyDecision } from '../policy/types.js'; import { WorkspaceContext } from '../utils/workspaceContext.js'; import { @@ -387,6 +388,157 @@ describe('mcp-client', () => { expect(mockedToolRegistry.registerTool).toHaveBeenCalledOnce(); }); + it('should register tool with readOnlyHint and add policy rule', async () => { + const mockedClient = { + connect: vi.fn(), + discover: vi.fn(), + disconnect: vi.fn(), + getStatus: vi.fn(), + registerCapabilities: vi.fn(), + setRequestHandler: vi.fn(), + setNotificationHandler: vi.fn(), + getServerCapabilities: vi.fn().mockReturnValue({ tools: {} }), + listTools: vi.fn().mockResolvedValue({ + tools: [ + { + name: 'readOnlyTool', + description: 'A read-only tool', + inputSchema: { type: 'object', properties: {} }, + annotations: { readOnlyHint: true }, + }, + ], + }), + listPrompts: vi.fn().mockResolvedValue({ prompts: [] }), + request: vi.fn().mockResolvedValue({}), + }; + vi.mocked(ClientLib.Client).mockReturnValue( + mockedClient as unknown as ClientLib.Client, + ); + vi.spyOn(SdkClientStdioLib, 'StdioClientTransport').mockReturnValue( + {} as SdkClientStdioLib.StdioClientTransport, + ); + + const mockPolicyEngine = { + addRule: vi.fn(), + }; + const mockConfig = { + getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), + } as unknown as Config; + + const mockedToolRegistry = { + registerTool: vi.fn(), + sortTools: vi.fn(), + getMessageBus: vi.fn().mockReturnValue(undefined), + removeMcpToolsByServer: vi.fn(), + } as unknown as ToolRegistry; + const promptRegistry = { + registerPrompt: vi.fn(), + removePromptsByServer: vi.fn(), + } as unknown as PromptRegistry; + const resourceRegistry = { + setResourcesForServer: vi.fn(), + removeResourcesByServer: vi.fn(), + } as unknown as ResourceRegistry; + + const client = new McpClient( + 'test-server', + { command: 'test-command' }, + mockedToolRegistry, + promptRegistry, + resourceRegistry, + workspaceContext, + { sanitizationConfig: EMPTY_CONFIG } as Config, + false, + '0.0.1', + ); + + await client.connect(); + await client.discover(mockConfig); + + // Verify tool registration + expect(mockedToolRegistry.registerTool).toHaveBeenCalledOnce(); + + // Verify policy rule addition + expect(mockPolicyEngine.addRule).toHaveBeenCalledWith({ + toolName: 'test-server__readOnlyTool', + decision: PolicyDecision.ALLOW, + priority: 50, + modes: [ApprovalMode.PLAN], + source: 'MCP Annotation (readOnlyHint) - test-server', + }); + }); + + it('should not add policy rule for tool without readOnlyHint', async () => { + const mockedClient = { + connect: vi.fn(), + discover: vi.fn(), + disconnect: vi.fn(), + getStatus: vi.fn(), + registerCapabilities: vi.fn(), + setRequestHandler: vi.fn(), + setNotificationHandler: vi.fn(), + getServerCapabilities: vi.fn().mockReturnValue({ tools: {} }), + listTools: vi.fn().mockResolvedValue({ + tools: [ + { + name: 'writeTool', + description: 'A write tool', + inputSchema: { type: 'object', properties: {} }, + // No annotations or readOnlyHint: false + }, + ], + }), + listPrompts: vi.fn().mockResolvedValue({ prompts: [] }), + request: vi.fn().mockResolvedValue({}), + }; + vi.mocked(ClientLib.Client).mockReturnValue( + mockedClient as unknown as ClientLib.Client, + ); + vi.spyOn(SdkClientStdioLib, 'StdioClientTransport').mockReturnValue( + {} as SdkClientStdioLib.StdioClientTransport, + ); + + const mockPolicyEngine = { + addRule: vi.fn(), + }; + const mockConfig = { + getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), + } as unknown as Config; + + const mockedToolRegistry = { + registerTool: vi.fn(), + sortTools: vi.fn(), + getMessageBus: vi.fn().mockReturnValue(undefined), + removeMcpToolsByServer: vi.fn(), + } as unknown as ToolRegistry; + const promptRegistry = { + registerPrompt: vi.fn(), + removePromptsByServer: vi.fn(), + } as unknown as PromptRegistry; + const resourceRegistry = { + setResourcesForServer: vi.fn(), + removeResourcesByServer: vi.fn(), + } as unknown as ResourceRegistry; + + const client = new McpClient( + 'test-server', + { command: 'test-command' }, + mockedToolRegistry, + promptRegistry, + resourceRegistry, + workspaceContext, + { sanitizationConfig: EMPTY_CONFIG } as Config, + false, + '0.0.1', + ); + + await client.connect(); + await client.discover(mockConfig); + + expect(mockedToolRegistry.registerTool).toHaveBeenCalledOnce(); + expect(mockPolicyEngine.addRule).not.toHaveBeenCalled(); + }); + it('should discover tools with $defs and $ref in schema', async () => { const mockedClient = { connect: vi.fn(), diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index e7aa866a09e..6bd1d5f93fd 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -32,6 +32,7 @@ import { PromptListChangedNotificationSchema, type Tool as McpTool, } from '@modelcontextprotocol/sdk/types.js'; +import { ApprovalMode, PolicyDecision } from '../policy/types.js'; import { parse } from 'shell-quote'; import type { Config, @@ -1006,6 +1007,11 @@ export async function discoverTools( mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC, ); + // Extract readOnlyHint from annotations + const isReadOnly = ( + toolDef.annotations as { readOnlyHint?: boolean } | undefined + )?.readOnlyHint; + const tool = new DiscoveredMCPTool( mcpCallableTool, mcpServerName, @@ -1014,12 +1020,24 @@ export async function discoverTools( toolDef.inputSchema ?? { type: 'object', properties: {} }, messageBus, mcpServerConfig.trust, + isReadOnly, undefined, cliConfig, mcpServerConfig.extension?.name, mcpServerConfig.extension?.id, ); + // If the tool is read-only, allow it in Plan mode + if (isReadOnly) { + cliConfig.getPolicyEngine().addRule({ + toolName: tool.getFullyQualifiedName(), + decision: PolicyDecision.ALLOW, + priority: 50, // Match priority of built-in plan tools + modes: [ApprovalMode.PLAN], + source: `MCP Annotation (readOnlyHint) - ${mcpServerName}`, + }); + } + discoveredTools.push(tool); } catch (error) { coreEvents.emitFeedback( diff --git a/packages/core/src/tools/mcp-tool.test.ts b/packages/core/src/tools/mcp-tool.test.ts index 5abc5779e9d..4cdad898274 100644 --- a/packages/core/src/tools/mcp-tool.test.ts +++ b/packages/core/src/tools/mcp-tool.test.ts @@ -203,6 +203,7 @@ describe('DiscoveredMCPTool', () => { undefined, undefined, undefined, + undefined, ); const params = { param: 'isErrorTrueCase' }; const functionCall = { @@ -249,6 +250,7 @@ describe('DiscoveredMCPTool', () => { undefined, undefined, undefined, + undefined, ); const params = { param: 'isErrorTopLevelCase' }; const functionCall = { @@ -298,6 +300,7 @@ describe('DiscoveredMCPTool', () => { undefined, undefined, undefined, + undefined, ); const params = { param: 'isErrorFalseCase' }; const mockToolSuccessResultObject = { @@ -756,6 +759,7 @@ describe('DiscoveredMCPTool', () => { createMockMessageBus(), true, undefined, + undefined, { isTrustedFolder: () => true } as any, undefined, undefined, @@ -901,6 +905,7 @@ describe('DiscoveredMCPTool', () => { bus, trust, undefined, + undefined, mockConfig(isTrusted) as any, undefined, undefined, diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index c096feeeeee..96d14fd5255 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/packages/core/src/tools/mcp-tool.ts @@ -247,6 +247,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< override readonly parameterSchema: unknown, messageBus: MessageBus, readonly trust?: boolean, + readonly isReadOnly?: boolean, nameOverride?: string, private readonly cliConfig?: Config, override readonly extensionName?: string, @@ -283,6 +284,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< this.parameterSchema, this.messageBus, this.trust, + this.isReadOnly, this.getFullyQualifiedName(), this.cliConfig, this.extensionName, From 86a26def43ba81ef0d7e9b037f40f9a0e139edc8 Mon Sep 17 00:00:00 2001 From: "A.K.M. Adib" Date: Tue, 3 Feb 2026 15:51:01 -0500 Subject: [PATCH 2/8] address comment by bot --- packages/core/src/tools/mcp-client.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index 6bd1d5f93fd..2f4a51a5587 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -1008,9 +1008,7 @@ export async function discoverTools( ); // Extract readOnlyHint from annotations - const isReadOnly = ( - toolDef.annotations as { readOnlyHint?: boolean } | undefined - )?.readOnlyHint; + const isReadOnly = toolDef.annotations?.readOnlyHint === true; const tool = new DiscoveredMCPTool( mcpCallableTool, From d7f5c9b6e3eb93f0d05619b3ae43871120cb4921 Mon Sep 17 00:00:00 2001 From: "A.K.M. Adib" Date: Wed, 4 Feb 2026 10:45:34 -0500 Subject: [PATCH 3/8] update policy decision to ask users for MCP tools execution in plan mode --- packages/core/src/tools/mcp-client.test.ts | 2 +- packages/core/src/tools/mcp-client.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index 86263fc587a..d407b8bec4c 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -461,7 +461,7 @@ describe('mcp-client', () => { // Verify policy rule addition expect(mockPolicyEngine.addRule).toHaveBeenCalledWith({ toolName: 'test-server__readOnlyTool', - decision: PolicyDecision.ALLOW, + decision: PolicyDecision.ASK_USER, priority: 50, modes: [ApprovalMode.PLAN], source: 'MCP Annotation (readOnlyHint) - test-server', diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index 2f4a51a5587..b4f4b686f41 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -1029,7 +1029,7 @@ export async function discoverTools( if (isReadOnly) { cliConfig.getPolicyEngine().addRule({ toolName: tool.getFullyQualifiedName(), - decision: PolicyDecision.ALLOW, + decision: PolicyDecision.ASK_USER, priority: 50, // Match priority of built-in plan tools modes: [ApprovalMode.PLAN], source: `MCP Annotation (readOnlyHint) - ${mcpServerName}`, From 41bcf6c3cb2df616575777e87f690d4e7ae15718 Mon Sep 17 00:00:00 2001 From: "A.K.M. Adib" Date: Wed, 4 Feb 2026 11:45:38 -0500 Subject: [PATCH 4/8] update planning workflow to include read only mcp servers --- packages/core/src/prompts/promptProvider.ts | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/packages/core/src/prompts/promptProvider.ts b/packages/core/src/prompts/promptProvider.ts index 74df6e0954a..d83c8d792ca 100644 --- a/packages/core/src/prompts/promptProvider.ts +++ b/packages/core/src/prompts/promptProvider.ts @@ -77,13 +77,6 @@ export class PromptProvider { } } - const planOptions: snippets.ApprovalModePlanOptions | undefined = isPlanMode - ? { - planModeToolsList, - plansDir: config.storage.getProjectTempPlansDir(), - } - : undefined; - let basePrompt: string; // --- Template File Override --- @@ -143,11 +136,7 @@ export class PromptProvider { planningWorkflow: this.withSection( 'planningWorkflow', () => ({ - planModeToolsList: PLAN_MODE_TOOLS.filter((t) => - new Set(toolNames).has(t), - ) - .map((t) => `- \`${t}\``) - .join('\n'), + planModeToolsList, plansDir: config.storage.getProjectTempPlansDir(), }), isPlanMode, From ecc24a57a11d056e252f0bd529b16390ae3d4988 Mon Sep 17 00:00:00 2001 From: "A.K.M. Adib" Date: Wed, 4 Feb 2026 13:57:10 -0500 Subject: [PATCH 5/8] update to create a set of tool names once in getCoreSystemPrompt --- packages/core/src/prompts/promptProvider.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/core/src/prompts/promptProvider.ts b/packages/core/src/prompts/promptProvider.ts index d83c8d792ca..76253554f0b 100644 --- a/packages/core/src/prompts/promptProvider.ts +++ b/packages/core/src/prompts/promptProvider.ts @@ -48,6 +48,7 @@ export class PromptProvider { const isPlanMode = approvalMode === ApprovalMode.PLAN; const skills = config.getSkillManager().getSkills(); const toolNames = config.getToolRegistry().getAllToolNames(); + const enabledToolNames = new Set(toolNames); const desiredModel = resolveModel( config.getActiveModel(), @@ -57,7 +58,7 @@ export class PromptProvider { // --- Context Gathering --- let planModeToolsList = PLAN_MODE_TOOLS.filter((t) => - new Set(toolNames).has(t), + enabledToolNames.has(t), ) .map((t) => `- \`${t}\``) .join('\n'); @@ -126,10 +127,10 @@ export class PromptProvider { 'primaryWorkflows', () => ({ interactive: interactiveMode, - enableCodebaseInvestigator: toolNames.includes( + enableCodebaseInvestigator: enabledToolNames.has( CodebaseInvestigatorAgent.name, ), - enableWriteTodosTool: toolNames.includes(WRITE_TODOS_TOOL_NAME), + enableWriteTodosTool: enabledToolNames.has(WRITE_TODOS_TOOL_NAME), }), !isPlanMode, ), From 836c58bae7e20241462dc08b8e17d4140d3a1294 Mon Sep 17 00:00:00 2001 From: "A.K.M. Adib" Date: Thu, 5 Feb 2026 14:12:14 -0500 Subject: [PATCH 6/8] add non-read only tool to test and verify that it's not included --- packages/core/src/core/prompts.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/core/src/core/prompts.test.ts b/packages/core/src/core/prompts.test.ts index c4af0986ed3..2a784875090 100644 --- a/packages/core/src/core/prompts.test.ts +++ b/packages/core/src/core/prompts.test.ts @@ -309,6 +309,7 @@ describe('Core System Prompt (prompts.ts)', () => { 'glob', 'read_file', 'ask_user', + 'run_shell_command', ]); const prompt = getCoreSystemPrompt(mockConfig); @@ -322,6 +323,9 @@ describe('Core System Prompt (prompts.ts)', () => { expect(prompt).not.toContain('`google_web_search`'); expect(prompt).not.toContain('`list_directory`'); expect(prompt).not.toContain('`grep_search`'); + + // should not include non read-only tools + expect(prompt).not.toContain('`run_shell_command`'); }); }); From b17a9ed30ce44f0a1daa0e099db1e1b49b76da48 Mon Sep 17 00:00:00 2001 From: "A.K.M. Adib" Date: Thu, 5 Feb 2026 15:16:24 -0500 Subject: [PATCH 7/8] add test to validate non-read only tools aren't available in plan mode --- packages/core/src/core/prompts.test.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/core/src/core/prompts.test.ts b/packages/core/src/core/prompts.test.ts index 2a784875090..d239c16dec8 100644 --- a/packages/core/src/core/prompts.test.ts +++ b/packages/core/src/core/prompts.test.ts @@ -25,6 +25,7 @@ import { ApprovalMode } from '../policy/types.js'; import { DiscoveredMCPTool } from '../tools/mcp-tool.js'; import type { CallableTool } from '@google/genai'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; +import { ShellTool } from '../tools/shell.js'; // Mock tool names if they are dynamically generated or complex vi.mock('../tools/ls', () => ({ LSTool: { Name: 'list_directory' } })); @@ -36,7 +37,10 @@ vi.mock('../tools/read-many-files', () => ({ ReadManyFilesTool: { Name: 'read_many_files' }, })); vi.mock('../tools/shell', () => ({ - ShellTool: { Name: 'run_shell_command' }, + ShellTool: class { + static readonly Name = 'run_shell_command'; + name = 'run_shell_command'; + }, })); vi.mock('../tools/write-file', () => ({ WriteFileTool: { Name: 'write_file' }, @@ -80,6 +84,7 @@ describe('Core System Prompt (prompts.ts)', () => { getModel: vi.fn().mockReturnValue(DEFAULT_GEMINI_MODEL_AUTO), getActiveModel: vi.fn().mockReturnValue(DEFAULT_GEMINI_MODEL), getPreviewFeatures: vi.fn().mockReturnValue(false), + getMessageBus: vi.fn(), getAgentRegistry: vi.fn().mockReturnValue({ getDirectoryContext: vi.fn().mockReturnValue('Mock Agent Directory'), }), @@ -292,6 +297,7 @@ describe('Core System Prompt (prompts.ts)', () => { vi.mocked(mockConfig.getToolRegistry().getAllTools).mockReturnValue([ mcpTool, + new ShellTool(mockConfig, mockConfig.getMessageBus()), ]); vi.mocked(mockConfig.getToolRegistry().getAllToolNames).mockReturnValue([ mcpTool.name, @@ -300,6 +306,7 @@ describe('Core System Prompt (prompts.ts)', () => { const prompt = getCoreSystemPrompt(mockConfig); expect(prompt).toContain('`read_static_value` (readonly-server)'); + expect(prompt).not.toContain('`run_shell_command`'); }); it('should only list available tools in PLAN mode', () => { @@ -309,7 +316,6 @@ describe('Core System Prompt (prompts.ts)', () => { 'glob', 'read_file', 'ask_user', - 'run_shell_command', ]); const prompt = getCoreSystemPrompt(mockConfig); @@ -323,9 +329,6 @@ describe('Core System Prompt (prompts.ts)', () => { expect(prompt).not.toContain('`google_web_search`'); expect(prompt).not.toContain('`list_directory`'); expect(prompt).not.toContain('`grep_search`'); - - // should not include non read-only tools - expect(prompt).not.toContain('`run_shell_command`'); }); }); From 84497a42f9b8bbc92b591eb0bc09a71131877277 Mon Sep 17 00:00:00 2001 From: "A.K.M. Adib" Date: Thu, 5 Feb 2026 16:04:34 -0500 Subject: [PATCH 8/8] add mcp tool that's non-read only --- packages/core/src/core/prompts.test.ts | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/core/src/core/prompts.test.ts b/packages/core/src/core/prompts.test.ts index 49e44e3af9f..931cfd66136 100644 --- a/packages/core/src/core/prompts.test.ts +++ b/packages/core/src/core/prompts.test.ts @@ -25,7 +25,6 @@ import { ApprovalMode } from '../policy/types.js'; import { DiscoveredMCPTool } from '../tools/mcp-tool.js'; import type { CallableTool } from '@google/genai'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; -import { ShellTool } from '../tools/shell.js'; // Mock tool names if they are dynamically generated or complex vi.mock('../tools/ls', () => ({ LSTool: { Name: 'list_directory' } })); @@ -311,7 +310,7 @@ describe('Core System Prompt (prompts.ts)', () => { it('should include read-only MCP tools in PLAN mode', () => { vi.mocked(mockConfig.getApprovalMode).mockReturnValue(ApprovalMode.PLAN); - const mcpTool = new DiscoveredMCPTool( + const readOnlyMcpTool = new DiscoveredMCPTool( {} as CallableTool, 'readonly-server', 'read_static_value', @@ -322,18 +321,32 @@ describe('Core System Prompt (prompts.ts)', () => { true, // isReadOnly ); + const nonReadOnlyMcpTool = new DiscoveredMCPTool( + {} as CallableTool, + 'nonreadonly-server', + 'non_read_static_value', + 'A non-read-only tool', + {}, + {} as MessageBus, + false, + false, + ); + vi.mocked(mockConfig.getToolRegistry().getAllTools).mockReturnValue([ - mcpTool, - new ShellTool(mockConfig, mockConfig.getMessageBus()), + readOnlyMcpTool, + nonReadOnlyMcpTool, ]); vi.mocked(mockConfig.getToolRegistry().getAllToolNames).mockReturnValue([ - mcpTool.name, + readOnlyMcpTool.name, + nonReadOnlyMcpTool.name, ]); const prompt = getCoreSystemPrompt(mockConfig); expect(prompt).toContain('`read_static_value` (readonly-server)'); - expect(prompt).not.toContain('`run_shell_command`'); + expect(prompt).not.toContain( + '`non_read_static_value` (nonreadonly-server)', + ); }); it('should only list available tools in PLAN mode', () => {