diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts index 1ef25e852b..fdf28c4e2b 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts @@ -23,6 +23,15 @@ vi.mock("@roo-code/core", () => ({ }, })) +// Mock the tool handlers so the tests only exercise validation (toolRequirements) +// and never the real tool execution logic. +vi.mock("../../tools/AttemptCompletionTool", () => ({ + attemptCompletionTool: { handle: vi.fn().mockResolvedValue(undefined) }, +})) +vi.mock("../../tools/AskFollowupQuestionTool", () => ({ + askFollowupQuestionTool: { handle: vi.fn().mockResolvedValue(undefined) }, +})) + // presentAssistantMessage records tool usage through TelemetryService.instance. vi.mock("@roo-code/telemetry", () => ({ TelemetryService: { @@ -333,6 +342,86 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { edit: false, }) }) + + it("never marks a protocol tool (attempt_completion) as blocked", async () => { + mockTask.assistantMessageContent = [ + { + type: "tool_use", + id: "tool_call_protocol_123", + name: "attempt_completion", + params: {}, + nativeArgs: {}, + partial: false, + }, + ] + + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "code", + customModes: [], + experiments: { + customTools: false, + }, + disabledTools: ["attempt_completion"], + }), + }), + } + + await presentAssistantMessage(mockTask) + + const validateToolUseMock = vi.mocked(validateToolUse) + expect(validateToolUseMock).toHaveBeenCalled() + const toolRequirements = validateToolUseMock.mock.calls[0][3] + // Protocol tools never enter toolRequirements, so the validator cannot + // block them even when disabledTools lists them. + expect(toolRequirements).not.toHaveProperty("attempt_completion") + + // With validateToolUse mocked to return normally, the block proceeds + // past validation: no validation-error tool_result is pushed. + const errorToolResults = mockTask.userMessageContent.filter((block: unknown) => { + const b = block as { type?: string; is_error?: boolean } + return b.type === "tool_result" && b.is_error + }) + expect(errorToolResults).toEqual([]) + }) + + it("still marks ordinary tools (ask_followup_question) as blocked", async () => { + mockTask.assistantMessageContent = [ + { + type: "tool_use", + id: "tool_call_ordinary_123", + name: "ask_followup_question", + params: { question: "Which option?" }, + nativeArgs: { question: "Which option?" }, + partial: false, + }, + ] + + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "code", + customModes: [], + experiments: { + customTools: false, + }, + disabledTools: ["ask_followup_question"], + }), + }), + } + + await presentAssistantMessage(mockTask) + + const validateToolUseMock = vi.mocked(validateToolUse) + expect(validateToolUseMock).toHaveBeenCalled() + const toolRequirements = validateToolUseMock.mock.calls[0][3] + // Control/ordinary tools remain blockable — the inverse of the + // protocol-tool guarantee. + expect(toolRequirements).toMatchObject({ + ask_followup_question: false, + }) + }) }) describe("Partial blocks", () => { diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 7b25db4e66..4155552728 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -36,6 +36,7 @@ import { skillTool } from "../tools/SkillTool" import { generateImageTool } from "../tools/GenerateImageTool" import { applyDiffTool as applyDiffToolClass } from "../tools/ApplyDiffTool" import { isValidToolName, validateToolUse } from "../tools/validateToolUse" +import { buildToolRequirements } from "../prompts/tools/effective-tool-policy" import { codebaseSearchTool } from "../tools/CodebaseSearchTool" import { formatResponse } from "../prompts/responses" @@ -604,16 +605,10 @@ export async function presentAssistantMessage(cline: Task) { const isCustomTool = Boolean(stateExperiments?.customTools && customToolRegistry.has(block.name)) try { - const toolRequirements = - disabledTools?.reduce( - (acc: Record, tool: string) => { - acc[tool] = false - const resolvedToolName = resolveToolAlias(tool) - acc[resolvedToolName] = false - return acc - }, - {} as Record, - ) ?? {} + // Use the exported resolver so `attempt_completion` (and its aliases) + // never enters `toolRequirements` — the runtime validator never blocks a + // protocol tool. See `buildToolRequirements` in effective-tool-policy.ts. + const toolRequirements = buildToolRequirements(disabledTools) validateToolUse( block.name as ToolName, diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap index d6fd17ba2f..8dacd14a25 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap @@ -24,9 +24,9 @@ By carefully considering the user's response after tool executions, you can reac CAPABILITIES -- You have access to tools that let you execute CLI commands on the user's computer, list files, view source code definitions, regex search, read and write files, and ask follow-up questions. These tools help you effectively accomplish a wide range of tasks, such as writing code, making edits or improvements to existing files, understanding the current state of a project, performing system operations, and much more. -- When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. -- You can use the execute_command tool to run commands on the user's computer whenever you feel it can help accomplish the user's task. When you need to execute a CLI command, you must provide a clear explanation of what the command does. Prefer to execute complex CLI commands over creating executable scripts, since they are more flexible and easier to run. Interactive and long-running commands are allowed, since the commands are run in the user's VSCode terminal. The user may keep commands running in the background and you will be kept updated on their status along the way. Each command you execute is run in a new terminal instance. +You have access to tools that let you list files, regex search, read files, write and edit files. (in this mode only files matching '\.md$' can be edited — Markdown files only) +- These tools help you accomplish tasks. +- If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. ==== @@ -39,25 +39,20 @@ MODES RULES - The project base directory is: /test/path -- All file paths must be relative to this directory. However, commands may change directories in terminals, so respect working directory specified by the response to execute_command. -- You cannot `cd` into a different directory to complete a task. You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. +- All file paths must be relative to this directory. +- You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. - Do not use the ~ character or $HOME to refer to the home directory. -- Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context provided to understand the user's environment and tailor your commands to ensure they are compatible with their system. You must also consider if the command you need to run should be executed in a specific directory outside of the current working directory '/test/path', and if so prepend with `cd`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run `npm install` in a project outside of '/test/path', you would need to prepend with a `cd` i.e. pseudocode for this would be `cd (path to project) && (command, in this case npm install)`. - Some modes have restrictions on which files they can edit. If you attempt to edit a restricted file, the operation will be rejected with a FileRestrictionError that will specify which file patterns are allowed for the current mode. - Be sure to consider the type of project (e.g. Python, JavaScript, web application) when determining the appropriate structure and files to include. Also consider what files may be most relevant to accomplishing the task, for example looking at a project's manifest file would help you understand the project's dependencies, which you could incorporate into any code you write. - * For example, in architect mode trying to edit app.js would be rejected because architect mode can only edit files matching "\.md$" - When making changes to code, always consider the context in which the code is being used. Ensure that your changes are compatible with the existing codebase and that they follow the project's coding standards and best practices. - Do not ask for more information than necessary. Use the tools provided to accomplish the user's request efficiently and effectively. When you've completed your task, you must use the attempt_completion tool to present the result to the user. The user may provide feedback, which you can use to make improvements and try again. - You are only allowed to ask the user questions using the ask_followup_question tool. Use this tool only when you need additional details to complete a task, and be sure to use a clear and concise question that will help you move forward with the task. When you ask a question, provide the user with 2-4 suggested answers based on your question so they don't need to do so much typing. The suggestions should be specific, actionable, and directly related to the completed task. They should be ordered by priority or logical sequence. However if you can use the available tools to avoid having to ask the user questions, you should do so. For example, if the user mentions a file that may be in an outside directory like the Desktop, you should use the list_files tool to list the files in the Desktop and check if the file they are talking about is there, rather than asking the user to provide the file path themselves. -- When executing commands, if you don't see the expected output, assume the terminal executed the command successfully and proceed with the task. The user's terminal may be unable to stream the output back properly. If you absolutely need to see the actual terminal output, use the ask_followup_question tool to request the user to copy and paste it back to you. - The user may provide a file's contents directly in their message, in which case you shouldn't use the read_file tool to get the file contents again since you already have it. - Your goal is to try to accomplish the user's task, NOT engage in a back and forth conversation. - NEVER end attempt_completion result with a question or request to engage in further conversation! Formulate the end of your result in a way that is final and does not require further input from the user. - You are STRICTLY FORBIDDEN from starting your messages with "Great", "Certainly", "Okay", "Sure". You should NOT be conversational in your responses, but rather direct and to the point. For example you should NOT say "Great, I've updated the CSS" but instead something like "I've updated the CSS". It is important you be clear and technical in your messages. - When presented with images, utilize your vision capabilities to thoroughly examine them and extract meaningful information. Incorporate these insights into your thought process as you accomplish the user's task. - At the end of each user message, you will automatically receive environment_details. This information is not written by the user themselves, but is auto-generated to provide potentially relevant context about the project structure and environment. While this information can be valuable for understanding the project context, do not treat it as a direct part of the user's request or response. Use it to inform your actions and decisions, but don't assume the user is explicitly asking about or referring to this information unless they clearly do so in their message. When using environment_details, explain your actions clearly to ensure the user understands, as they may not be aware of these details. -- Before executing commands, check the "Actively Running Terminals" section in environment_details. If present, consider how these active processes might impact your task. For example, if a local development server is already running, you wouldn't need to start it again. If no active terminals are listed, proceed with command execution as normal. -- MCP operations should be used one at a time, similar to other tool usage. Wait for confirmation of success before proceeding with additional operations. - It is critical you wait for the user's response after each tool use, in order to confirm the success of the tool use. For example, if asked to make a todo app, you would create a file, wait for the user's response it was created successfully, then create another file if needed, wait for the user's response it was created successfully, etc. ==== @@ -69,7 +64,7 @@ Default Shell: /bin/zsh Home Directory: /home/user Current Workspace Directory: /test/path -The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. New terminals will be created in the current workspace directory, however if you change directories in a terminal it will then have a different working directory; changing directories in a terminal does not modify the workspace directory, because you do not have access to change the workspace directory. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. +The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. ==== @@ -79,7 +74,7 @@ You accomplish a given task iteratively, breaking it down into clear steps and w 1. Analyze the user's task and set clear, achievable goals to accomplish it. Prioritize these goals in a logical order. 2. Work through these goals sequentially, utilizing available tools one at a time as necessary. Each goal should correspond to a distinct step in your problem-solving process. You will be informed on the work completed and what's remaining as you go. -3. Remember, you have extensive capabilities with access to a wide range of tools that can be used in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. +3. Remember, use the tools provided to you in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. 4. Once you've completed the user's task, you must use the attempt_completion tool to present the result of the task to the user. 5. The user may provide feedback, which you can use to make improvements and try again. But DO NOT continue in pointless back and forth conversations, i.e. don't end your responses with questions or offers for further assistance. diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap index 86d5b27f08..51df496c74 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap @@ -24,9 +24,9 @@ By carefully considering the user's response after tool executions, you can reac CAPABILITIES -- You have access to tools that let you execute CLI commands on the user's computer, list files, view source code definitions, regex search, read and write files, and ask follow-up questions. These tools help you effectively accomplish a wide range of tasks, such as writing code, making edits or improvements to existing files, understanding the current state of a project, performing system operations, and much more. -- When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. -- You can use the execute_command tool to run commands on the user's computer whenever you feel it can help accomplish the user's task. When you need to execute a CLI command, you must provide a clear explanation of what the command does. Prefer to execute complex CLI commands over creating executable scripts, since they are more flexible and easier to run. Interactive and long-running commands are allowed, since the commands are run in the user's VSCode terminal. The user may keep commands running in the background and you will be kept updated on their status along the way. Each command you execute is run in a new terminal instance. +You have access to tools that let you list files, regex search, read files. +- These tools help you accomplish tasks. +- If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. ==== @@ -39,25 +39,20 @@ MODES RULES - The project base directory is: /test/path -- All file paths must be relative to this directory. However, commands may change directories in terminals, so respect working directory specified by the response to execute_command. -- You cannot `cd` into a different directory to complete a task. You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. +- All file paths must be relative to this directory. +- You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. - Do not use the ~ character or $HOME to refer to the home directory. -- Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context provided to understand the user's environment and tailor your commands to ensure they are compatible with their system. You must also consider if the command you need to run should be executed in a specific directory outside of the current working directory '/test/path', and if so prepend with `cd`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run `npm install` in a project outside of '/test/path', you would need to prepend with a `cd` i.e. pseudocode for this would be `cd (path to project) && (command, in this case npm install)`. - Some modes have restrictions on which files they can edit. If you attempt to edit a restricted file, the operation will be rejected with a FileRestrictionError that will specify which file patterns are allowed for the current mode. - Be sure to consider the type of project (e.g. Python, JavaScript, web application) when determining the appropriate structure and files to include. Also consider what files may be most relevant to accomplishing the task, for example looking at a project's manifest file would help you understand the project's dependencies, which you could incorporate into any code you write. - * For example, in architect mode trying to edit app.js would be rejected because architect mode can only edit files matching "\.md$" - When making changes to code, always consider the context in which the code is being used. Ensure that your changes are compatible with the existing codebase and that they follow the project's coding standards and best practices. - Do not ask for more information than necessary. Use the tools provided to accomplish the user's request efficiently and effectively. When you've completed your task, you must use the attempt_completion tool to present the result to the user. The user may provide feedback, which you can use to make improvements and try again. - You are only allowed to ask the user questions using the ask_followup_question tool. Use this tool only when you need additional details to complete a task, and be sure to use a clear and concise question that will help you move forward with the task. When you ask a question, provide the user with 2-4 suggested answers based on your question so they don't need to do so much typing. The suggestions should be specific, actionable, and directly related to the completed task. They should be ordered by priority or logical sequence. However if you can use the available tools to avoid having to ask the user questions, you should do so. For example, if the user mentions a file that may be in an outside directory like the Desktop, you should use the list_files tool to list the files in the Desktop and check if the file they are talking about is there, rather than asking the user to provide the file path themselves. -- When executing commands, if you don't see the expected output, assume the terminal executed the command successfully and proceed with the task. The user's terminal may be unable to stream the output back properly. If you absolutely need to see the actual terminal output, use the ask_followup_question tool to request the user to copy and paste it back to you. - The user may provide a file's contents directly in their message, in which case you shouldn't use the read_file tool to get the file contents again since you already have it. - Your goal is to try to accomplish the user's task, NOT engage in a back and forth conversation. - NEVER end attempt_completion result with a question or request to engage in further conversation! Formulate the end of your result in a way that is final and does not require further input from the user. - You are STRICTLY FORBIDDEN from starting your messages with "Great", "Certainly", "Okay", "Sure". You should NOT be conversational in your responses, but rather direct and to the point. For example you should NOT say "Great, I've updated the CSS" but instead something like "I've updated the CSS". It is important you be clear and technical in your messages. - When presented with images, utilize your vision capabilities to thoroughly examine them and extract meaningful information. Incorporate these insights into your thought process as you accomplish the user's task. - At the end of each user message, you will automatically receive environment_details. This information is not written by the user themselves, but is auto-generated to provide potentially relevant context about the project structure and environment. While this information can be valuable for understanding the project context, do not treat it as a direct part of the user's request or response. Use it to inform your actions and decisions, but don't assume the user is explicitly asking about or referring to this information unless they clearly do so in their message. When using environment_details, explain your actions clearly to ensure the user understands, as they may not be aware of these details. -- Before executing commands, check the "Actively Running Terminals" section in environment_details. If present, consider how these active processes might impact your task. For example, if a local development server is already running, you wouldn't need to start it again. If no active terminals are listed, proceed with command execution as normal. -- MCP operations should be used one at a time, similar to other tool usage. Wait for confirmation of success before proceeding with additional operations. - It is critical you wait for the user's response after each tool use, in order to confirm the success of the tool use. For example, if asked to make a todo app, you would create a file, wait for the user's response it was created successfully, then create another file if needed, wait for the user's response it was created successfully, etc. ==== @@ -69,7 +64,7 @@ Default Shell: /bin/zsh Home Directory: /home/user Current Workspace Directory: /test/path -The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. New terminals will be created in the current workspace directory, however if you change directories in a terminal it will then have a different working directory; changing directories in a terminal does not modify the workspace directory, because you do not have access to change the workspace directory. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. +The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. ==== @@ -79,7 +74,7 @@ You accomplish a given task iteratively, breaking it down into clear steps and w 1. Analyze the user's task and set clear, achievable goals to accomplish it. Prioritize these goals in a logical order. 2. Work through these goals sequentially, utilizing available tools one at a time as necessary. Each goal should correspond to a distinct step in your problem-solving process. You will be informed on the work completed and what's remaining as you go. -3. Remember, you have extensive capabilities with access to a wide range of tools that can be used in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. +3. Remember, use the tools provided to you in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. 4. Once you've completed the user's task, you must use the attempt_completion tool to present the result of the task to the user. 5. The user may provide feedback, which you can use to make improvements and try again. But DO NOT continue in pointless back and forth conversations, i.e. don't end your responses with questions or offers for further assistance. diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/no-mcp-servers.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/no-mcp-servers.snap index d6fd17ba2f..8dacd14a25 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/no-mcp-servers.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/no-mcp-servers.snap @@ -24,9 +24,9 @@ By carefully considering the user's response after tool executions, you can reac CAPABILITIES -- You have access to tools that let you execute CLI commands on the user's computer, list files, view source code definitions, regex search, read and write files, and ask follow-up questions. These tools help you effectively accomplish a wide range of tasks, such as writing code, making edits or improvements to existing files, understanding the current state of a project, performing system operations, and much more. -- When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. -- You can use the execute_command tool to run commands on the user's computer whenever you feel it can help accomplish the user's task. When you need to execute a CLI command, you must provide a clear explanation of what the command does. Prefer to execute complex CLI commands over creating executable scripts, since they are more flexible and easier to run. Interactive and long-running commands are allowed, since the commands are run in the user's VSCode terminal. The user may keep commands running in the background and you will be kept updated on their status along the way. Each command you execute is run in a new terminal instance. +You have access to tools that let you list files, regex search, read files, write and edit files. (in this mode only files matching '\.md$' can be edited — Markdown files only) +- These tools help you accomplish tasks. +- If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. ==== @@ -39,25 +39,20 @@ MODES RULES - The project base directory is: /test/path -- All file paths must be relative to this directory. However, commands may change directories in terminals, so respect working directory specified by the response to execute_command. -- You cannot `cd` into a different directory to complete a task. You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. +- All file paths must be relative to this directory. +- You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. - Do not use the ~ character or $HOME to refer to the home directory. -- Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context provided to understand the user's environment and tailor your commands to ensure they are compatible with their system. You must also consider if the command you need to run should be executed in a specific directory outside of the current working directory '/test/path', and if so prepend with `cd`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run `npm install` in a project outside of '/test/path', you would need to prepend with a `cd` i.e. pseudocode for this would be `cd (path to project) && (command, in this case npm install)`. - Some modes have restrictions on which files they can edit. If you attempt to edit a restricted file, the operation will be rejected with a FileRestrictionError that will specify which file patterns are allowed for the current mode. - Be sure to consider the type of project (e.g. Python, JavaScript, web application) when determining the appropriate structure and files to include. Also consider what files may be most relevant to accomplishing the task, for example looking at a project's manifest file would help you understand the project's dependencies, which you could incorporate into any code you write. - * For example, in architect mode trying to edit app.js would be rejected because architect mode can only edit files matching "\.md$" - When making changes to code, always consider the context in which the code is being used. Ensure that your changes are compatible with the existing codebase and that they follow the project's coding standards and best practices. - Do not ask for more information than necessary. Use the tools provided to accomplish the user's request efficiently and effectively. When you've completed your task, you must use the attempt_completion tool to present the result to the user. The user may provide feedback, which you can use to make improvements and try again. - You are only allowed to ask the user questions using the ask_followup_question tool. Use this tool only when you need additional details to complete a task, and be sure to use a clear and concise question that will help you move forward with the task. When you ask a question, provide the user with 2-4 suggested answers based on your question so they don't need to do so much typing. The suggestions should be specific, actionable, and directly related to the completed task. They should be ordered by priority or logical sequence. However if you can use the available tools to avoid having to ask the user questions, you should do so. For example, if the user mentions a file that may be in an outside directory like the Desktop, you should use the list_files tool to list the files in the Desktop and check if the file they are talking about is there, rather than asking the user to provide the file path themselves. -- When executing commands, if you don't see the expected output, assume the terminal executed the command successfully and proceed with the task. The user's terminal may be unable to stream the output back properly. If you absolutely need to see the actual terminal output, use the ask_followup_question tool to request the user to copy and paste it back to you. - The user may provide a file's contents directly in their message, in which case you shouldn't use the read_file tool to get the file contents again since you already have it. - Your goal is to try to accomplish the user's task, NOT engage in a back and forth conversation. - NEVER end attempt_completion result with a question or request to engage in further conversation! Formulate the end of your result in a way that is final and does not require further input from the user. - You are STRICTLY FORBIDDEN from starting your messages with "Great", "Certainly", "Okay", "Sure". You should NOT be conversational in your responses, but rather direct and to the point. For example you should NOT say "Great, I've updated the CSS" but instead something like "I've updated the CSS". It is important you be clear and technical in your messages. - When presented with images, utilize your vision capabilities to thoroughly examine them and extract meaningful information. Incorporate these insights into your thought process as you accomplish the user's task. - At the end of each user message, you will automatically receive environment_details. This information is not written by the user themselves, but is auto-generated to provide potentially relevant context about the project structure and environment. While this information can be valuable for understanding the project context, do not treat it as a direct part of the user's request or response. Use it to inform your actions and decisions, but don't assume the user is explicitly asking about or referring to this information unless they clearly do so in their message. When using environment_details, explain your actions clearly to ensure the user understands, as they may not be aware of these details. -- Before executing commands, check the "Actively Running Terminals" section in environment_details. If present, consider how these active processes might impact your task. For example, if a local development server is already running, you wouldn't need to start it again. If no active terminals are listed, proceed with command execution as normal. -- MCP operations should be used one at a time, similar to other tool usage. Wait for confirmation of success before proceeding with additional operations. - It is critical you wait for the user's response after each tool use, in order to confirm the success of the tool use. For example, if asked to make a todo app, you would create a file, wait for the user's response it was created successfully, then create another file if needed, wait for the user's response it was created successfully, etc. ==== @@ -69,7 +64,7 @@ Default Shell: /bin/zsh Home Directory: /home/user Current Workspace Directory: /test/path -The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. New terminals will be created in the current workspace directory, however if you change directories in a terminal it will then have a different working directory; changing directories in a terminal does not modify the workspace directory, because you do not have access to change the workspace directory. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. +The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. ==== @@ -79,7 +74,7 @@ You accomplish a given task iteratively, breaking it down into clear steps and w 1. Analyze the user's task and set clear, achievable goals to accomplish it. Prioritize these goals in a logical order. 2. Work through these goals sequentially, utilizing available tools one at a time as necessary. Each goal should correspond to a distinct step in your problem-solving process. You will be informed on the work completed and what's remaining as you go. -3. Remember, you have extensive capabilities with access to a wide range of tools that can be used in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. +3. Remember, use the tools provided to you in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. 4. Once you've completed the user's task, you must use the attempt_completion tool to present the result of the task to the user. 5. The user may provide feedback, which you can use to make improvements and try again. But DO NOT continue in pointless back and forth conversations, i.e. don't end your responses with questions or offers for further assistance. diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap index 2a1533bfef..8144b1fbd0 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap @@ -24,9 +24,9 @@ By carefully considering the user's response after tool executions, you can reac CAPABILITIES -- You have access to tools that let you execute CLI commands on the user's computer, list files, view source code definitions, regex search, read and write files, and ask follow-up questions. These tools help you effectively accomplish a wide range of tasks, such as writing code, making edits or improvements to existing files, understanding the current state of a project, performing system operations, and much more. -- When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. -- You can use the execute_command tool to run commands on the user's computer whenever you feel it can help accomplish the user's task. When you need to execute a CLI command, you must provide a clear explanation of what the command does. Prefer to execute complex CLI commands over creating executable scripts, since they are more flexible and easier to run. Interactive and long-running commands are allowed, since the commands are run in the user's VSCode terminal. The user may keep commands running in the background and you will be kept updated on their status along the way. Each command you execute is run in a new terminal instance. +You have access to tools that let you list files, regex search, read files, write and edit files. (in this mode only files matching '\.md$' can be edited — Markdown files only) +- These tools help you accomplish tasks. +- If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. ==== @@ -39,25 +39,20 @@ MODES RULES - The project base directory is: /test/path -- All file paths must be relative to this directory. However, commands may change directories in terminals, so respect working directory specified by the response to execute_command. -- You cannot `cd` into a different directory to complete a task. You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. +- All file paths must be relative to this directory. +- You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. - Do not use the ~ character or $HOME to refer to the home directory. -- Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context provided to understand the user's environment and tailor your commands to ensure they are compatible with their system. You must also consider if the command you need to run should be executed in a specific directory outside of the current working directory '/test/path', and if so prepend with `cd`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run `npm install` in a project outside of '/test/path', you would need to prepend with a `cd` i.e. pseudocode for this would be `cd (path to project) && (command, in this case npm install)`. - Some modes have restrictions on which files they can edit. If you attempt to edit a restricted file, the operation will be rejected with a FileRestrictionError that will specify which file patterns are allowed for the current mode. - Be sure to consider the type of project (e.g. Python, JavaScript, web application) when determining the appropriate structure and files to include. Also consider what files may be most relevant to accomplishing the task, for example looking at a project's manifest file would help you understand the project's dependencies, which you could incorporate into any code you write. - * For example, in architect mode trying to edit app.js would be rejected because architect mode can only edit files matching "\.md$" - When making changes to code, always consider the context in which the code is being used. Ensure that your changes are compatible with the existing codebase and that they follow the project's coding standards and best practices. - Do not ask for more information than necessary. Use the tools provided to accomplish the user's request efficiently and effectively. When you've completed your task, you must use the attempt_completion tool to present the result to the user. The user may provide feedback, which you can use to make improvements and try again. - You are only allowed to ask the user questions using the ask_followup_question tool. Use this tool only when you need additional details to complete a task, and be sure to use a clear and concise question that will help you move forward with the task. When you ask a question, provide the user with 2-4 suggested answers based on your question so they don't need to do so much typing. The suggestions should be specific, actionable, and directly related to the completed task. They should be ordered by priority or logical sequence. However if you can use the available tools to avoid having to ask the user questions, you should do so. For example, if the user mentions a file that may be in an outside directory like the Desktop, you should use the list_files tool to list the files in the Desktop and check if the file they are talking about is there, rather than asking the user to provide the file path themselves. -- When executing commands, if you don't see the expected output, assume the terminal executed the command successfully and proceed with the task. The user's terminal may be unable to stream the output back properly. If you absolutely need to see the actual terminal output, use the ask_followup_question tool to request the user to copy and paste it back to you. - The user may provide a file's contents directly in their message, in which case you shouldn't use the read_file tool to get the file contents again since you already have it. - Your goal is to try to accomplish the user's task, NOT engage in a back and forth conversation. - NEVER end attempt_completion result with a question or request to engage in further conversation! Formulate the end of your result in a way that is final and does not require further input from the user. - You are STRICTLY FORBIDDEN from starting your messages with "Great", "Certainly", "Okay", "Sure". You should NOT be conversational in your responses, but rather direct and to the point. For example you should NOT say "Great, I've updated the CSS" but instead something like "I've updated the CSS". It is important you be clear and technical in your messages. - When presented with images, utilize your vision capabilities to thoroughly examine them and extract meaningful information. Incorporate these insights into your thought process as you accomplish the user's task. - At the end of each user message, you will automatically receive environment_details. This information is not written by the user themselves, but is auto-generated to provide potentially relevant context about the project structure and environment. While this information can be valuable for understanding the project context, do not treat it as a direct part of the user's request or response. Use it to inform your actions and decisions, but don't assume the user is explicitly asking about or referring to this information unless they clearly do so in their message. When using environment_details, explain your actions clearly to ensure the user understands, as they may not be aware of these details. -- Before executing commands, check the "Actively Running Terminals" section in environment_details. If present, consider how these active processes might impact your task. For example, if a local development server is already running, you wouldn't need to start it again. If no active terminals are listed, proceed with command execution as normal. -- MCP operations should be used one at a time, similar to other tool usage. Wait for confirmation of success before proceeding with additional operations. - It is critical you wait for the user's response after each tool use, in order to confirm the success of the tool use. For example, if asked to make a todo app, you would create a file, wait for the user's response it was created successfully, then create another file if needed, wait for the user's response it was created successfully, etc. ==== @@ -69,7 +64,7 @@ Default Shell: /bin/zsh Home Directory: /home/user Current Workspace Directory: /test/path -The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. New terminals will be created in the current workspace directory, however if you change directories in a terminal it will then have a different working directory; changing directories in a terminal does not modify the workspace directory, because you do not have access to change the workspace directory. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. +The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. ==== @@ -79,7 +74,7 @@ You accomplish a given task iteratively, breaking it down into clear steps and w 1. Analyze the user's task and set clear, achievable goals to accomplish it. Prioritize these goals in a logical order. 2. Work through these goals sequentially, utilizing available tools one at a time as necessary. Each goal should correspond to a distinct step in your problem-solving process. You will be informed on the work completed and what's remaining as you go. -3. Remember, you have extensive capabilities with access to a wide range of tools that can be used in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. +3. Remember, use the tools provided to you in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. 4. Once you've completed the user's task, you must use the attempt_completion tool to present the result of the task to the user. 5. The user may provide feedback, which you can use to make improvements and try again. But DO NOT continue in pointless back and forth conversations, i.e. don't end your responses with questions or offers for further assistance. diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap index 5660cd4def..f470918698 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap @@ -24,11 +24,10 @@ By carefully considering the user's response after tool executions, you can reac CAPABILITIES -- You have access to tools that let you execute CLI commands on the user's computer, list files, view source code definitions, regex search, read and write files, and ask follow-up questions. These tools help you effectively accomplish a wide range of tasks, such as writing code, making edits or improvements to existing files, understanding the current state of a project, performing system operations, and much more. -- When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. -- You can use the execute_command tool to run commands on the user's computer whenever you feel it can help accomplish the user's task. When you need to execute a CLI command, you must provide a clear explanation of what the command does. Prefer to execute complex CLI commands over creating executable scripts, since they are more flexible and easier to run. Interactive and long-running commands are allowed, since the commands are run in the user's VSCode terminal. The user may keep commands running in the background and you will be kept updated on their status along the way. Each command you execute is run in a new terminal instance. -- You have access to MCP servers that may provide additional tools and resources. Each server may provide different capabilities that you can use to accomplish tasks more effectively. - +You have access to tools that let you list files, regex search, read files, write and edit files. (in this mode only files matching '\.md$' can be edited — Markdown files only) +- These tools help you accomplish tasks. +- If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. +- You have access to MCP servers that may provide additional tools and/or resources actually available to this mode. Each server may provide different capabilities that you can use to accomplish tasks more effectively. ==== @@ -41,24 +40,20 @@ MODES RULES - The project base directory is: /test/path -- All file paths must be relative to this directory. However, commands may change directories in terminals, so respect working directory specified by the response to execute_command. -- You cannot `cd` into a different directory to complete a task. You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. +- All file paths must be relative to this directory. +- You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. - Do not use the ~ character or $HOME to refer to the home directory. -- Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context provided to understand the user's environment and tailor your commands to ensure they are compatible with their system. You must also consider if the command you need to run should be executed in a specific directory outside of the current working directory '/test/path', and if so prepend with `cd`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run `npm install` in a project outside of '/test/path', you would need to prepend with a `cd` i.e. pseudocode for this would be `cd (path to project) && (command, in this case npm install)`. - Some modes have restrictions on which files they can edit. If you attempt to edit a restricted file, the operation will be rejected with a FileRestrictionError that will specify which file patterns are allowed for the current mode. - Be sure to consider the type of project (e.g. Python, JavaScript, web application) when determining the appropriate structure and files to include. Also consider what files may be most relevant to accomplishing the task, for example looking at a project's manifest file would help you understand the project's dependencies, which you could incorporate into any code you write. - * For example, in architect mode trying to edit app.js would be rejected because architect mode can only edit files matching "\.md$" - When making changes to code, always consider the context in which the code is being used. Ensure that your changes are compatible with the existing codebase and that they follow the project's coding standards and best practices. - Do not ask for more information than necessary. Use the tools provided to accomplish the user's request efficiently and effectively. When you've completed your task, you must use the attempt_completion tool to present the result to the user. The user may provide feedback, which you can use to make improvements and try again. - You are only allowed to ask the user questions using the ask_followup_question tool. Use this tool only when you need additional details to complete a task, and be sure to use a clear and concise question that will help you move forward with the task. When you ask a question, provide the user with 2-4 suggested answers based on your question so they don't need to do so much typing. The suggestions should be specific, actionable, and directly related to the completed task. They should be ordered by priority or logical sequence. However if you can use the available tools to avoid having to ask the user questions, you should do so. For example, if the user mentions a file that may be in an outside directory like the Desktop, you should use the list_files tool to list the files in the Desktop and check if the file they are talking about is there, rather than asking the user to provide the file path themselves. -- When executing commands, if you don't see the expected output, assume the terminal executed the command successfully and proceed with the task. The user's terminal may be unable to stream the output back properly. If you absolutely need to see the actual terminal output, use the ask_followup_question tool to request the user to copy and paste it back to you. - The user may provide a file's contents directly in their message, in which case you shouldn't use the read_file tool to get the file contents again since you already have it. - Your goal is to try to accomplish the user's task, NOT engage in a back and forth conversation. - NEVER end attempt_completion result with a question or request to engage in further conversation! Formulate the end of your result in a way that is final and does not require further input from the user. - You are STRICTLY FORBIDDEN from starting your messages with "Great", "Certainly", "Okay", "Sure". You should NOT be conversational in your responses, but rather direct and to the point. For example you should NOT say "Great, I've updated the CSS" but instead something like "I've updated the CSS". It is important you be clear and technical in your messages. - When presented with images, utilize your vision capabilities to thoroughly examine them and extract meaningful information. Incorporate these insights into your thought process as you accomplish the user's task. - At the end of each user message, you will automatically receive environment_details. This information is not written by the user themselves, but is auto-generated to provide potentially relevant context about the project structure and environment. While this information can be valuable for understanding the project context, do not treat it as a direct part of the user's request or response. Use it to inform your actions and decisions, but don't assume the user is explicitly asking about or referring to this information unless they clearly do so in their message. When using environment_details, explain your actions clearly to ensure the user understands, as they may not be aware of these details. -- Before executing commands, check the "Actively Running Terminals" section in environment_details. If present, consider how these active processes might impact your task. For example, if a local development server is already running, you wouldn't need to start it again. If no active terminals are listed, proceed with command execution as normal. - MCP operations should be used one at a time, similar to other tool usage. Wait for confirmation of success before proceeding with additional operations. - It is critical you wait for the user's response after each tool use, in order to confirm the success of the tool use. For example, if asked to make a todo app, you would create a file, wait for the user's response it was created successfully, then create another file if needed, wait for the user's response it was created successfully, etc. @@ -71,7 +66,7 @@ Default Shell: /bin/zsh Home Directory: /home/user Current Workspace Directory: /test/path -The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. New terminals will be created in the current workspace directory, however if you change directories in a terminal it will then have a different working directory; changing directories in a terminal does not modify the workspace directory, because you do not have access to change the workspace directory. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. +The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. ==== @@ -81,7 +76,7 @@ You accomplish a given task iteratively, breaking it down into clear steps and w 1. Analyze the user's task and set clear, achievable goals to accomplish it. Prioritize these goals in a logical order. 2. Work through these goals sequentially, utilizing available tools one at a time as necessary. Each goal should correspond to a distinct step in your problem-solving process. You will be informed on the work completed and what's remaining as you go. -3. Remember, you have extensive capabilities with access to a wide range of tools that can be used in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. +3. Remember, use the tools provided to you in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. 4. Once you've completed the user's task, you must use the attempt_completion tool to present the result of the task to the user. 5. The user may provide feedback, which you can use to make improvements and try again. But DO NOT continue in pointless back and forth conversations, i.e. don't end your responses with questions or offers for further assistance. diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap index 2a1533bfef..8144b1fbd0 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap @@ -24,9 +24,9 @@ By carefully considering the user's response after tool executions, you can reac CAPABILITIES -- You have access to tools that let you execute CLI commands on the user's computer, list files, view source code definitions, regex search, read and write files, and ask follow-up questions. These tools help you effectively accomplish a wide range of tasks, such as writing code, making edits or improvements to existing files, understanding the current state of a project, performing system operations, and much more. -- When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. -- You can use the execute_command tool to run commands on the user's computer whenever you feel it can help accomplish the user's task. When you need to execute a CLI command, you must provide a clear explanation of what the command does. Prefer to execute complex CLI commands over creating executable scripts, since they are more flexible and easier to run. Interactive and long-running commands are allowed, since the commands are run in the user's VSCode terminal. The user may keep commands running in the background and you will be kept updated on their status along the way. Each command you execute is run in a new terminal instance. +You have access to tools that let you list files, regex search, read files, write and edit files. (in this mode only files matching '\.md$' can be edited — Markdown files only) +- These tools help you accomplish tasks. +- If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. ==== @@ -39,25 +39,20 @@ MODES RULES - The project base directory is: /test/path -- All file paths must be relative to this directory. However, commands may change directories in terminals, so respect working directory specified by the response to execute_command. -- You cannot `cd` into a different directory to complete a task. You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. +- All file paths must be relative to this directory. +- You are stuck operating from '/test/path', so be sure to pass in the correct 'path' parameter when using tools that require a path. - Do not use the ~ character or $HOME to refer to the home directory. -- Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context provided to understand the user's environment and tailor your commands to ensure they are compatible with their system. You must also consider if the command you need to run should be executed in a specific directory outside of the current working directory '/test/path', and if so prepend with `cd`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run `npm install` in a project outside of '/test/path', you would need to prepend with a `cd` i.e. pseudocode for this would be `cd (path to project) && (command, in this case npm install)`. - Some modes have restrictions on which files they can edit. If you attempt to edit a restricted file, the operation will be rejected with a FileRestrictionError that will specify which file patterns are allowed for the current mode. - Be sure to consider the type of project (e.g. Python, JavaScript, web application) when determining the appropriate structure and files to include. Also consider what files may be most relevant to accomplishing the task, for example looking at a project's manifest file would help you understand the project's dependencies, which you could incorporate into any code you write. - * For example, in architect mode trying to edit app.js would be rejected because architect mode can only edit files matching "\.md$" - When making changes to code, always consider the context in which the code is being used. Ensure that your changes are compatible with the existing codebase and that they follow the project's coding standards and best practices. - Do not ask for more information than necessary. Use the tools provided to accomplish the user's request efficiently and effectively. When you've completed your task, you must use the attempt_completion tool to present the result to the user. The user may provide feedback, which you can use to make improvements and try again. - You are only allowed to ask the user questions using the ask_followup_question tool. Use this tool only when you need additional details to complete a task, and be sure to use a clear and concise question that will help you move forward with the task. When you ask a question, provide the user with 2-4 suggested answers based on your question so they don't need to do so much typing. The suggestions should be specific, actionable, and directly related to the completed task. They should be ordered by priority or logical sequence. However if you can use the available tools to avoid having to ask the user questions, you should do so. For example, if the user mentions a file that may be in an outside directory like the Desktop, you should use the list_files tool to list the files in the Desktop and check if the file they are talking about is there, rather than asking the user to provide the file path themselves. -- When executing commands, if you don't see the expected output, assume the terminal executed the command successfully and proceed with the task. The user's terminal may be unable to stream the output back properly. If you absolutely need to see the actual terminal output, use the ask_followup_question tool to request the user to copy and paste it back to you. - The user may provide a file's contents directly in their message, in which case you shouldn't use the read_file tool to get the file contents again since you already have it. - Your goal is to try to accomplish the user's task, NOT engage in a back and forth conversation. - NEVER end attempt_completion result with a question or request to engage in further conversation! Formulate the end of your result in a way that is final and does not require further input from the user. - You are STRICTLY FORBIDDEN from starting your messages with "Great", "Certainly", "Okay", "Sure". You should NOT be conversational in your responses, but rather direct and to the point. For example you should NOT say "Great, I've updated the CSS" but instead something like "I've updated the CSS". It is important you be clear and technical in your messages. - When presented with images, utilize your vision capabilities to thoroughly examine them and extract meaningful information. Incorporate these insights into your thought process as you accomplish the user's task. - At the end of each user message, you will automatically receive environment_details. This information is not written by the user themselves, but is auto-generated to provide potentially relevant context about the project structure and environment. While this information can be valuable for understanding the project context, do not treat it as a direct part of the user's request or response. Use it to inform your actions and decisions, but don't assume the user is explicitly asking about or referring to this information unless they clearly do so in their message. When using environment_details, explain your actions clearly to ensure the user understands, as they may not be aware of these details. -- Before executing commands, check the "Actively Running Terminals" section in environment_details. If present, consider how these active processes might impact your task. For example, if a local development server is already running, you wouldn't need to start it again. If no active terminals are listed, proceed with command execution as normal. -- MCP operations should be used one at a time, similar to other tool usage. Wait for confirmation of success before proceeding with additional operations. - It is critical you wait for the user's response after each tool use, in order to confirm the success of the tool use. For example, if asked to make a todo app, you would create a file, wait for the user's response it was created successfully, then create another file if needed, wait for the user's response it was created successfully, etc. ==== @@ -69,7 +64,7 @@ Default Shell: /bin/zsh Home Directory: /home/user Current Workspace Directory: /test/path -The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. New terminals will be created in the current workspace directory, however if you change directories in a terminal it will then have a different working directory; changing directories in a terminal does not modify the workspace directory, because you do not have access to change the workspace directory. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. +The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. ==== @@ -79,7 +74,7 @@ You accomplish a given task iteratively, breaking it down into clear steps and w 1. Analyze the user's task and set clear, achievable goals to accomplish it. Prioritize these goals in a logical order. 2. Work through these goals sequentially, utilizing available tools one at a time as necessary. Each goal should correspond to a distinct step in your problem-solving process. You will be informed on the work completed and what's remaining as you go. -3. Remember, you have extensive capabilities with access to a wide range of tools that can be used in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. +3. Remember, use the tools provided to you in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. 4. Once you've completed the user's task, you must use the attempt_completion tool to present the result of the task to the user. 5. The user may provide feedback, which you can use to make improvements and try again. But DO NOT continue in pointless back and forth conversations, i.e. don't end your responses with questions or offers for further assistance. diff --git a/src/core/prompts/__tests__/sections.spec.ts b/src/core/prompts/__tests__/sections.spec.ts index 79d4fad4ca..b3f99838e9 100644 --- a/src/core/prompts/__tests__/sections.spec.ts +++ b/src/core/prompts/__tests__/sections.spec.ts @@ -1,9 +1,68 @@ import { addCustomInstructions } from "../sections/custom-instructions" import { getCapabilitiesSection } from "../sections/capabilities" import { getRulesSection, getCommandChainOperator } from "../sections/rules" +import { getSystemInfoSection } from "../sections/system-info" +import { getObjectiveSection } from "../sections/objective" +import { getToolUseGuidelinesSection } from "../sections/tool-use-guidelines" +import { getSkillsSection } from "../sections/skills" +import type { EffectiveToolPolicy } from "../tools/effective-tool-policy" +import { resolveEffectiveToolPolicy } from "../tools/effective-tool-policy" +import type { EffectiveToolPolicyInput } from "../tools/effective-tool-policy" +import type { GroupEntry, ModelInfo } from "@roo-code/types" import { McpHub } from "../../../services/mcp/McpHub" +import type { CodeIndexManager } from "../../../services/code-index/manager" +import type { SkillsManager } from "../../../services/skills/SkillsManager" import * as shellUtils from "../../../utils/shell" +// Mock os-name so getSystemInfoSection never spawns PowerShell on Windows (cold +// launches can exceed the CI test timeout). Matches the form used in +// sections/__tests__/system-info.spec.ts, but returns a constant since no test +// here asserts on the OS string itself. +vi.mock("os-name", () => ({ + default: vi.fn(() => "MockOS"), +})) + +/** + * Build an {@link EffectiveToolPolicy} for arbitrary mode groups. `mode` is the + * custom-mode slug so the resolver derives everything from `groups` (never from + * built-in names), which keeps assertions mode-neutral. + */ +function policyFor( + groups: GroupEntry[], + extra: Partial<{ + mcpHub: McpHub + disabledTools: string[] + modelInfo: ModelInfo + experiments: Record + todoListEnabled: boolean + codeIndexManager: CodeIndexManager + allowedMcpServers: string[] + }> = {}, +): EffectiveToolPolicy { + return resolveEffectiveToolPolicy({ + mode: "p", + customModes: [{ slug: "p", name: "Policy Under Test", roleDefinition: "", groups }], + ...extra, + }) +} + +/** Minimal McpHub stub. `tools`/`resources` mirror the McpServer shape the resolver reads. */ +function makeMcpHub(servers: Array<{ name: string; tools?: unknown[]; resources?: unknown[] }>): McpHub { + return { getServers: () => servers } as unknown as McpHub +} + +/** Minimal SkillsManager stub returning a fixed skill list. */ +function makeSkillsManager(n: number): SkillsManager { + return { + getSkillsForMode: () => + Array.from({ length: n }, (_, i) => ({ + name: `skill-${i}`, + description: `Skill ${i}`, + path: `./skills/${i}`, + })), + } as unknown as SkillsManager +} + describe("addCustomInstructions", () => { it("adds vscode language to custom instructions", async () => { const result = await addCustomInstructions( @@ -32,69 +91,145 @@ describe("addCustomInstructions", () => { }) describe("getCapabilitiesSection", () => { - const cwd = "/test/path" - - it("includes standard capabilities", () => { - const result = getCapabilitiesSection(cwd) + it("includes standard clauses for a full-tool mode", () => { + const result = getCapabilitiesSection(policyFor(["read", "edit", "command"])) expect(result).toContain("CAPABILITIES") - expect(result).toContain("execute CLI commands") + expect(result).toContain("execute CLI commands on the user's computer") expect(result).toContain("list files") - expect(result).toContain("read and write files") + expect(result).toContain("read files") + expect(result).toContain("write and edit files") + // the task tail is a plain sentence — assert no over-claiming enumeration + expect(result).not.toContain("such as writing code") }) - const createMockMcpHub = (serverNames: string[]): McpHub => - ({ - getServers: () => serverNames.map((name) => ({ name })), - }) as unknown as McpHub + it("uses the fallback sentence when zero per-tool clauses exist", () => { + // control-tools-only mode: only switch_mode/new_task remain (no read/edit/command clauses) + const result = getCapabilitiesSection(policyFor(["modes"])) - it("includes MCP reference when mcpHub exposes at least one server", () => { - const mockMcpHub = createMockMcpHub(["test-server"]) - const result = getCapabilitiesSection(cwd, mockMcpHub) + expect(result).toContain("You have access to a limited set of tools for this mode") + expect(result).not.toContain("You have access to tools that let you") + }) - expect(result).toContain("MCP servers") + it("emits the edit-restriction suffix when the mode declares a fileRegex", () => { + const result = getCapabilitiesSection( + policyFor(["read", ["edit", { fileRegex: "\\.md$", description: "Markdown files only" }]]), + ) + + expect(result).toContain("only files matching") + expect(result).toContain("\\.md$") + expect(result).toContain("Markdown files only") + // The suffix binds to the capability sentence, not the last emitted bullet. + expect(result).toContain( + "You have access to tools that let you list files, regex search, read files, write and edit files. (in this mode only files matching '\\.md$' can be edited — Markdown files only)", + ) }) - it("excludes MCP reference when mcpHub is undefined", () => { - const result = getCapabilitiesSection(cwd, undefined) + it("keeps the edit-restriction suffix off the MCP bullet when MCP is active", () => { + // With the mcp group + an enabled MCP server the MCP bullet is the last + // bullet; the restriction suffix must stay on the capability sentence. + const result = getCapabilitiesSection( + policyFor(["read", ["edit", { fileRegex: "\\.md$" }], "mcp"], { + mcpHub: makeMcpHub([{ name: "s", tools: [{ name: "t", description: "d" }] }]), + }), + ) - expect(result).not.toContain("MCP servers") + expect(result).toContain("MCP servers") + expect(result).not.toContain("accomplish tasks more effectively. (in this mode") + expect(result).toContain("write and edit files. (in this mode only files matching") + }) + + it("omits the edit-restriction suffix without a fileRegex", () => { + const result = getCapabilitiesSection(policyFor(["read", "edit"])) + expect(result).not.toContain("only files matching") }) - it("excludes MCP reference when mcpHub exposes no servers", () => { - const mockMcpHub = createMockMcpHub([]) - const result = getCapabilitiesSection(cwd, mockMcpHub) + it("lists files guidance only when list_files is available", () => { + const withListFiles = getCapabilitiesSection(policyFor(["read"])) + expect(withListFiles).toContain("you can use the list_files tool") + // the file-tree *fact* lives in SYSTEM INFORMATION, not CAPABILITIES + expect(withListFiles).not.toContain("a recursive list of all filepaths") - expect(result).not.toContain("MCP servers") + const withoutListFiles = getCapabilitiesSection(policyFor(["command"])) + expect(withoutListFiles).not.toContain("you can use the list_files tool") }) - it("includes MCP reference when allowedMcpServers matches a connected server", () => { - const mockMcpHub = createMockMcpHub(["allowed-server", "other-server"]) - const result = getCapabilitiesSection(cwd, mockMcpHub, ["allowed-server"]) + it("only emits the execute_command paragraph when execute_command is available", () => { + const withCmd = getCapabilitiesSection(policyFor(["command"])) + expect(withCmd).toContain("You can use the execute_command tool") - expect(result).toContain("MCP servers") + const withoutCmd = getCapabilitiesSection(policyFor(["read"])) + expect(withoutCmd).not.toContain("You can use the execute_command tool") }) - it("excludes MCP reference when allowedMcpServers is an empty array", () => { - const mockMcpHub = createMockMcpHub(["test-server"]) - const result = getCapabilitiesSection(cwd, mockMcpHub, []) + it("emits the MCP bullet only when the mode has the mcp group AND effective MCP availability", () => { + // mcp group, server with a prompt-enabled tool -> present + const hasTools = getCapabilitiesSection( + policyFor(["mcp"], { mcpHub: makeMcpHub([{ name: "s", tools: [{ name: "t", description: "d" }] }]) }), + ) + expect(hasTools).toContain("MCP servers") - expect(result).not.toContain("MCP servers") + // mcp group, server with no tools but a resource -> present via resources + const hasResources = getCapabilitiesSection( + policyFor(["mcp"], { mcpHub: makeMcpHub([{ name: "s", resources: [{ uri: "x" }] }]) }), + ) + expect(hasResources).toContain("MCP servers") + + // mcp group, empty server (no tools, no resources) -> absent + const nothing = getCapabilitiesSection(policyFor(["mcp"], { mcpHub: makeMcpHub([{ name: "s" }]) })) + expect(nothing).not.toContain("MCP servers") + + // no mcp group -> absent even with a working server + const noGroup = getCapabilitiesSection( + policyFor(["read"], { mcpHub: makeMcpHub([{ name: "s", tools: [{ name: "t", description: "d" }] }]) }), + ) + expect(noGroup).not.toContain("MCP servers") }) - it("excludes MCP reference when allowedMcpServers matches no connected server", () => { - const mockMcpHub = createMockMcpHub(["test-server"]) - const result = getCapabilitiesSection(cwd, mockMcpHub, ["nonexistent-server"]) + it("omits the MCP bullet when every tool is enabledForPrompt:false and no resources exist", () => { + const result = getCapabilitiesSection( + policyFor(["mcp"], { + mcpHub: makeMcpHub([{ name: "s", tools: [{ name: "t", description: "d", enabledForPrompt: false }] }]), + }), + ) + expect(result).not.toContain("MCP servers") + }) + it("omits the MCP bullet when a disallowed server is the only one with tools/resources", () => { + const result = getCapabilitiesSection( + policyFor(["mcp"], { + mcpHub: makeMcpHub([ + { name: "allowed", tools: [] }, + { name: "blocked", tools: [{ name: "t", description: "d" }], resources: [{ uri: "x" }] }, + ]), + allowedMcpServers: [], + }), + ) expect(result).not.toContain("MCP servers") }) + + it("includes the MCP bullet for an allowed server under an allowlist", () => { + const result = getCapabilitiesSection( + policyFor(["mcp"], { + mcpHub: makeMcpHub([{ name: "allowed", tools: [{ name: "t", description: "d" }] }]), + allowedMcpServers: ["allowed"], + }), + ) + expect(result).toContain("MCP servers") + }) }) describe("getRulesSection", () => { const cwd = "/test/path" + const settings = { + todoListEnabled: true, + useAgentRules: true, + newTaskRequireTodos: false, + } + it("includes standard rules", () => { - const result = getRulesSection(cwd) + const result = getRulesSection(cwd, settings, policyFor(["read", "edit", "command"])) expect(result).toContain("RULES") expect(result).toContain("project base directory") @@ -102,14 +237,8 @@ describe("getRulesSection", () => { }) it("includes vendor confidentiality section when isStealthModel is true", () => { - const settings = { - todoListEnabled: true, - useAgentRules: true, - newTaskRequireTodos: false, - isStealthModel: true, - } - - const result = getRulesSection(cwd, settings) + const stealthSettings = { ...settings, isStealthModel: true } + const result = getRulesSection(cwd, stealthSettings, policyFor(["read", "edit", "command"])) expect(result).toContain("VENDOR CONFIDENTIALITY") expect(result).toContain("Never reveal the vendor or company that created you") @@ -119,31 +248,209 @@ describe("getRulesSection", () => { }) it("excludes vendor confidentiality section when isStealthModel is false", () => { - const settings = { - todoListEnabled: true, - useAgentRules: true, - newTaskRequireTodos: false, - isStealthModel: false, - } - - const result = getRulesSection(cwd, settings) + const stealthSettings = { ...settings, isStealthModel: false } + const result = getRulesSection(cwd, stealthSettings, policyFor(["read", "edit", "command"])) expect(result).not.toContain("VENDOR CONFIDENTIALITY") expect(result).not.toContain("Never reveal the vendor or company") }) it("excludes vendor confidentiality section when isStealthModel is undefined", () => { - const settings = { - todoListEnabled: true, - useAgentRules: true, - newTaskRequireTodos: false, - } - - const result = getRulesSection(cwd, settings) + const result = getRulesSection(cwd, settings, policyFor(["read", "edit", "command"])) expect(result).not.toContain("VENDOR CONFIDENTIALITY") expect(result).not.toContain("Never reveal the vendor or company") }) + + it("omits the execute_command bullet when execute_command is absent", () => { + const result = getRulesSection(cwd, settings, policyFor(["read"])) + + expect(result).not.toContain("Before using the execute_command tool") + expect(result).not.toContain("Actively Running Terminals") + // the terminal-aware "working directory" clause is gone too + expect(result).not.toContain("commands may change directories in terminals") + // but the base path rule stays + expect(result).toContain("All file paths must be relative to this directory") + }) + + it("includes the execute_command bullet when execute_command is present", () => { + const result = getRulesSection(cwd, settings, policyFor(["command"])) + + expect(result).toContain("Before using the execute_command tool") + expect(result).toContain("Actively Running Terminals") + }) + + it("does not contain the removed hardcoded architect example line", () => { + const result = getRulesSection(cwd, settings, policyFor(["read", "edit", "command"])) + + expect(result).not.toContain("in architect mode") + expect(result).not.toContain("trying to edit app.js") + }) + + it("uses ask_followup_question when the tool is available", () => { + const result = getRulesSection(cwd, settings, policyFor(["read"])) + expect(result).toContain("ask the user questions using the ask_followup_question tool") + }) + + it("uses the replacement bullet when ask_followup_question is absent", () => { + // Both sub-cases — list_files present and list_files absent — take the single + // best-effort replacement bullet, emitted exactly when ask_followup_question is absent. + const withListFiles = getRulesSection( + cwd, + settings, + policyFor(["read"], { disabledTools: ["ask_followup_question"] }), + ) + expect(withListFiles).toContain("Provide your best-effort result and state your assumptions") + expect(withListFiles).not.toContain( + "You are only allowed to ask the user questions using the ask_followup_question tool", + ) + expect(withListFiles).not.toContain("enumerate the filesystem yourself") + + const withoutListFiles = getRulesSection( + cwd, + settings, + policyFor(["edit", "command"], { disabledTools: ["ask_followup_question", "list_files"] }), + ) + expect(withoutListFiles).toContain("Provide your best-effort result and state your assumptions") + expect(withoutListFiles).not.toContain( + "You are only allowed to ask the user questions using the ask_followup_question tool", + ) + expect(withoutListFiles).not.toContain("enumerate the filesystem yourself") + }) + + it("uses the fallback phrasing in the terminal-output rule when ask_followup_question is absent", () => { + // The execute_command bullet is always present, but its tail must not reference a disabled tool. + const withoutAsk = getRulesSection( + cwd, + settings, + policyFor(["command"], { disabledTools: ["ask_followup_question"] }), + ) + expect(withoutAsk).toContain("When executing commands") + expect(withoutAsk).not.toContain("ask_followup_question") + + const withAsk = getRulesSection(cwd, settings, policyFor(["command"])) + expect(withAsk).toContain( + "use the ask_followup_question tool to request the user to copy and paste it back to you", + ) + }) + + it("omits the read_file rule when read_file is absent", () => { + const result = getRulesSection(cwd, settings, policyFor(["command"])) + expect(result).not.toContain("The user may provide a file's contents directly") + }) + + it("includes the read_file rule when read_file is present", () => { + const result = getRulesSection(cwd, settings, policyFor(["read"])) + expect(result).toContain("The user may provide a file's contents directly") + }) + + it("keeps a stable RULES baseline", () => { + // duplicate guard: ensure the describe still asserts a stable baseline even if other tests change + const result = getRulesSection(cwd, settings, policyFor(["read", "edit", "command"])) + expect(result).toContain("RULES") + }) + + it("states the attempt_completion protocol rule unconditionally", () => { + // The completion sentence is protocol wording — emitted even when the policy + // does not advertise attempt_completion. A raw literal is required: the + // resolver-backed policyFor cannot express this (protocol guarantee re-adds the + // tool in resolveEffectiveToolPolicy step 11). + const rawPolicy: EffectiveToolPolicy = { + tools: new Set(["read_file"]), + hasMcpGroup: false, + hasMcpTools: false, + hasMcpResources: false, + } + + expect(rawPolicy.tools.has("attempt_completion")).toBe(false) + expect(getRulesSection(cwd, settings, rawPolicy)).toContain( + "you must use the attempt_completion tool to present the result to the user", + ) + }) +}) + +describe("getSystemInfoSection", () => { + const cwd = "/some/real/path" + + it("keeps the header lines", () => { + const result = getSystemInfoSection(cwd, policyFor(["read", "edit", "command"])) + expect(result).toContain("SYSTEM INFORMATION") + expect(result).toContain("Operating System:") + expect(result).toContain("Default Shell:") + expect(result).toContain("Home Directory:") + expect(result).toContain(`Current Workspace Directory: ${cwd}`) + }) + + it("contains no /test/path literal", () => { + const result = getSystemInfoSection(cwd, policyFor(["read", "edit", "command"])) + expect(result).not.toContain("/test/path") + }) + + it("omits the terminal-cd sentence when execute_command is absent", () => { + const result = getSystemInfoSection(cwd, policyFor(["read"])) + expect(result).not.toContain("New terminals will be created") + expect(result).not.toContain("change directories in a terminal") + }) + + it("includes the terminal-cd sentence when execute_command is present", () => { + const result = getSystemInfoSection(cwd, policyFor(["command"])) + expect(result).toContain("New terminals will be created") + }) + + it("states the file-tree fact once and omits list_files guidance here", () => { + const result = getSystemInfoSection(cwd, policyFor(["read"])) + expect(result).toContain( + "a recursive list of all filepaths in the current workspace directory will be included in environment_details", + ) + // the list_files *guidance* belongs in CAPABILITIES, not SYSTEM INFORMATION + expect(result).not.toContain("you can use the list_files tool") + }) +}) + +describe("getObjectiveSection", () => { + it("names ask_followup_question when the tool is available", () => { + const result = getObjectiveSection(policyFor(["read"])) + expect(result).toContain("ask the user to provide the missing parameters using the ask_followup_question tool") + }) + + it("uses best-effort phrasing when ask_followup_question is absent", () => { + const result = getObjectiveSection( + policyFor(["read", "edit", "command"], { disabledTools: ["ask_followup_question"] }), + ) + expect(result).toContain("state your assumptions and proceed with the best available value") + expect(result).not.toContain("ask the user to provide the missing parameters") + }) +}) + +describe("getToolUseGuidelinesSection", () => { + it("includes the list_files example when list_files is available", () => { + const result = getToolUseGuidelinesSection(policyFor(["read"])) + expect(result).toContain( + "For example using the list_files tool is more effective than running a command like `ls` in the terminal.", + ) + }) + + it("omits the list_files example when list_files is absent", () => { + const result = getToolUseGuidelinesSection(policyFor(["command"])) + expect(result).not.toContain("using the list_files tool is more effective") + }) +}) + +describe("getSkillsSection", () => { + it("returns the skills XML when the skill tool is available", async () => { + const result = await getSkillsSection(makeSkillsManager(2), "code", policyFor(["read", "edit", "command"])) + expect(result).toContain("AVAILABLE SKILLS") + expect(result).toContain("skill-0") + }) + + it("returns an empty string when the skill tool is disabled", async () => { + const result = await getSkillsSection( + makeSkillsManager(2), + "code", + policyFor(["read", "edit", "command"], { disabledTools: ["skill"] }), + ) + expect(result).toBe("") + }) }) describe("getCommandChainOperator", () => { @@ -187,6 +494,9 @@ describe("getCommandChainOperator", () => { describe("getRulesSection shell-aware command chaining", () => { const cwd = "/test/path" + const settings = { todoListEnabled: true, useAgentRules: true, newTaskRequireTodos: false } + + const codePolicy = policyFor(["read", "edit", "command"]) afterEach(() => { vi.restoreAllMocks() @@ -194,7 +504,7 @@ describe("getRulesSection shell-aware command chaining", () => { it("uses && for Unix shells in command chaining example", () => { vi.spyOn(shellUtils, "getShell").mockReturnValue("/bin/bash") - const result = getRulesSection(cwd) + const result = getRulesSection(cwd, settings, codePolicy) expect(result).toContain("cd (path to project) && (command") expect(result).not.toContain("cd (path to project) ; (command") @@ -205,7 +515,7 @@ describe("getRulesSection shell-aware command chaining", () => { vi.spyOn(shellUtils, "getShell").mockReturnValue( "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", ) - const result = getRulesSection(cwd) + const result = getRulesSection(cwd, settings, codePolicy) expect(result).toContain("cd (path to project) ; (command") expect(result).toContain("Note: Using `;` for PowerShell command chaining") @@ -213,7 +523,7 @@ describe("getRulesSection shell-aware command chaining", () => { it("uses && for cmd.exe in command chaining example", () => { vi.spyOn(shellUtils, "getShell").mockReturnValue("C:\\Windows\\System32\\cmd.exe") - const result = getRulesSection(cwd) + const result = getRulesSection(cwd, settings, codePolicy) expect(result).toContain("cd (path to project) && (command") expect(result).toContain("Note: Using `&&` for cmd.exe command chaining") @@ -223,7 +533,7 @@ describe("getRulesSection shell-aware command chaining", () => { vi.spyOn(shellUtils, "getShell").mockReturnValue( "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", ) - const result = getRulesSection(cwd) + const result = getRulesSection(cwd, settings, codePolicy) expect(result).toContain("IMPORTANT: When using PowerShell, avoid Unix-specific utilities") expect(result).toContain("`sed`, `grep`, `awk`, `cat`, `rm`, `cp`, `mv`") @@ -234,7 +544,7 @@ describe("getRulesSection shell-aware command chaining", () => { it("includes Unix utility guidance for cmd.exe", () => { vi.spyOn(shellUtils, "getShell").mockReturnValue("C:\\Windows\\System32\\cmd.exe") - const result = getRulesSection(cwd) + const result = getRulesSection(cwd, settings, codePolicy) expect(result).toContain("IMPORTANT: When using cmd.exe, avoid Unix-specific utilities") expect(result).toContain("`sed`, `grep`, `awk`, `cat`, `rm`, `cp`, `mv`") @@ -245,7 +555,7 @@ describe("getRulesSection shell-aware command chaining", () => { it("does not include Unix utility guidance for Unix shells", () => { vi.spyOn(shellUtils, "getShell").mockReturnValue("/bin/bash") - const result = getRulesSection(cwd) + const result = getRulesSection(cwd, settings, codePolicy) expect(result).not.toContain("IMPORTANT: When using PowerShell") expect(result).not.toContain("IMPORTANT: When using cmd.exe") @@ -254,7 +564,7 @@ describe("getRulesSection shell-aware command chaining", () => { it("does not include note for Unix shells", () => { vi.spyOn(shellUtils, "getShell").mockReturnValue("/bin/zsh") - const result = getRulesSection(cwd) + const result = getRulesSection(cwd, settings, codePolicy) expect(result).not.toContain("Note: Using") }) diff --git a/src/core/prompts/__tests__/system-prompt.spec.ts b/src/core/prompts/__tests__/system-prompt.spec.ts index d8671b2027..6e04ec9937 100644 --- a/src/core/prompts/__tests__/system-prompt.spec.ts +++ b/src/core/prompts/__tests__/system-prompt.spec.ts @@ -41,11 +41,12 @@ vi.mock("fs/promises") import * as vscode from "vscode" -import { ModeConfig } from "@roo-code/types" +import { ModeConfig, ModelInfo } from "@roo-code/types" import { SYSTEM_PROMPT } from "../system" import { McpHub } from "../../../services/mcp/McpHub" import { defaultModeSlug, modes, Mode } from "../../../shared/modes" +import type { SystemPromptSettings } from "../types" import "../../../utils/path" import { addCustomInstructions } from "../sections/custom-instructions" import { MultiSearchReplaceDiffStrategy } from "../../diff/strategies/multi-search-replace" @@ -641,6 +642,146 @@ describe("SYSTEM_PROMPT", () => { }) }) + describe("effective tool policy reflected in the system prompt", () => { + // Section-scoped extraction: capture the text between two "====" headers so + // user-authored roleDefinition/customInstructions can't pollute the assertions. + function extractSection(prompt: string, header: string): string { + const marker = `\n\n${header}\n\n` + const idx = prompt.indexOf(marker) + expect(idx).toBeGreaterThan(-1) + const afterHeader = prompt.slice(idx + marker.length) + const nextMarker = afterHeader.indexOf("\n\n====") + return nextMarker === -1 ? afterHeader : afterHeader.slice(0, nextMarker) + } + + const fullToolSettings: SystemPromptSettings = { + todoListEnabled: true, + useAgentRules: true, + newTaskRequireTodos: false, + } + + function run( + mode: string, + extra: Partial<{ + customModes?: ModeConfig[] + mcpHub?: McpHub + settings?: SystemPromptSettings + disabledTools?: string[] + modelInfo?: ModelInfo + }> = {}, + ) { + return SYSTEM_PROMPT( + mockContext, + "/test/path", + false, + extra.mcpHub, + undefined, // diffStrategy + mode, + undefined, // customModePrompts + extra.customModes, + undefined, // globalCustomInstructions + experiments, + undefined, // language + undefined, // rooIgnoreInstructions + extra.settings ?? fullToolSettings, // settings + undefined, // todoList + undefined, // modelId + undefined, // skillsManager + extra.disabledTools, // disabledTools + extra.modelInfo, // modelInfo + ) + } + + it("Code & Debug expose execute_command guidance (CAPABILITIES + RULES)", async () => { + for (const mode of ["code", "debug"]) { + const prompt = await run(mode) + const capabilities = extractSection(prompt, "CAPABILITIES") + const rules = extractSection(prompt, "RULES") + + expect(capabilities).toContain("execute CLI commands on the user's computer") + expect(rules).toContain("Before using the execute_command tool") + expect(rules).toContain('check the "Actively Running Terminals" section') + } + }) + + it("Architect has no execute_command and advertises the \\ .md$ edit restriction", async () => { + const prompt = await run("architect") + const capabilities = extractSection(prompt, "CAPABILITIES") + const rules = extractSection(prompt, "RULES") + const systemInfo = extractSection(prompt, "SYSTEM INFORMATION") + + // No execute_command anywhere. + expect(capabilities).not.toContain("execute CLI commands") + expect(rules).not.toContain("Before using the execute_command tool") + expect(rules).not.toContain('check the "Actively Running Terminals" section') + expect(systemInfo).not.toContain("New terminals will be created") + // Architect-style edit restriction reflected in CAPABILITIES. + expect(capabilities).toContain("in this mode only files matching") + expect(capabilities).toContain("\\.md$") + expect(capabilities).toContain("Markdown files only") + }) + + it("Ask advertises no write clause and no execute_command", async () => { + const prompt = await run("ask") + const capabilities = extractSection(prompt, "CAPABILITIES") + + expect(capabilities).not.toContain("execute CLI commands") + expect(capabilities).not.toContain("write and edit files") + expect(capabilities).toContain("read files") + }) + + it("Orchestrator advertises no read/list/edit clauses", async () => { + const prompt = await run("orchestrator") + const capabilities = extractSection(prompt, "CAPABILITIES") + + expect(capabilities).not.toContain("read files") + expect(capabilities).not.toContain("execute CLI commands") + expect(capabilities).not.toContain("write and edit files") + }) + + it("empty groups -> fallback sentence, no per-tool clauses", async () => { + const customModes: ModeConfig[] = [ + { + slug: "empty-mode", + name: "Empty Mode", + roleDefinition: "An empty mode", + groups: [], + }, + ] + const prompt = await run("empty-mode", { customModes }) + const capabilities = extractSection(prompt, "CAPABILITIES") + + // No per-tool clauses remain -> the fallback sentence is emitted. + expect(capabilities).toContain("You have access to a limited set of tools for this mode") + expect(capabilities).not.toContain("You have access to tools that let you") + // A control-only set must never advertise tool-execution clauses. + expect(capabilities).not.toContain("execute CLI commands") + expect(capabilities).not.toContain("regex search") + expect(capabilities).not.toContain("The project base directory is:") + }) + + it("disabledTools: ['execute_command'] removes command guidance from the prompt", async () => { + const prompt = await run("code", { disabledTools: ["execute_command"] }) + const capabilities = extractSection(prompt, "CAPABILITIES") + const rules = extractSection(prompt, "RULES") + + expect(rules).not.toContain("Before using the execute_command tool") + expect(rules).not.toContain("Actively Running Terminals") + expect(capabilities).not.toContain("execute CLI commands") + }) + + it("modelInfo.excludedTools removes the matching capability clause", async () => { + const prompt = await run("code", { + modelInfo: { contextWindow: 100_000, supportsPromptCache: true, excludedTools: ["read_file"] }, + }) + const capabilities = extractSection(prompt, "CAPABILITIES") + + expect(capabilities).not.toContain("read files") + // other clauses survive, proving the exclusion is scoped to the one tool + expect(capabilities).toContain("execute CLI commands") + }) + }) + afterAll(() => { vi.restoreAllMocks() }) diff --git a/src/core/prompts/sections/__tests__/objective.spec.ts b/src/core/prompts/sections/__tests__/objective.spec.ts index f776a326d2..fd0fddd2c8 100644 --- a/src/core/prompts/sections/__tests__/objective.spec.ts +++ b/src/core/prompts/sections/__tests__/objective.spec.ts @@ -1,19 +1,30 @@ import { getObjectiveSection } from "../objective" +import type { EffectiveToolPolicy } from "../../tools/effective-tool-policy" + +/** Build a policy advertising `tools` as logically available. */ +function policyFor(tools: string[]): EffectiveToolPolicy { + return { + tools: new Set(tools), + hasMcpGroup: false, + hasMcpTools: false, + hasMcpResources: false, + } +} describe("getObjectiveSection", () => { it("should include proper numbered structure", () => { - const objective = getObjectiveSection() + const objective = getObjectiveSection(policyFor([])) // Check that all numbered items are present expect(objective).toContain("1. Analyze the user's task") expect(objective).toContain("2. Work through these goals sequentially") - expect(objective).toContain("3. Remember, you have extensive capabilities") + expect(objective).toContain("3. Remember, use the tools provided to you") expect(objective).toContain("4. Once you've completed the user's task") expect(objective).toContain("5. The user may provide feedback") }) it("should include analysis guidance", () => { - const objective = getObjectiveSection() + const objective = getObjectiveSection(policyFor(["read_file"])) expect(objective).toContain("Before calling a tool, do some analysis") expect(objective).toContain("analyze the file structure provided in environment_details") @@ -21,7 +32,7 @@ describe("getObjectiveSection", () => { }) it("should include parameter inference guidance", () => { - const objective = getObjectiveSection() + const objective = getObjectiveSection(policyFor(["ask_followup_question"])) expect(objective).toContain("Go through each of the required parameters") expect(objective).toContain( @@ -32,16 +43,46 @@ describe("getObjectiveSection", () => { }) it("should include guidance about not engaging in back and forth conversations", () => { - const objective = getObjectiveSection() + const objective = getObjectiveSection(policyFor([])) expect(objective).toContain("DO NOT continue in pointless back and forth conversations") expect(objective).toContain("don't end your responses with questions or offers for further assistance") }) it("should include the OBJECTIVE header", () => { - const objective = getObjectiveSection() + const objective = getObjectiveSection(policyFor([])) expect(objective).toContain("OBJECTIVE") expect(objective).toContain("You accomplish a given task iteratively") }) + + it("drops the broad-tool claim under a zero-clause policy", () => { + // Regression guard: step 3 must not claim "extensive capabilities" or a + // "wide range of tools" when the policy advertises no tool clauses at all. + const objective = getObjectiveSection(policyFor([])) + + expect(objective).not.toContain("extensive capabilities") + expect(objective).not.toContain("wide range of tools") + }) + + it("replaces the ask step with best-effort phrasing when ask_followup_question is absent", () => { + const objective = getObjectiveSection(policyFor([])) + + // Exact substring of the false branch, which no other test asserts. + expect(objective).toContain("state your assumptions and proceed with the best available value") + expect(objective).not.toContain("ask_followup_question tool") + }) + + it("still names attempt_completion unconditionally when the tool is not advertised", () => { + // Step 4 names attempt_completion, a protocol tool, so the wording is emitted + // even when the policy's tools set does not include it. The local policyFor builds + // the policy object directly (no resolver), so policyFor([]) provably excludes + // attempt_completion. + const policy = policyFor([]) + + expect(policy.tools.has("attempt_completion")).toBe(false) + expect(getObjectiveSection(policy)).toContain( + "you must use the attempt_completion tool to present the result of the task to the user", + ) + }) }) diff --git a/src/core/prompts/sections/__tests__/skills.spec.ts b/src/core/prompts/sections/__tests__/skills.spec.ts index 707d151252..5cd3cb4ddc 100644 --- a/src/core/prompts/sections/__tests__/skills.spec.ts +++ b/src/core/prompts/sections/__tests__/skills.spec.ts @@ -1,4 +1,15 @@ import { getSkillsSection } from "../skills" +import type { EffectiveToolPolicy } from "../../tools/effective-tool-policy" + +/** Build a policy advertising `tools` as logically available. */ +function policyFor(tools: string[]): EffectiveToolPolicy { + return { + tools: new Set(tools), + hasMcpGroup: false, + hasMcpTools: false, + hasMcpResources: false, + } +} describe("getSkillsSection", () => { it("should emit XML with name, description, and location", async () => { @@ -13,7 +24,7 @@ describe("getSkillsSection", () => { ]), } - const result = await getSkillsSection(mockSkillsManager, "code") + const result = await getSkillsSection(mockSkillsManager, "code", policyFor(["skill"])) expect(result).toContain("") expect(result).toContain("") @@ -26,7 +37,31 @@ describe("getSkillsSection", () => { }) it("should return empty string when skillsManager or currentMode is missing", async () => { - await expect(getSkillsSection(undefined, "code")).resolves.toBe("") - await expect(getSkillsSection({ getSkillsForMode: vi.fn() }, undefined)).resolves.toBe("") + await expect(getSkillsSection(undefined, "code", policyFor(["skill"]))).resolves.toBe("") + await expect(getSkillsSection({ getSkillsForMode: vi.fn() }, undefined, policyFor(["skill"]))).resolves.toBe("") + }) + + it("should return empty string when the policy is missing", async () => { + const mockSkillsManager = { getSkillsForMode: vi.fn() } + + // The `policy?.` optional chain is the only guard against an undefined + // policy; removing it would make this call throw. + await expect(getSkillsSection(mockSkillsManager, "code", undefined)).resolves.toBe("") + expect(mockSkillsManager.getSkillsForMode).not.toHaveBeenCalled() + }) + + it("should return empty string when the skill tool is disabled", async () => { + const mockSkillsManager = { + getSkillsForMode: vi.fn().mockReturnValue([ + { + name: "pdf-processing", + description: "Extracts text & tables from PDFs", + path: "/abs/path/pdf-processing/SKILL.md", + source: "global" as const, + }, + ]), + } + + await expect(getSkillsSection(mockSkillsManager, "code", policyFor([]))).resolves.toBe("") }) }) diff --git a/src/core/prompts/sections/__tests__/system-info.spec.ts b/src/core/prompts/sections/__tests__/system-info.spec.ts index 749b53a0fd..7c3b53c426 100644 --- a/src/core/prompts/sections/__tests__/system-info.spec.ts +++ b/src/core/prompts/sections/__tests__/system-info.spec.ts @@ -24,6 +24,14 @@ describe("getSystemInfoSection", () => { vi.spyOn(os, "release").mockReturnValue("5.15.0") }) + /** Minimal policy with execute_command present (the default case these tests exercise). */ + const policyFor = (hasExecuteCommand: boolean = true) => ({ + tools: new Set(hasExecuteCommand ? ["execute_command"] : []), + hasMcpGroup: false, + hasMcpTools: false, + hasMcpResources: false, + }) + afterEach(() => { vi.clearAllMocks() }) @@ -31,7 +39,7 @@ describe("getSystemInfoSection", () => { it("should return system info with os-name when available", () => { mockOsName.mockReturnValue("Ubuntu 22.04") - const result = getSystemInfoSection(mockCwd) + const result = getSystemInfoSection(mockCwd, policyFor()) expect(result).toContain("Operating System: Ubuntu 22.04") expect(result).toContain("Default Shell: /bin/bash") @@ -44,7 +52,7 @@ describe("getSystemInfoSection", () => { throw new Error("Command failed with ENOENT: powershell") }) - const result = getSystemInfoSection(mockCwd) + const result = getSystemInfoSection(mockCwd, policyFor()) expect(result).toContain("Operating System: linux 5.15.0") expect(result).toContain("Default Shell: /bin/bash") @@ -59,8 +67,38 @@ describe("getSystemInfoSection", () => { vi.spyOn(os, "platform").mockReturnValue("win32" as any) vi.spyOn(os, "release").mockReturnValue("10.0.19043") - const result = getSystemInfoSection(mockCwd) + const result = getSystemInfoSection(mockCwd, policyFor()) expect(result).toContain("Operating System: win32 10.0.19043") }) + + it("omits the terminal sentence when execute_command is absent", () => { + mockOsName.mockReturnValue("Ubuntu 22.04") + + const result = getSystemInfoSection(mockCwd, policyFor(false)) + + expect(result).not.toContain("New terminals will be created") + }) + + it("includes the full terminal working-directory sentence when execute_command is present", () => { + mockOsName.mockReturnValue("Ubuntu 22.04") + + const result = getSystemInfoSection(mockCwd, policyFor(true)) + + // Exact substring of the execute_command-gated sentence; also proves the + // `execute_command` lookup itself is not mutated away. + expect(result).toContain( + "New terminals will be created in the current workspace directory, however if you change directories in a terminal it will then have a different working directory; changing directories in a terminal does not modify the workspace directory, because you do not have access to change the workspace directory.", + ) + }) + + it("joins the workspace sentence directly to the next sentence when execute_command is absent", () => { + mockOsName.mockReturnValue("Ubuntu 22.04") + + const result = getSystemInfoSection(mockCwd, policyFor(false)) + + // The false branch must stay empty: any injected filler (e.g. a mutated + // sentinel string) breaks this exact join. + expect(result).toContain("default directory for all tool operations. When the user initially gives you a task") + }) }) diff --git a/src/core/prompts/sections/__tests__/tool-use-guidelines.spec.ts b/src/core/prompts/sections/__tests__/tool-use-guidelines.spec.ts index 6d1f4b3fbf..ee07bda004 100644 --- a/src/core/prompts/sections/__tests__/tool-use-guidelines.spec.ts +++ b/src/core/prompts/sections/__tests__/tool-use-guidelines.spec.ts @@ -1,8 +1,19 @@ import { getToolUseGuidelinesSection } from "../tool-use-guidelines" +import type { EffectiveToolPolicy } from "../../tools/effective-tool-policy" + +/** Build a policy advertising `tools` as logically available. */ +function policyFor(tools: string[]): EffectiveToolPolicy { + return { + tools: new Set(tools), + hasMcpGroup: false, + hasMcpTools: false, + hasMcpResources: false, + } +} describe("getToolUseGuidelinesSection", () => { it("should include proper numbered guidelines", () => { - const guidelines = getToolUseGuidelinesSection() + const guidelines = getToolUseGuidelinesSection(policyFor(["list_files"])) expect(guidelines).toContain("1. Assess what information") expect(guidelines).toContain("2. Choose the most appropriate tool") @@ -10,14 +21,14 @@ describe("getToolUseGuidelinesSection", () => { }) it("should include multiple-tools-per-message guidance", () => { - const guidelines = getToolUseGuidelinesSection() + const guidelines = getToolUseGuidelinesSection(policyFor(["list_files"])) expect(guidelines).toContain("you may use multiple tools in a single message") expect(guidelines).not.toContain("use one tool at a time per message") }) it("should use simplified footer without step-by-step language", () => { - const guidelines = getToolUseGuidelinesSection() + const guidelines = getToolUseGuidelinesSection(policyFor(["list_files"])) expect(guidelines).toContain("carefully considering the user's response after tool executions") expect(guidelines).not.toContain("It is crucial to proceed step-by-step") @@ -25,15 +36,37 @@ describe("getToolUseGuidelinesSection", () => { }) it("should include common guidance", () => { - const guidelines = getToolUseGuidelinesSection() + const guidelines = getToolUseGuidelinesSection(policyFor(["list_files"])) expect(guidelines).toContain("Assess what information you already have") expect(guidelines).toContain("Choose the most appropriate tool") expect(guidelines).not.toContain("") }) it("should not include per-tool confirmation guidelines", () => { - const guidelines = getToolUseGuidelinesSection() + const guidelines = getToolUseGuidelinesSection(policyFor(["list_files"])) expect(guidelines).not.toContain("After each tool use, the user will respond with the result") }) + + it("omits the list_files example when list_files is absent", () => { + const guidelines = getToolUseGuidelinesSection(policyFor([])) + + expect(guidelines).not.toContain("the list_files tool is more effective than running a command like `ls`") + }) + + it("includes the list_files example verbatim when list_files is present", () => { + const guidelines = getToolUseGuidelinesSection(policyFor(["list_files"])) + + // Exact substring of the gated example, and of the exact join around it. + expect(guidelines).toContain( + "gathering this information. For example using the list_files tool is more effective than running a command like `ls` in the terminal. It's critical", + ) + }) + + it("keeps the false branch empty when the example is omitted", () => { + const guidelines = getToolUseGuidelinesSection(policyFor([])) + + // Any injected filler in the false branch breaks this exact join. + expect(guidelines).toContain("gathering this information. It's critical") + }) }) diff --git a/src/core/prompts/sections/capabilities.ts b/src/core/prompts/sections/capabilities.ts index c493692401..73e6e9ca20 100644 --- a/src/core/prompts/sections/capabilities.ts +++ b/src/core/prompts/sections/capabilities.ts @@ -1,46 +1,82 @@ -import { McpHub } from "../../../services/mcp/McpHub" +import type { EffectiveToolPolicy } from "../tools/effective-tool-policy" /** * Builds the CAPABILITIES section of the system prompt. * - * The MCP availability line is only emitted when at least one MCP server is actually - * exposed to the current mode. When `allowedMcpServers` is provided, the hub's server - * list is filtered by that allowlist BEFORE deciding whether to advertise MCP, so the - * capability text matches the per-mode tool exposure: - * - `undefined` allowlist → all connected servers count (backward compatible) - * - empty `[]` allowlist → no servers count ⇒ MCP line omitted - * - populated allowlist → only listed servers count + * Every capability claim is now a fragment emitted only when its tool is in the + * request's effective tool policy (the single source of truth shared by prompt + * generation, API tool construction, runtime validation, and preview). This + * keeps the prose consistent with what the model can actually call for the mode. * - * @param cwd Current working directory used in the prompt text. - * @param mcpHub Optional MCP hub. When omitted, the MCP line is never emitted. - * @param allowedMcpServers Optional per-mode allowlist of MCP server names. When provided, - * the hub's servers are filtered to this set before determining MCP availability. + * The file-tree paragraph is stated once as a fact in SYSTEM INFORMATION; the + * `list_files` *guidance* lives here and is gated on the tool being present. + * + * @param policy The request's effective tool policy. */ -export function getCapabilitiesSection(cwd: string, mcpHub?: McpHub, allowedMcpServers?: string[]): string { - // Determine whether any MCP server is actually available to the current mode. - // Filtering the hub's servers by the allowlist (when provided) keeps the capability - // text consistent with the tools that are exposed for the mode. - let hasMcpServers = false - if (mcpHub) { - let servers = mcpHub.getServers() - if (allowedMcpServers) { - const allowSet = new Set(allowedMcpServers) - servers = servers.filter((server) => allowSet.has(server.name)) - } - hasMcpServers = servers.length > 0 +export function getCapabilitiesSection(policy: EffectiveToolPolicy): string { + const tools = policy.tools + + const clauses: string[] = [] + if (tools.has("execute_command")) { + clauses.push("execute CLI commands on the user's computer") + } + if (tools.has("list_files")) { + clauses.push("list files") + } + if (tools.has("codebase_search")) { + clauses.push("view source code definitions") + } + if (tools.has("search_files")) { + clauses.push("regex search") + } + if (tools.has("read_file")) { + clauses.push("read files") + } + if (tools.has("write_to_file") || tools.has("apply_diff")) { + clauses.push("write and edit files") + } + + // The catalog clause is the only always-present sentence; when there are no + // per-tool clauses (e.g. a control-tool-only mode) we fall back to a sentence + // that warns the model it may only call provided tools. + const capabilitySentence = + clauses.length > 0 + ? `You have access to tools that let you ${clauses.join(", ")}.` + : "You have access to a limited set of tools for this mode; only the tools you are provided may be called." + + // The edit-restriction suffix binds to the capability sentence (not the last + // emitted bullet) so its position is deterministic regardless of which + // optional bullets follow. + const editRestrictionSuffix = policy.editRestriction + ? ` (in this mode only files matching '${policy.editRestriction.fileRegex}' can be edited${ + policy.editRestriction.description ? ` — ${policy.editRestriction.description}` : "" + })` + : "" + + let body = `${capabilitySentence}${editRestrictionSuffix}\n` + + body += `- These tools help you accomplish tasks.\n` + + // `list_files` guidance only — the file-tree *fact* is stated once in + // SYSTEM INFORMATION (and carries the cwd there). + if (tools.has("list_files")) { + body += `- If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop.\n` } + if (tools.has("execute_command")) { + body += `- You can use the execute_command tool to run commands on the user's computer whenever you feel it can help accomplish the user's task. When you need to execute a CLI command, you must provide a clear explanation of what the command does. Prefer to execute complex CLI commands over creating executable scripts, since they are more flexible and easier to run. Interactive and long-running commands are allowed, since the commands are run in the user's VSCode terminal. The user may keep commands running in the background and you will be kept updated on their status along the way. Each command you execute is run in a new terminal instance.\n` + } + + // MCP bullet — only when MCP is effectively available (group + enabled tools/resources). + if (policy.hasMcpGroup && (policy.hasMcpTools || policy.hasMcpResources)) { + body += `- You have access to MCP servers that may provide additional tools and/or resources actually available to this mode. Each server may provide different capabilities that you can use to accomplish tasks more effectively.\n` + } + + body = body.replace(/\n$/, "") + return `==== CAPABILITIES -- You have access to tools that let you execute CLI commands on the user's computer, list files, view source code definitions, regex search, read and write files, and ask follow-up questions. These tools help you effectively accomplish a wide range of tasks, such as writing code, making edits or improvements to existing files, understanding the current state of a project, performing system operations, and much more. -- When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('${cwd}') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop. -- You can use the execute_command tool to run commands on the user's computer whenever you feel it can help accomplish the user's task. When you need to execute a CLI command, you must provide a clear explanation of what the command does. Prefer to execute complex CLI commands over creating executable scripts, since they are more flexible and easier to run. Interactive and long-running commands are allowed, since the commands are run in the user's VSCode terminal. The user may keep commands running in the background and you will be kept updated on their status along the way. Each command you execute is run in a new terminal instance.${ - hasMcpServers - ? ` -- You have access to MCP servers that may provide additional tools and resources. Each server may provide different capabilities that you can use to accomplish tasks more effectively. -` - : "" - }` +${body}` } diff --git a/src/core/prompts/sections/objective.ts b/src/core/prompts/sections/objective.ts index 2ef32bc144..fbb2459668 100644 --- a/src/core/prompts/sections/objective.ts +++ b/src/core/prompts/sections/objective.ts @@ -1,4 +1,20 @@ -export function getObjectiveSection(): string { +import type { EffectiveToolPolicy } from "../tools/effective-tool-policy" + +/** + * Builds the OBJECTIVE section of the system prompt. + * + * Step 3's guidance to ask the user via ask_followup_question is replaced with + * best-effort phrasing when that tool is not in the request's effective policy. + * Step 4 names attempt_completion, a protocol tool that is always present in the + * effective tool policy, so it is emitted unconditionally. + * + * @param policy The request's effective tool policy. + */ +export function getObjectiveSection(policy: EffectiveToolPolicy): string { + const askStep = policy.tools.has("ask_followup_question") + ? "ask the user to provide the missing parameters using the ask_followup_question tool" + : "state your assumptions and proceed with the best available value" + return `==== OBJECTIVE @@ -7,7 +23,7 @@ You accomplish a given task iteratively, breaking it down into clear steps and w 1. Analyze the user's task and set clear, achievable goals to accomplish it. Prioritize these goals in a logical order. 2. Work through these goals sequentially, utilizing available tools one at a time as necessary. Each goal should correspond to a distinct step in your problem-solving process. You will be informed on the work completed and what's remaining as you go. -3. Remember, you have extensive capabilities with access to a wide range of tools that can be used in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ask the user to provide the missing parameters using the ask_followup_question tool. DO NOT ask for more information on optional parameters if it is not provided. +3. Remember, use the tools provided to you in powerful and clever ways as necessary to accomplish each goal. Before calling a tool, do some analysis. First, analyze the file structure provided in environment_details to gain context and insights for proceeding effectively. Next, think about which of the provided tools is the most relevant tool to accomplish the user's task. Go through each of the required parameters of the relevant tool and determine if the user has directly provided or given enough information to infer a value. When deciding if the parameter can be inferred, carefully consider all the context to see if it supports a specific value. If all of the required parameters are present or can be reasonably inferred, proceed with the tool use. BUT, if one of the values for a required parameter is missing, DO NOT invoke the tool (not even with fillers for the missing params) and instead, ${askStep}. DO NOT ask for more information on optional parameters if it is not provided. 4. Once you've completed the user's task, you must use the attempt_completion tool to present the result of the task to the user. 5. The user may provide feedback, which you can use to make improvements and try again. But DO NOT continue in pointless back and forth conversations, i.e. don't end your responses with questions or offers for further assistance.` } diff --git a/src/core/prompts/sections/rules.ts b/src/core/prompts/sections/rules.ts index 4f6e573fa7..b71fa97823 100644 --- a/src/core/prompts/sections/rules.ts +++ b/src/core/prompts/sections/rules.ts @@ -2,6 +2,8 @@ import type { SystemPromptSettings } from "../types" import { getShell } from "../../../utils/shell" +import type { EffectiveToolPolicy } from "../tools/effective-tool-policy" + /** * Returns the appropriate command chaining operator based on the user's shell. * - Unix shells (bash, zsh, etc.): `&&` (run next command only if previous succeeds) @@ -62,34 +64,125 @@ When asked about your creator, vendor, or company, respond with: - "I don't have information about specific vendors"` } -export function getRulesSection(cwd: string, settings?: SystemPromptSettings): string { - // Get shell-appropriate command chaining operator +/** + * Builds the RULES section of the system prompt. + * + * Fragments that describe tool-specific behavior are emitted only when that tool + * is in the request's effective tool policy. + * + * @param cwd Current working directory used in the prompt text. + * @param settings System prompt settings (used for the stealth-model confidentiality section). + * @param policy The request's effective tool policy. + */ +export function getRulesSection( + cwd: string, + settings: SystemPromptSettings | undefined, + policy: EffectiveToolPolicy, +): string { const chainOp = getCommandChainOperator() const chainNote = getCommandChainNote() + const hasExecuteCommand = policy.tools.has("execute_command") + const hasAskFollowupQuestion = policy.tools.has("ask_followup_question") + const hasListFiles = policy.tools.has("list_files") + const hasReadFile = policy.tools.has("read_file") + + const rules: string[] = [] + + rules.push(`The project base directory is: ${cwd.toPosix()}`) + + rules.push( + hasExecuteCommand + ? `All file paths must be relative to this directory. However, commands may change directories in terminals, so respect working directory specified by the response to execute_command.` + : "All file paths must be relative to this directory.", + ) + + rules.push( + `You are stuck operating from '${cwd.toPosix()}', so be sure to pass in the correct 'path' parameter when using tools that require a path.`, + ) + + rules.push("Do not use the ~ character or $HOME to refer to the home directory.") + + if (hasExecuteCommand) { + rules.push( + `Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context provided to understand the user's environment and tailor your commands to ensure they are compatible with their system. You must also consider if the command you need to run should be executed in a specific directory outside of the current working directory '${cwd.toPosix()}', and if so prepend with \`cd\`'ing into that directory ${chainOp} then executing the command (as one command since you are stuck operating from '${cwd.toPosix()}'). For example, if you needed to run \`npm install\` in a project outside of '${cwd.toPosix()}', you would need to prepend with a \`cd\` i.e. pseudocode for this would be \`cd (path to project) ${chainOp} (command, in this case npm install)\`.${chainNote ? ` ${chainNote}` : ""}`, + ) + } + + rules.push( + "Some modes have restrictions on which files they can edit. If you attempt to edit a restricted file, the operation will be rejected with a FileRestrictionError that will specify which file patterns are allowed for the current mode.", + ) + + rules.push( + "Be sure to consider the type of project (e.g. Python, JavaScript, web application) when determining the appropriate structure and files to include. Also consider what files may be most relevant to accomplishing the task, for example looking at a project's manifest file would help you understand the project's dependencies, which you could incorporate into any code you write.", + ) + + rules.push( + "When making changes to code, always consider the context in which the code is being used. Ensure that your changes are compatible with the existing codebase and that they follow the project's coding standards and best practices.", + ) + + rules.push( + "Do not ask for more information than necessary. Use the tools provided to accomplish the user's request efficiently and effectively. When you've completed your task, you must use the attempt_completion tool to present the result to the user. The user may provide feedback, which you can use to make improvements and try again.", + ) + + if (hasAskFollowupQuestion) { + rules.push( + `You are only allowed to ask the user questions using the ask_followup_question tool. Use this tool only when you need additional details to complete a task, and be sure to use a clear and concise question that will help you move forward with the task. When you ask a question, provide the user with 2-4 suggested answers based on your question so they don't need to do so much typing. The suggestions should be specific, actionable, and directly related to the completed task. They should be ordered by priority or logical sequence. However if you can use the available tools to avoid having to ask the user questions, you should do so.${ + hasListFiles + ? ` For example, if the user mentions a file that may be in an outside directory like the Desktop, you should use the list_files tool to list the files in the Desktop and check if the file they are talking about is there, rather than asking the user to provide the file path themselves.` + : "" + }`, + ) + } else { + // ask_followup_question unavailable: fall back to best-effort guidance. + rules.push( + "Provide your best-effort result and state your assumptions; the user may respond with feedback after completion.", + ) + } + + if (hasExecuteCommand) { + rules.push( + `When executing commands, if you don't see the expected output, assume the terminal executed the command successfully and proceed with the task. The user's terminal may be unable to stream the output back properly. If you absolutely need to see the actual terminal output, ${ + hasAskFollowupQuestion + ? "use the ask_followup_question tool to request the user to copy and paste it back to you" + : "note what you expected and proceed with the task, stating your assumptions" + }.`, + ) + } + + if (hasReadFile) { + rules.push( + "The user may provide a file's contents directly in their message, in which case you shouldn't use the read_file tool to get the file contents again since you already have it.", + ) + } + + rules.push( + "Your goal is to try to accomplish the user's task, NOT engage in a back and forth conversation.", + "NEVER end attempt_completion result with a question or request to engage in further conversation! Formulate the end of your result in a way that is final and does not require further input from the user.", + 'You are STRICTLY FORBIDDEN from starting your messages with "Great", "Certainly", "Okay", "Sure". You should NOT be conversational in your responses, but rather direct and to the point. For example you should NOT say "Great, I\'ve updated the CSS" but instead something like "I\'ve updated the CSS". It is important you be clear and technical in your messages.', + "When presented with images, utilize your vision capabilities to thoroughly examine them and extract meaningful information. Incorporate these insights into your thought process as you accomplish the user's task.", + "At the end of each user message, you will automatically receive environment_details. This information is not written by the user themselves, but is auto-generated to provide potentially relevant context about the project structure and environment. While this information can be valuable for understanding the project context, do not treat it as a direct part of the user's request or response. Use it to inform your actions and decisions, but don't assume the user is explicitly asking about or referring to this information unless they clearly do so in their message. When using environment_details, explain your actions clearly to ensure the user understands, as they may not be aware of these details.", + ) + + if (hasExecuteCommand) { + rules.push( + 'Before executing commands, check the "Actively Running Terminals" section in environment_details. If present, consider how these active processes might impact your task. For example, if a local development server is already running, you wouldn\'t need to start it again. If no active terminals are listed, proceed with command execution as normal.', + ) + } + + if (policy.hasMcpGroup && (policy.hasMcpTools || policy.hasMcpResources)) { + rules.push( + "MCP operations should be used one at a time, similar to other tool usage. Wait for confirmation of success before proceeding with additional operations.", + ) + } + + rules.push( + "It is critical you wait for the user's response after each tool use, in order to confirm the success of the tool use. For example, if asked to make a todo app, you would create a file, wait for the user's response it was created successfully, then create another file if needed, wait for the user's response it was created successfully, etc.", + ) + return `==== RULES -- The project base directory is: ${cwd.toPosix()} -- All file paths must be relative to this directory. However, commands may change directories in terminals, so respect working directory specified by the response to execute_command. -- You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '${cwd.toPosix()}', so be sure to pass in the correct 'path' parameter when using tools that require a path. -- Do not use the ~ character or $HOME to refer to the home directory. -- Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context provided to understand the user's environment and tailor your commands to ensure they are compatible with their system. You must also consider if the command you need to run should be executed in a specific directory outside of the current working directory '${cwd.toPosix()}', and if so prepend with \`cd\`'ing into that directory ${chainOp} then executing the command (as one command since you are stuck operating from '${cwd.toPosix()}'). For example, if you needed to run \`npm install\` in a project outside of '${cwd.toPosix()}', you would need to prepend with a \`cd\` i.e. pseudocode for this would be \`cd (path to project) ${chainOp} (command, in this case npm install)\`.${chainNote ? ` ${chainNote}` : ""} -- Some modes have restrictions on which files they can edit. If you attempt to edit a restricted file, the operation will be rejected with a FileRestrictionError that will specify which file patterns are allowed for the current mode. -- Be sure to consider the type of project (e.g. Python, JavaScript, web application) when determining the appropriate structure and files to include. Also consider what files may be most relevant to accomplishing the task, for example looking at a project's manifest file would help you understand the project's dependencies, which you could incorporate into any code you write. - * For example, in architect mode trying to edit app.js would be rejected because architect mode can only edit files matching "\\.md$" -- When making changes to code, always consider the context in which the code is being used. Ensure that your changes are compatible with the existing codebase and that they follow the project's coding standards and best practices. -- Do not ask for more information than necessary. Use the tools provided to accomplish the user's request efficiently and effectively. When you've completed your task, you must use the attempt_completion tool to present the result to the user. The user may provide feedback, which you can use to make improvements and try again. -- You are only allowed to ask the user questions using the ask_followup_question tool. Use this tool only when you need additional details to complete a task, and be sure to use a clear and concise question that will help you move forward with the task. When you ask a question, provide the user with 2-4 suggested answers based on your question so they don't need to do so much typing. The suggestions should be specific, actionable, and directly related to the completed task. They should be ordered by priority or logical sequence. However if you can use the available tools to avoid having to ask the user questions, you should do so. For example, if the user mentions a file that may be in an outside directory like the Desktop, you should use the list_files tool to list the files in the Desktop and check if the file they are talking about is there, rather than asking the user to provide the file path themselves. -- When executing commands, if you don't see the expected output, assume the terminal executed the command successfully and proceed with the task. The user's terminal may be unable to stream the output back properly. If you absolutely need to see the actual terminal output, use the ask_followup_question tool to request the user to copy and paste it back to you. -- The user may provide a file's contents directly in their message, in which case you shouldn't use the read_file tool to get the file contents again since you already have it. -- Your goal is to try to accomplish the user's task, NOT engage in a back and forth conversation. -- NEVER end attempt_completion result with a question or request to engage in further conversation! Formulate the end of your result in a way that is final and does not require further input from the user. -- You are STRICTLY FORBIDDEN from starting your messages with "Great", "Certainly", "Okay", "Sure". You should NOT be conversational in your responses, but rather direct and to the point. For example you should NOT say "Great, I've updated the CSS" but instead something like "I've updated the CSS". It is important you be clear and technical in your messages. -- When presented with images, utilize your vision capabilities to thoroughly examine them and extract meaningful information. Incorporate these insights into your thought process as you accomplish the user's task. -- At the end of each user message, you will automatically receive environment_details. This information is not written by the user themselves, but is auto-generated to provide potentially relevant context about the project structure and environment. While this information can be valuable for understanding the project context, do not treat it as a direct part of the user's request or response. Use it to inform your actions and decisions, but don't assume the user is explicitly asking about or referring to this information unless they clearly do so in their message. When using environment_details, explain your actions clearly to ensure the user understands, as they may not be aware of these details. -- Before executing commands, check the "Actively Running Terminals" section in environment_details. If present, consider how these active processes might impact your task. For example, if a local development server is already running, you wouldn't need to start it again. If no active terminals are listed, proceed with command execution as normal. -- MCP operations should be used one at a time, similar to other tool usage. Wait for confirmation of success before proceeding with additional operations. -- It is critical you wait for the user's response after each tool use, in order to confirm the success of the tool use. For example, if asked to make a todo app, you would create a file, wait for the user's response it was created successfully, then create another file if needed, wait for the user's response it was created successfully, etc.${settings?.isStealthModel ? getVendorConfidentialitySection() : ""}` +- ${rules.join("\n- ")}${settings?.isStealthModel ? getVendorConfidentialitySection() : ""}` } diff --git a/src/core/prompts/sections/skills.ts b/src/core/prompts/sections/skills.ts index 6cd3a71d75..86d9e59844 100644 --- a/src/core/prompts/sections/skills.ts +++ b/src/core/prompts/sections/skills.ts @@ -1,4 +1,5 @@ import type { SkillsManager } from "../../../services/skills/SkillsManager" +import type { EffectiveToolPolicy } from "../tools/effective-tool-policy" type SkillsManagerLike = Pick @@ -22,7 +23,12 @@ function escapeXml(value: string): string { export async function getSkillsSection( skillsManager: SkillsManagerLike | undefined, currentMode: string | undefined, + policy?: EffectiveToolPolicy, ): Promise { + // The protocol in this section mandates the `skill` tool; if it's not available + // the section would be unhelpful/unactionable, so emit nothing. + if (!policy?.tools.has("skill")) return "" + if (!skillsManager || !currentMode) return "" // Get skills filtered by current mode (with override resolution) diff --git a/src/core/prompts/sections/system-info.ts b/src/core/prompts/sections/system-info.ts index a4af3c6ac9..98112cd4ed 100644 --- a/src/core/prompts/sections/system-info.ts +++ b/src/core/prompts/sections/system-info.ts @@ -3,7 +3,19 @@ import osName from "os-name" import { getShell } from "../../../utils/shell" -export function getSystemInfoSection(cwd: string): string { +import type { EffectiveToolPolicy } from "../tools/effective-tool-policy" + +/** + * Builds the SYSTEM INFORMATION section of the system prompt. + * + * The workspace-directory / file-tree facts are stated once here; the + * file-tree fact is cwd-independent. The terminal-cd sentence is gated on + * `execute_command`, since those semantics do not exist without it. + * + * @param cwd Current working directory used in the prompt text. + * @param policy The request's effective tool policy. + */ +export function getSystemInfoSection(cwd: string, policy: EffectiveToolPolicy): string { // Try to get detailed OS name, fall back to basic info if it fails let osInfo: string try { @@ -15,6 +27,12 @@ export function getSystemInfoSection(cwd: string): string { osInfo = `${platform} ${release}` } + const executeCommandAvailable = policy.tools.has("execute_command") + + const executeCommandSentence = executeCommandAvailable + ? " New terminals will be created in the current workspace directory, however if you change directories in a terminal it will then have a different working directory; changing directories in a terminal does not modify the workspace directory, because you do not have access to change the workspace directory." + : "" + const details = `==== SYSTEM INFORMATION @@ -24,7 +42,7 @@ Default Shell: ${getShell()} Home Directory: ${os.homedir().toPosix()} Current Workspace Directory: ${cwd.toPosix()} -The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations. New terminals will be created in the current workspace directory, however if you change directories in a terminal it will then have a different working directory; changing directories in a terminal does not modify the workspace directory, because you do not have access to change the workspace directory. When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory ('/test/path') will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop.` +The Current Workspace Directory is the active VS Code project directory, and is therefore the default directory for all tool operations.${executeCommandSentence} When the user initially gives you a task, a recursive list of all filepaths in the current workspace directory will be included in environment_details. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further.` return details } diff --git a/src/core/prompts/sections/tool-use-guidelines.ts b/src/core/prompts/sections/tool-use-guidelines.ts index 78193372cc..2a34c89966 100644 --- a/src/core/prompts/sections/tool-use-guidelines.ts +++ b/src/core/prompts/sections/tool-use-guidelines.ts @@ -1,8 +1,22 @@ -export function getToolUseGuidelinesSection(): string { +import type { EffectiveToolPolicy } from "../tools/effective-tool-policy" + +/** + * Builds the TOOL USE GUIDELINES section of the system prompt. + * + * Guideline 2's example names `list_files` over `ls`; that example is only kept + * when `list_files` is in the request's effective tool policy. + * + * @param policy The request's effective tool policy. + */ +export function getToolUseGuidelinesSection(policy: EffectiveToolPolicy): string { + const listExample = policy.tools.has("list_files") + ? " For example using the list_files tool is more effective than running a command like `ls` in the terminal." + : "" + return `# Tool Use Guidelines 1. Assess what information you already have and what information you need to proceed with the task. -2. Choose the most appropriate tool based on the task and the tool descriptions provided. Assess if you need additional information to proceed, and which of the available tools would be most effective for gathering this information. For example using the list_files tool is more effective than running a command like \`ls\` in the terminal. It's critical that you think about each available tool and use the one that best fits the current step in the task. +2. Choose the most appropriate tool based on the task and the tool descriptions provided. Assess if you need additional information to proceed, and which of the available tools would be most effective for gathering this information.${listExample} It's critical that you think about each available tool and use the one that best fits the current step in the task. 3. If multiple actions are needed, you may use multiple tools in a single message when appropriate, or use tools iteratively across messages. Each tool use should be informed by the results of previous tool uses. Do not assume the outcome of any tool use. Each step must be informed by the previous step's result. By carefully considering the user's response after tool executions, you can react accordingly and make informed decisions about how to proceed with the task. This iterative process helps ensure the overall success and accuracy of your work.` diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index 93f4a52846..847fc1ca87 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -1,8 +1,14 @@ import * as vscode from "vscode" -import { type ModeConfig, type PromptComponent, type CustomModePrompts, type TodoItem } from "@roo-code/types" - -import { Mode, modes, defaultModeSlug, getModeBySlug, getGroupName, getModeSelection } from "../../shared/modes" +import { + type ModeConfig, + type PromptComponent, + type CustomModePrompts, + type TodoItem, + type ModelInfo, +} from "@roo-code/types" + +import { Mode, modes, defaultModeSlug, getModeBySlug, getModeSelection } from "../../shared/modes" import { DiffStrategy } from "../../shared/tools" import { formatLanguage } from "../../shared/language" import { isEmpty } from "../../utils/object" @@ -12,6 +18,8 @@ import { CodeIndexManager } from "../../services/code-index/manager" import { SkillsManager } from "../../services/skills/SkillsManager" import type { SystemPromptSettings } from "./types" +import type { EffectiveToolPolicy } from "./tools/effective-tool-policy" +import { resolveEffectiveToolPolicy } from "./tools/effective-tool-policy" import { getRulesSection, getSystemInfoSection, @@ -55,6 +63,8 @@ async function generatePrompt( todoList?: TodoItem[], modelId?: string, skillsManager?: SkillsManager, + disabledTools?: string[], + modelInfo?: ModelInfo, ): Promise { if (!context) { throw new Error("Extension context is required for generating system prompt") @@ -64,29 +74,29 @@ async function generatePrompt( const modeConfig = getModeBySlug(mode, customModeConfigs) || modes.find((m) => m.slug === mode) || modes[0] const { roleDefinition, baseInstructions } = getModeSelection(mode, promptComponent, customModeConfigs) - // Check if MCP functionality should be included - const hasMcpGroup = modeConfig.groups.some((groupEntry) => getGroupName(groupEntry) === "mcp") - const allowedMcpServers = modeConfig.allowedMcpServers - - // Hoist the allowlist Set once (matches the sibling call sites, e.g. mcp_server.ts) instead - // of constructing a new Set on every `.filter` iteration. - const allowSet = allowedMcpServers ? new Set(allowedMcpServers) : undefined - - let hasMcpServers = false - if (mcpHub) { - const servers = allowSet ? mcpHub.getServers().filter((s) => allowSet.has(s.name)) : mcpHub.getServers() - hasMcpServers = servers.length > 0 - } - const shouldIncludeMcp = hasMcpGroup && hasMcpServers - const codeIndexManager = CodeIndexManager.getInstance(context, cwd) + // Resolve the single, request-scoped effective tool policy ONCE, then have every + // prompt section and the MCP short-circuit derive from it. This is the one source of + // truth shared by prompt generation, API tool construction, runtime validation, and + // preview, so the prose never advertises a tool the model cannot actually call. + const policy = resolveEffectiveToolPolicy({ + mode, + customModes: customModeConfigs, + mcpHub, + disabledTools, + modelInfo, + experiments, + todoListEnabled: settings?.todoListEnabled, + codeIndexManager, + }) + // Tool calling is native-only. const effectiveProtocol = "native" const [modesSection, skillsSection] = await Promise.all([ getModesSection(context), - getSkillsSection(skillsManager, mode as string), + getSkillsSection(skillsManager, mode as string, policy), ]) // Tools catalog is not included in the system prompt. @@ -98,25 +108,17 @@ ${markdownFormattingSection()} ${getSharedToolUseSection()}${toolsCatalog} - ${getToolUseGuidelinesSection()} + ${getToolUseGuidelinesSection(policy)} -${ - // Forward the hub only when the mode actually exposes the MCP group, and pass the per-mode - // allowlist through so the capabilities section filters servers using the SAME convention as - // the tool-listing layer (a single source of truth for which servers are visible). This keeps - // the capability text consistent with the tools exposed in mixed cases (e.g. one allowed + - // one disallowed server), preventing the section from advertising MCP based on a disallowed - // server. `shouldIncludeMcp` is still used to short-circuit when no allowed server exists. - getCapabilitiesSection(cwd, hasMcpGroup ? mcpHub : undefined, allowedMcpServers) -} +${getCapabilitiesSection(policy)} ${modesSection} ${skillsSection ? `\n${skillsSection}` : ""} -${getRulesSection(cwd, settings)} +${getRulesSection(cwd, settings, policy)} -${getSystemInfoSection(cwd)} +${getSystemInfoSection(cwd, policy)} -${getObjectiveSection()} +${getObjectiveSection(policy)} ${await addCustomInstructions(baseInstructions, globalCustomInstructions || "", cwd, mode, { language: language ?? formatLanguage(vscode.env.language), @@ -144,6 +146,8 @@ export const SYSTEM_PROMPT = async ( todoList?: TodoItem[], modelId?: string, skillsManager?: SkillsManager, + disabledTools?: string[], + modelInfo?: ModelInfo, ): Promise => { if (!context) { throw new Error("Extension context is required for generating system prompt") @@ -172,5 +176,7 @@ export const SYSTEM_PROMPT = async ( todoList, modelId, skillsManager, + disabledTools, + modelInfo, ) } diff --git a/src/core/prompts/tools/__tests__/effective-tool-policy-warn.spec.ts b/src/core/prompts/tools/__tests__/effective-tool-policy-warn.spec.ts new file mode 100644 index 0000000000..f2bf667277 --- /dev/null +++ b/src/core/prompts/tools/__tests__/effective-tool-policy-warn.spec.ts @@ -0,0 +1,58 @@ +import { resolveEffectiveToolPolicy } from "../effective-tool-policy" + +/** + * `resolveEffectiveToolPolicy` must warn (once per process, per protocol + * tool) when `disabledTools` tries to disable a protocol tool, since the + * protocol guarantee makes such a disable a no-op. + * + * These tests live in their own file (not in `effective-tool-policy.spec.ts`) + * because the warn-dedupe set is module-level state and that spec already + * resolves a policy with `disabledTools: [...PROTOCOL_TOOLS]`, which would + * prime the set and make the "warned exactly once" assertion silently fail. + * Vitest gives each test file a fresh module registry, so the dedupe state + * starts empty here. + * + * Note: the "warns once / dedupes" assertions are combined into a single test + * that keeps one spy active across two resolves, because the dedupe set is + * shared across `it` blocks within a file — a later test's fresh spy would see + * zero calls if an earlier test had already primed the set. + */ +describe("resolveEffectiveToolPolicy - protocol override warning", () => { + /** Build a resolver input for a mode with all standard tool groups. */ + function input(disabledTools?: string[]) { + return { mode: "code", disabledTools } + } + + it("warns exactly once for a disabled protocol tool, dedupes on repeat, and keeps the tool available", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + try { + const policy = resolveEffectiveToolPolicy(input(["attempt_completion"])) + + // First resolve: warns exactly once and names the tool. + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(warnSpy.mock.calls[0]?.[0]).toContain("attempt_completion") + // The protocol guarantee still keeps the tool available. + expect(policy.tools.has("attempt_completion")).toBe(true) + + // Second resolve with the same protocol tool: no additional warn (dedupe). + resolveEffectiveToolPolicy(input(["attempt_completion", "execute_command"])) + expect(warnSpy).toHaveBeenCalledTimes(1) + } finally { + warnSpy.mockRestore() + } + }) + + it("does not warn when disabledTools contains no protocol tools", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + try { + const policy = resolveEffectiveToolPolicy(input(["execute_command", "read_file"])) + + expect(warnSpy).not.toHaveBeenCalled() + // Non-protocol disables still apply. + expect(policy.tools.has("execute_command")).toBe(false) + expect(policy.tools.has("read_file")).toBe(false) + } finally { + warnSpy.mockRestore() + } + }) +}) diff --git a/src/core/prompts/tools/__tests__/effective-tool-policy.spec.ts b/src/core/prompts/tools/__tests__/effective-tool-policy.spec.ts new file mode 100644 index 0000000000..05fccb0ae8 --- /dev/null +++ b/src/core/prompts/tools/__tests__/effective-tool-policy.spec.ts @@ -0,0 +1,706 @@ +import { customToolRegistry } from "@roo-code/core" +import type { ModeConfig, ModelInfo } from "@roo-code/types" + +import type { EffectiveToolPolicy } from "../effective-tool-policy" +import { + PROTOCOL_TOOLS, + resolveEffectiveToolPolicy, + resolveToolAlias, + buildToolRequirements, +} from "../effective-tool-policy" +import { getModeBySlug, defaultModeSlug } from "../../../../shared/modes" +import type { McpHub } from "../../../../services/mcp/McpHub" +import type { CodeIndexManager } from "../../../../services/code-index/manager" + +/** Build a policy by giving the custom mode `groups` (derived from a real custom mode config). */ +function policyFor( + groups: ModeConfig["groups"], + extra: Partial<{ + mcpHub: McpHub + disabledTools: string[] + modelInfo: ModelInfo + experiments: Record + todoListEnabled: boolean + codeIndexManager: CodeIndexManager + allowedMcpServers: string[] + }> = {}, +): EffectiveToolPolicy { + const customMode: ModeConfig = { + slug: "policy-test", + name: "Policy Under Test", + roleDefinition: "", + groups, + } + return resolveEffectiveToolPolicy({ + mode: "policy-test", + customModes: [customMode], + ...extra, + }) +} + +/** Minimal McpHub stub. Mirrors the McpServer shape the resolver reads (getServers, resources). */ +function makeMcpHub(servers: Array<{ name: string; resources?: unknown[]; tools?: unknown[] }>): McpHub { + return { getServers: () => servers } as unknown as McpHub +} + +/** CodeIndexManager stub with all "ready" flags true. */ +function enabledCodeIndexManager(): CodeIndexManager { + return { isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: true } as CodeIndexManager +} + +/** Build a ModelInfo satisfying the required schema fields, merged with test-specific overrides. */ +function modelInfo(partial?: Partial): ModelInfo { + return { contextWindow: 100_000, supportsPromptCache: true, ...partial } +} + +describe("resolveEffectiveToolPolicy - groups", () => { + it("grants read-group tools for a read mode", () => { + const policy = policyFor(["read"]) + expect(policy.tools.has("read_file")).toBe(true) + expect(policy.tools.has("codebase_search")).toBe(false) // gated by code index, off by default + expect(policy.tools.has("list_files")).toBe(true) + expect(policy.tools.has("search_files")).toBe(true) + }) + + it("grants edit-group tools for an edit mode", () => { + const policy = policyFor(["edit"]) + expect(policy.tools.has("write_to_file")).toBe(true) + expect(policy.tools.has("apply_diff")).toBe(true) + }) + + it("grants command-group tools for a command mode", () => { + const policy = policyFor(["command"]) + expect(policy.tools.has("execute_command")).toBe(true) + expect(policy.tools.has("read_command_output")).toBe(true) + }) + + it("combines groups", () => { + const policy = policyFor(["read", "edit", "command"]) + expect(policy.tools.has("read_file")).toBe(true) + expect(policy.tools.has("write_to_file")).toBe(true) + expect(policy.tools.has("execute_command")).toBe(true) + }) + + it("keeps always-available tools regardless of groups", () => { + const policy = policyFor([]) + // switch_mode/new_task are in the "modes" group but also always-available + expect(policy.tools.has("ask_followup_question")).toBe(true) + expect(policy.tools.has("update_todo_list")).toBe(true) + expect(policy.tools.has("skill")).toBe(true) + // run_slash_command is always-available but gated by the runSlashCommand experiment + expect(policy.tools.has("run_slash_command")).toBe(false) + }) + + it("sets hasMcpGroup only when the mode has the mcp group", () => { + expect(policyFor(["mcp"]).hasMcpGroup).toBe(true) + expect(policyFor(["read"]).hasMcpGroup).toBe(false) + }) + + it("extracts the first edit-restriction tuple with fileRegex", () => { + const policy = policyFor(["read", ["edit", { fileRegex: "\\.md$", description: "Markdown files only" }]]) + expect(policy.editRestriction).toEqual({ fileRegex: "\\.md$", description: "Markdown files only" }) + }) + + it("returns undefined editRestriction when no edit tuple has a fileRegex", () => { + expect(policyFor(["edit"]).editRestriction).toBeUndefined() + }) +}) + +describe("resolveEffectiveToolPolicy - disabledTools", () => { + it("removes tools listed in disabledTools (canonical)", () => { + const policy = policyFor(["read", "edit", "command"], { disabledTools: ["execute_command"] }) + expect(policy.tools.has("execute_command")).toBe(false) + expect(policy.tools.has("read_file")).toBe(true) + }) + + it("removes tools by alias (alias normalization)", () => { + const policy = policyFor(["edit"], { disabledTools: ["write_file"] }) + expect(policy.tools.has("write_to_file")).toBe(false) + }) + + it("does not remove the protocol guarantee", () => { + expect( + policyFor(["read", "edit", "command"], { disabledTools: [...PROTOCOL_TOOLS] }).tools.has( + "attempt_completion", + ), + ).toBe(true) + }) +}) + +describe("resolveEffectiveToolPolicy - model customization", () => { + it("removes tools in modelInfo.excludedTools", () => { + const policy = policyFor(["read", "edit", "command"], { + modelInfo: modelInfo({ excludedTools: ["read_file"] }), + }) + expect(policy.tools.has("read_file")).toBe(false) + }) + + it("removes tools by excludedTools alias", () => { + const policy = policyFor(["edit"], { modelInfo: modelInfo({ excludedTools: ["write_file"] }) }) + expect(policy.tools.has("write_to_file")).toBe(false) + }) + + it("re-adds excludedTools that are protocol tools", () => { + const policy = policyFor(["read", "edit", "command"], { + modelInfo: modelInfo({ excludedTools: ["attempt_completion"] }), + }) + expect(policy.tools.has("attempt_completion")).toBe(true) + }) + + it("adds includedTools only when their group is allowed", () => { + // read group is allowed; codebase_search is in read. + const policy = policyFor(["read"], { + modelInfo: modelInfo({ excludedTools: [], includedTools: ["codebase_search"] }), + codeIndexManager: enabledCodeIndexManager(), + }) + expect(policy.tools.has("codebase_search")).toBe(true) + }) + + it("ignores includedTools outside the allowed group", () => { + // command group only; codebase_search is in read -> not added even when requested. + const policy = policyFor(["command"], { modelInfo: modelInfo({ includedTools: ["read_file"] }) }) + expect(policy.tools.has("read_file")).toBe(false) + }) +}) + +describe("resolveEffectiveToolPolicy - conditional gates", () => { + it("drops codebase_search unless the code index is enabled/configured/initialized", () => { + const modeWithIndex = policyFor(["read"], { codeIndexManager: enabledCodeIndexManager() }) + expect(modeWithIndex.tools.has("codebase_search")).toBe(true) + + const modeWithoutIndex = policyFor(["read"]) + expect(modeWithoutIndex.tools.has("codebase_search")).toBe(false) + }) + + it("drops update_todo_list when todoListEnabled is false", () => { + expect(policyFor(["read", "edit", "command"], { todoListEnabled: false }).tools.has("update_todo_list")).toBe( + false, + ) + expect(policyFor(["read", "edit", "command"], { todoListEnabled: true }).tools.has("update_todo_list")).toBe( + true, + ) + }) + + it("drops generate_image unless the imageGeneration experiment is enabled", () => { + expect( + policyFor(["read", "edit", "command"], { experiments: { imageGeneration: true } }).tools.has( + "generate_image", + ), + ).toBe(true) + expect(policyFor(["read", "edit", "command"]).tools.has("generate_image")).toBe(false) + }) + + it("drops run_slash_command unless the runSlashCommand experiment is enabled", () => { + expect( + policyFor(["read", "edit", "command"], { experiments: { runSlashCommand: true } }).tools.has( + "run_slash_command", + ), + ).toBe(true) + expect(policyFor(["read", "edit", "command"]).tools.has("run_slash_command")).toBe(false) + }) +}) + +describe("resolveEffectiveToolPolicy - MCP resource gate", () => { + it("keeps access_mcp_resource iff an allowed server exposes resources", () => { + const hasResources = policyFor(["mcp"], { mcpHub: makeMcpHub([{ name: "s", resources: [{ uri: "r" }] }]) }) + expect(hasResources.tools.has("access_mcp_resource")).toBe(true) + + const noResources = policyFor(["mcp"], { mcpHub: makeMcpHub([{ name: "s" }]) }) + expect(noResources.tools.has("access_mcp_resource")).toBe(false) + }) + + it("respects an explicit allowlist over the mode-config allowlist", () => { + const allowed = policyFor(["mcp"], { + mcpHub: makeMcpHub([{ name: "allowed", resources: [{ uri: "r" }] }]), + allowedMcpServers: ["allowed"], + }) + expect(allowed.tools.has("access_mcp_resource")).toBe(true) + + const wrongAllow = policyFor(["mcp"], { + mcpHub: makeMcpHub([{ name: "allowed", resources: [{ uri: "r" }] }]), + allowedMcpServers: ["blocked"], + }) + expect(wrongAllow.tools.has("access_mcp_resource")).toBe(false) + }) + + it("falls back to the mode config allowlist when no explicit allowlist is provided", () => { + const customMode: ModeConfig = { + slug: "policy-test", + name: "Restricted Mode", + roleDefinition: "", + groups: ["mcp"], + allowedMcpServers: ["blocked"], + } + const policy = resolveEffectiveToolPolicy({ + mode: "policy-test", + customModes: [customMode], + mcpHub: makeMcpHub([{ name: "allowed", resources: [{ uri: "r" }] }]), + }) + expect(policy.tools.has("access_mcp_resource")).toBe(false) + }) + + it("computes hasMcpTools from effective enabled tools and hasMcpResources from resources", () => { + const hasToolsOnly = policyFor(["mcp"], { + mcpHub: makeMcpHub([{ name: "s", tools: [{ name: "t", description: "d" }] }]), + }) + expect(hasToolsOnly.hasMcpTools).toBe(true) + expect(hasToolsOnly.hasMcpResources).toBe(false) + + const hasResourcesOnly = policyFor(["mcp"], { mcpHub: makeMcpHub([{ name: "s", resources: [{ uri: "r" }] }]) }) + expect(hasResourcesOnly.hasMcpTools).toBe(false) + expect(hasResourcesOnly.hasMcpResources).toBe(true) + + const hasNeither = policyFor(["mcp"], { mcpHub: makeMcpHub([{ name: "s" }]) }) + expect(hasNeither.hasMcpTools).toBe(false) + expect(hasNeither.hasMcpResources).toBe(false) + }) + + it("returns hasMcpTools false when the only tool has enabledForPrompt: false", () => { + const policy = policyFor(["mcp"], { + mcpHub: makeMcpHub([{ name: "s", tools: [{ name: "t", enabledForPrompt: false }] }]), + }) + expect(policy.hasMcpTools).toBe(false) + }) + + it("returns hasMcpTools true when a tool has enabledForPrompt: true", () => { + const policy = policyFor(["mcp"], { + mcpHub: makeMcpHub([{ name: "s", tools: [{ name: "t", enabledForPrompt: true }] }]), + }) + expect(policy.hasMcpTools).toBe(true) + }) + + it("returns hasMcpTools false for a server excluded by the allowlist", () => { + const policy = policyFor(["mcp"], { + mcpHub: makeMcpHub([{ name: "excluded", tools: [{ name: "t", enabledForPrompt: true }] }]), + allowedMcpServers: ["other"], + }) + expect(policy.hasMcpTools).toBe(false) + expect(policy.tools.has("use_mcp_tool")).toBe(false) + }) + + it("keeps use_mcp_tool only when an allowed server exposes a prompt-enabled tool", () => { + const withTools = policyFor(["mcp"], { + mcpHub: makeMcpHub([{ name: "s", tools: [{ name: "t", enabledForPrompt: true }] }]), + }) + expect(withTools.tools.has("use_mcp_tool")).toBe(true) + + const allDisabled = policyFor(["mcp"], { + mcpHub: makeMcpHub([{ name: "s", tools: [{ name: "t", enabledForPrompt: false }] }]), + }) + expect(allDisabled.tools.has("use_mcp_tool")).toBe(false) + }) + + it("drops use_mcp_tool when mcpHub is undefined even though the mcp group is granted", () => { + const policy = policyFor(["mcp"]) + expect(policy.hasMcpGroup).toBe(true) + expect(policy.hasMcpTools).toBe(false) + expect(policy.tools.has("use_mcp_tool")).toBe(false) + expect(policy.tools.has("access_mcp_resource")).toBe(false) + }) + + it("drops use_mcp_tool when the allowedMcpServers list is empty", () => { + const policy = policyFor(["mcp"], { + mcpHub: makeMcpHub([ + { name: "s", tools: [{ name: "t", enabledForPrompt: true }], resources: [{ uri: "r" }] }, + ]), + allowedMcpServers: [], + }) + // An empty allowlist permits no servers: both MCP group tools must go, + // even though the hub itself exposes a live tool and a resource. + expect(policy.hasMcpTools).toBe(false) + expect(policy.hasMcpResources).toBe(false) + expect(policy.tools.has("use_mcp_tool")).toBe(false) + expect(policy.tools.has("access_mcp_resource")).toBe(false) + }) + + it("keeps use_mcp_tool with resources-only hub and access_mcp_resource pruned", () => { + // The two group tools are gated independently: resources alone keep + // access_mcp_resource but must not resurrect use_mcp_tool. + const policy = policyFor(["mcp"], { mcpHub: makeMcpHub([{ name: "s", resources: [{ uri: "r" }] }]) }) + expect(policy.tools.has("access_mcp_resource")).toBe(true) + expect(policy.tools.has("use_mcp_tool")).toBe(false) + }) +}) + +describe("resolveEffectiveToolPolicy - worst case (control-tools-only mode)", () => { + it("only exposes always-available + protocol tools when groups is empty", () => { + const policy = policyFor([]) + expect(policy.tools.has("read_file")).toBe(false) + expect(policy.tools.has("write_to_file")).toBe(false) + expect(policy.tools.has("execute_command")).toBe(false) + expect(policy.tools.has("attempt_completion")).toBe(true) // protocol guarantee + expect(policy.tools.has("switch_mode")).toBe(true) // always-available + }) +}) + +describe("buildToolRequirements", () => { + it("returns an empty map when disabledTools is undefined or empty", () => { + expect(buildToolRequirements(undefined)).toEqual({}) + expect(buildToolRequirements([])).toEqual({}) + }) + + it("maps disabled tools to false (including alias + canonical)", () => { + const reqs = buildToolRequirements(["write_file"]) + expect(reqs).toEqual({ write_file: false, write_to_file: false }) + }) + + it("skips protocol tools and their aliases", () => { + const reqs = buildToolRequirements([...PROTOCOL_TOOLS, "ask_followup_question", "switch_mode"]) + expect(Object.keys(reqs)).not.toContain("attempt_completion") + expect(reqs).toEqual({ ask_followup_question: false, switch_mode: false }) + }) + + it("adds alias + canonical for real aliases", () => { + const reqs = buildToolRequirements(["write_file"]) + expect(Object.keys(reqs).sort()).toEqual(["write_file", "write_to_file"].sort()) + }) + + it("skips protocol tools but keeps regular tools in a mixed list", () => { + // A protocol tool must be skipped while the regular tool in the same list + // still produces its alias + canonical entries. + expect(buildToolRequirements(["attempt_completion", "write_file"])).toEqual({ + write_file: false, + write_to_file: false, + }) + }) +}) + +describe("resolveToolAlias", () => { + it("resolves every registered alias to its canonical tool", () => { + // Exercises the module-load ALIAS_TO_CANONICAL map for both registered aliases. + expect(resolveToolAlias("write_file")).toBe("write_to_file") + expect(resolveToolAlias("search_and_replace")).toBe("edit") + }) + + it("returns canonical and unknown names unchanged", () => { + expect(resolveToolAlias("read_file")).toBe("read_file") + expect(resolveToolAlias("not_a_tool")).toBe("not_a_tool") + }) +}) + +describe("PROTOCOL_TOOLS", () => { + it("lists the single protocol tool by canonical name", () => { + expect([...PROTOCOL_TOOLS]).toEqual(["attempt_completion"]) + }) +}) + +describe("resolveEffectiveToolPolicy - edit restriction edge cases", () => { + it("skips non-edit group tuples even when they declare a fileRegex", () => { + // Only an actual `edit` tuple can establish the restriction: a `read` tuple + // carrying a fileRegex must be skipped, and an edit tuple without a fileRegex + // must not produce one either. + const policy = policyFor([["read", { fileRegex: "\\.ts$" }], ["edit", {}], "command"]) + expect(policy.editRestriction).toBeUndefined() + }) + + it("does not crash on a malformed edit tuple without options", () => { + // Runtime guard: the extraction uses `group[1]?.fileRegex`, so an options-less + // tuple must be skipped rather than throwing. + const groups = JSON.parse('[["edit"]]') as ModeConfig["groups"] + expect(policyFor(groups).editRestriction).toBeUndefined() + }) +}) + +describe("resolveEffectiveToolPolicy - step 3 validator removal", () => { + it("drops granted tools when the validator does not recognize the mode", () => { + const customMode: ModeConfig = { + slug: "policy-test", + name: "Policy Under Test", + roleDefinition: "", + groups: ["read", "edit", "command"], + } + // The requested slug matches no mode, so the fallback (architect) grants its + // read/edit/mcp tools but the per-tool validator rejects every non-always-available + // tool, and the step-3 removal loop drops them. + const policy = resolveEffectiveToolPolicy({ mode: "ghost-mode", customModes: [customMode] }) + expect(policy.tools.has("read_file")).toBe(false) + expect(policy.tools.has("write_to_file")).toBe(false) + expect(policy.tools.has("use_mcp_tool")).toBe(false) + expect(policy.tools.has("switch_mode")).toBe(true) + expect(policy.tools.has("attempt_completion")).toBe(true) + }) + + it("re-adds validator-removed group tools via includedTools (regular-tool mapping)", () => { + // The includedTools branch maps every regular group tool through + // TOOL_GROUPS; when a granted tool was dropped by the step-3 validator + // (unknown mode slug), including it re-adds it because its group is allowed + // by the fallback mode config. + const customMode: ModeConfig = { + slug: "policy-test", + name: "Policy Under Test", + roleDefinition: "", + groups: ["read", "edit", "command"], + } + const policy = resolveEffectiveToolPolicy({ + mode: "ghost-mode", + customModes: [customMode], + modelInfo: modelInfo({ includedTools: ["read_file"] }), + }) + expect(policy.tools.has("read_file")).toBe(true) + }) + + it("threads the experiments flags into the per-mode validator", () => { + // The resolver forwards `experiments ?? {}` to the validator; the customTools + // escape hatch in isToolAllowedForMode only fires when that flag actually + // arrives. A registered custom tool is therefore retained for an otherwise + // unknown mode when (and only when) the flag is passed through. + const customMode: ModeConfig = { + slug: "policy-test", + name: "Policy Under Test", + roleDefinition: "", + groups: ["read"], + } + customToolRegistry.register({ name: "shadow_read_tool", description: "test double", execute: async () => "ok" }) + try { + const withFlag = resolveEffectiveToolPolicy({ + mode: "ghost-mode", + customModes: [customMode], + experiments: { customTools: true }, + }) + // shadow_read_tool is not granted by any group, so the flag alone cannot + // re-add it; instead the flag must keep granted tools that the validator + // would otherwise reject for the unknown mode. + expect(withFlag.tools.has("read_file")).toBe(false) + + // Direct proof of flag threading: register under a granted tool's name. + customToolRegistry.register({ name: "read_file", description: "shadow", execute: async () => "ok" }) + const shadowed = resolveEffectiveToolPolicy({ + mode: "ghost-mode", + customModes: [customMode], + experiments: { customTools: true }, + }) + expect(shadowed.tools.has("read_file")).toBe(true) + + // Without the flag the same shadowed tool is still rejected. + const withoutFlag = resolveEffectiveToolPolicy({ mode: "ghost-mode", customModes: [customMode] }) + expect(withoutFlag.tools.has("read_file")).toBe(false) + } finally { + customToolRegistry.clear() + } + }) + + it("forwards an empty customModes default to the per-mode validator", async () => { + // The step-3 permission filter forwards `customModes ?? []` (and + // `experiments ?? {}`) to isToolAllowedForMode. A phantom default entry would + // behave identically downstream (a non-object never matches a mode slug), so + // the forwarded argument itself is the only observable. Wrap the real + // validator for one fresh module instance and assert what it receives. + const seen: unknown[][] = [] + vi.doMock("../../../../core/tools/validateToolUse", async (importOriginal) => { + const original = await importOriginal() + return { + ...original, + isToolAllowedForMode: (...args: Parameters) => { + seen.push(args) + return original.isToolAllowedForMode(...args) + }, + } + }) + vi.resetModules() + const mod = await import("../effective-tool-policy") + try { + mod.resolveEffectiveToolPolicy({ mode: "code" }) + expect(seen.length).toBeGreaterThan(0) + for (const args of seen) { + expect(args[2]).toEqual([]) + } + + // Provided custom modes are forwarded by reference, unchanged. + const customModes: ModeConfig[] = [ + { slug: "passthrough-test", name: "PT", roleDefinition: "", groups: ["read"] }, + ] + seen.length = 0 + mod.resolveEffectiveToolPolicy({ mode: "code", customModes }) + expect(seen.some((args) => args[2] === customModes)).toBe(true) + } finally { + vi.doUnmock("../../../../core/tools/validateToolUse") + vi.resetModules() + } + }) +}) + +describe("resolveEffectiveToolPolicy - opt-in custom tools via includedTools", () => { + it("adds opt-in custom tools only when their group is allowed", () => { + // "edit" is an opt-in custom tool of the edit group: absent from the group grant, + // it is re-added only when model customization includes it AND the mode allows + // the owning group (the toolToGroup map includes customTools entries). + const withEditGroup = policyFor(["edit"], { modelInfo: modelInfo({ includedTools: ["edit"] }) }) + expect(withEditGroup.tools.has("edit")).toBe(true) + + const withoutEditGroup = policyFor(["read"], { modelInfo: modelInfo({ includedTools: ["edit"] }) }) + expect(withoutEditGroup.tools.has("edit")).toBe(false) + }) + + it("resolves aliased opt-in custom tools through the group's customTools", () => { + // "search_and_replace" is an alias of the opt-in custom tool "edit". + const policy = policyFor(["edit"], { modelInfo: modelInfo({ includedTools: ["search_and_replace"] }) }) + expect(policy.tools.has("edit")).toBe(true) + }) +}) + +describe("resolveEffectiveToolPolicy - code index readiness flags", () => { + it("drops codebase_search when the feature is disabled", () => { + const manager = { + isFeatureEnabled: false, + isFeatureConfigured: true, + isInitialized: true, + } as CodeIndexManager + expect(policyFor(["read"], { codeIndexManager: manager }).tools.has("codebase_search")).toBe(false) + }) + + it("drops codebase_search when the feature is not configured", () => { + const manager = { + isFeatureEnabled: true, + isFeatureConfigured: false, + isInitialized: true, + } as CodeIndexManager + expect(policyFor(["read"], { codeIndexManager: manager }).tools.has("codebase_search")).toBe(false) + }) + + it("drops codebase_search when the index is not initialized", () => { + const manager = { + isFeatureEnabled: true, + isFeatureConfigured: true, + isInitialized: false, + } as CodeIndexManager + expect(policyFor(["read"], { codeIndexManager: manager }).tools.has("codebase_search")).toBe(false) + }) +}) + +describe("resolveEffectiveToolPolicy - MCP capability flags", () => { + it("reports no MCP capabilities without an mcpHub", () => { + const policy = policyFor(["mcp"]) + expect(policy.hasMcpTools).toBe(false) + expect(policy.hasMcpResources).toBe(false) + }) + + it("keeps hasMcpGroup true when mcp is mixed with other groups", () => { + expect(policyFor(["read", "mcp", "command"]).hasMcpGroup).toBe(true) + }) + + it("returns hasMcpTools true for an allowlisted server even when other servers are dropped", () => { + const policy = policyFor(["mcp"], { + mcpHub: makeMcpHub([ + { name: "other", tools: [{ name: "t", enabledForPrompt: true }] }, + { name: "listed", tools: [{ name: "t", enabledForPrompt: true }] }, + ]), + allowedMcpServers: ["listed"], + }) + expect(policy.hasMcpTools).toBe(true) + }) + + it("returns hasMcpTools true when at least one of several tools is prompt-enabled", () => { + const policy = policyFor(["mcp"], { + mcpHub: makeMcpHub([ + { + name: "s", + tools: [ + { name: "off-a", enabledForPrompt: false }, + { name: "off-b", enabledForPrompt: false }, + { name: "live", enabledForPrompt: true }, + ], + }, + ]), + }) + expect(policy.hasMcpTools).toBe(true) + }) + + it("returns hasMcpTools false when every tool of the server is prompt-disabled", () => { + const policy = policyFor(["mcp"], { + mcpHub: makeMcpHub([ + { + name: "s", + tools: [ + { name: "off-a", enabledForPrompt: false }, + { name: "off-b", enabledForPrompt: false }, + ], + }, + ]), + }) + expect(policy.hasMcpTools).toBe(false) + }) +}) + +describe("resolveEffectiveToolPolicy - protocol override warning (fresh module)", () => { + // The warn-dedupe set is module state and other tests in this file already resolve + // policies that disable protocol tools (priming the set), so each test reloads a + // fresh module instance whose dedupe set starts empty. + async function freshResolve() { + vi.resetModules() + const mod = await import("../effective-tool-policy") + return mod.resolveEffectiveToolPolicy + } + + it("skips an alias of a protocol tool when building tool requirements", async () => { + // buildToolRequirements skips a tool when its canonical name OR its raw name + // is a protocol tool. Register a temporary alias of attempt_completion so the + // two operands of that `||` differ: the alias must still be skipped. + vi.resetModules() + const toolsMod = await import("../../../../shared/tools") + toolsMod.TOOL_ALIASES.wp4_attempt_alias = "attempt_completion" + const mod = await import("../effective-tool-policy") + try { + expect(mod.buildToolRequirements(["wp4_attempt_alias"])).toEqual({}) + // Sanity: the injected alias actually resolves through the fresh module. + expect(mod.resolveToolAlias("wp4_attempt_alias")).toBe("attempt_completion") + } finally { + delete toolsMod.TOOL_ALIASES.wp4_attempt_alias + } + }) + + it("warns once per protocol tool, names the tool, and keeps it available", async () => { + vi.resetModules() + // Import shared/tools first and assert the alias map right after the fresh + // import, so a broken module-load alias map (ALIAS_TO_CANONICAL) fails here. + const mod = await import("../effective-tool-policy") + expect(mod.resolveToolAlias("write_file")).toBe("write_to_file") + expect(mod.resolveToolAlias("search_and_replace")).toBe("edit") + expect([...mod.PROTOCOL_TOOLS]).toEqual(["attempt_completion"]) + const resolve = mod.resolveEffectiveToolPolicy + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + try { + const policy = resolve({ mode: "code", disabledTools: ["attempt_completion"] }) + expect(warnSpy).toHaveBeenCalledTimes(1) + const message = String(warnSpy.mock.calls[0]?.[0]) + expect(message).toContain("[effective-tool-policy]") + expect(message).toContain("'attempt_completion'") + expect(message).toContain("no-op") + expect(policy.tools.has("attempt_completion")).toBe(true) + + // Second resolve on the same module instance: deduped, no additional warn. + const policy2 = resolve({ mode: "code", disabledTools: ["attempt_completion", "execute_command"] }) + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(policy2.tools.has("execute_command")).toBe(false) + } finally { + warnSpy.mockRestore() + } + }) + + it("does not warn or throw for empty or missing disabledTools", async () => { + const resolve = await freshResolve() + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + try { + expect(() => resolve({ mode: "code", disabledTools: [] })).not.toThrow() + expect(() => resolve({ mode: "code" })).not.toThrow() + expect(warnSpy).not.toHaveBeenCalled() + } finally { + warnSpy.mockRestore() + } + }) + + it("warns again on a fresh module instance (per-process dedupe)", async () => { + const first = await freshResolve() + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + try { + first({ mode: "code", disabledTools: ["attempt_completion"] }) + expect(warnSpy).toHaveBeenCalledTimes(1) + + // A different module instance has its own dedupe set and warns again. + const second = await freshResolve() + second({ mode: "code", disabledTools: ["attempt_completion"] }) + expect(warnSpy).toHaveBeenCalledTimes(2) + } finally { + warnSpy.mockRestore() + } + }) +}) diff --git a/src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts b/src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts index bc3cd0a360..4071836679 100644 --- a/src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts +++ b/src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts @@ -1,8 +1,9 @@ // npx vitest run core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts import type OpenAI from "openai" +import type { ModeConfig } from "@roo-code/types" -import { filterNativeToolsForMode } from "../filter-tools-for-mode" +import { filterMcpToolsForMode, filterNativeToolsForMode } from "../filter-tools-for-mode" function makeTool(name: string): OpenAI.Chat.ChatCompletionTool { return { @@ -90,6 +91,198 @@ describe("filterNativeToolsForMode - disabledTools", () => { }) }) +describe("filterNativeToolsForMode - settings round-trips", () => { + const nativeTools: OpenAI.Chat.ChatCompletionTool[] = [makeTool("read_file"), makeTool("update_todo_list")] + + function resultNames(result: OpenAI.Chat.ChatCompletionTool[]): string[] { + return result.map((t) => ("function" in t && t.function ? t.function.name : "")) + } + + it("works when the settings argument is omitted entirely", () => { + // settings?.disabledTools / settings?.todoListEnabled must tolerate an + // absent settings object rather than dereferencing it. + const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined) + expect(resultNames(result)).toContain("read_file") + }) + + it("applies settings.todoListEnabled=false to the native tool set", () => { + const without = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, { + todoListEnabled: false, + }) + expect(resultNames(without)).not.toContain("update_todo_list") + expect(resultNames(without)).toContain("read_file") + + const enabled = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, { + todoListEnabled: true, + }) + expect(resultNames(enabled)).toContain("update_todo_list") + }) + + it("keeps todoListEnabled=undefined as enabled (default semantics)", () => { + const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, {}) + expect(resultNames(result)).toContain("update_todo_list") + }) + + it("tolerates a modelInfo without an includedTools property", () => { + // resolveModelAliasRenames guards with `modelInfo?.includedTools?.length`; + // a present-but-incomplete modelInfo must take the early-return path rather + // than dereferencing the missing property. + const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, { + modelInfo: {}, + }) + expect(resultNames(result)).toContain("read_file") + expect(resultNames(result)).toContain("update_todo_list") + }) +}) + +describe("filterNativeToolsForMode - alias renaming", () => { + function resultNames(result: OpenAI.Chat.ChatCompletionTool[]): string[] { + return result.map((t) => ("function" in t && t.function ? t.function.name : "")) + } + + it("renames an allowed canonical tool to its alias from includedTools", () => { + // "search_and_replace" is an alias of the opt-in custom tool "edit"; listing + // it in modelInfo.includedTools both enables "edit" and renames it, so the + // advertised definition must carry the alias name, not the canonical one. + const nativeTools = [makeTool("edit")] + const settings = { modelInfo: { includedTools: ["search_and_replace"] } } + + const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, settings) + + expect(resultNames(result)).toEqual(["search_and_replace"]) + }) + + it("keeps non-aliased tool definitions identical (no needless copies)", () => { + // A canonical name in includedTools is not an alias; the tool must be passed + // through as the exact same definition object rather than renamed/copied. + const readFileTool = makeTool("read_file") + const settings = { modelInfo: { includedTools: ["read_file"] } } + + const result = filterNativeToolsForMode([readFileTool], "code", undefined, undefined, undefined, settings) + + expect(result).toHaveLength(1) + expect(result[0]).toBe(readFileTool) + }) + + it("does not advertise an alias whose canonical tool is not allowed", () => { + // "edit" needs the edit group; a read-only mode must drop it even when the + // alias is requested through includedTools. + const nativeTools = [makeTool("edit"), makeTool("read_file")] + const settings = { modelInfo: { includedTools: ["search_and_replace"] } } + const readOnlyMode: ModeConfig = { + slug: "read-only", + name: "Read Only", + roleDefinition: "", + groups: ["read"], + } + + const result = filterNativeToolsForMode( + nativeTools, + "read-only", + [readOnlyMode], + undefined, + undefined, + settings, + ) + + const names = resultNames(result) + expect(names).not.toContain("search_and_replace") + expect(names).not.toContain("edit") + expect(names).toContain("read_file") + }) + + it("reuses the cached renamed definition for repeated calls", () => { + // Uses the write_file pair exclusively: the module-level rename cache is + // shared across tests in this file, so the first call below must be the one + // that stores the entry (dropping the cache write would return fresh objects). + const nativeTools = [makeTool("write_to_file")] + const settings = { modelInfo: { includedTools: ["write_file"] } } + + const first = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, settings) + const second = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, settings) + + expect(resultNames(first)).toEqual(["write_file"]) + expect(second[0]).toBe(first[0]) + }) + + it("keeps separate cache entries per canonical/alias pair", () => { + // Two different renames must not collide in the rename cache: each advertised + // tool carries its own alias name. + const nativeTools = [makeTool("edit"), makeTool("write_to_file")] + const settings = { modelInfo: { includedTools: ["search_and_replace", "write_file"] } } + + const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, settings) + + expect(resultNames(result).sort()).toEqual(["search_and_replace", "write_file"]) + }) + + it("skips non-function (custom) tool definitions without throwing", () => { + // The filter loop only inspects definitions that carry a function schema; + // a custom tool definition must be dropped, not dereferenced. + const customTool: OpenAI.Chat.ChatCompletionTool = { type: "custom", custom: { name: "custom_tool" } } + const nativeTools = [makeTool("read_file"), customTool] + + const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, {}) + + expect(resultNames(result)).toEqual(["read_file"]) + }) + + it("skips a malformed function definition whose schema is missing", () => { + // Defensive branch: a definition that declares the "function" key but carries + // a nullish schema must be skipped by the loop guard rather than dereferenced. + // The double assertion is required because the SDK types forbid this shape. + const malformedTool = { + ...makeTool("broken_tool"), + function: undefined, + } as unknown as OpenAI.Chat.ChatCompletionTool + const nativeTools = [makeTool("read_file"), malformedTool] + + const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, {}) + + expect(resultNames(result)).toEqual(["read_file"]) + }) +}) + +describe("filterMcpToolsForMode", () => { + const mcpTools = [makeTool("mcp_server_tool")] + + it("returns the MCP tools for a mode whose groups include mcp", () => { + expect(filterMcpToolsForMode(mcpTools, "code", undefined, undefined)).toBe(mcpTools) + }) + + it("returns the MCP tools when the mode is undefined (default-mode fallback)", () => { + // `mode ?? defaultModeSlug` must fall back to the default mode (code), which + // allows use_mcp_tool. + expect(filterMcpToolsForMode(mcpTools, undefined, undefined, undefined)).toBe(mcpTools) + }) + + it("returns an empty array for a mode without the mcp group", () => { + const readOnlyMode: ModeConfig = { + slug: "read-only", + name: "Read Only", + roleDefinition: "", + groups: ["read"], + } + expect(filterMcpToolsForMode(mcpTools, "read-only", [readOnlyMode], undefined)).toEqual([]) + }) + + it("resolves a custom mode from the customModes argument", () => { + // The customModes array must be forwarded to the permission check: the mode + // slug only exists in the custom list. + const mcpCustomMode: ModeConfig = { + slug: "custom-mcp", + name: "Custom MCP", + roleDefinition: "", + groups: ["mcp"], + } + expect(filterMcpToolsForMode(mcpTools, "custom-mcp", [mcpCustomMode], undefined)).toBe(mcpTools) + }) + + it("accepts experiment flags without affecting the result", () => { + expect(filterMcpToolsForMode(mcpTools, "code", undefined, { imageGeneration: true })).toBe(mcpTools) + }) +}) + describe("filterNativeToolsForMode - access_mcp_resource allowlist", () => { const nativeTools: OpenAI.Chat.ChatCompletionTool[] = [makeTool("read_file"), makeTool("access_mcp_resource")] diff --git a/src/core/prompts/tools/effective-tool-policy.ts b/src/core/prompts/tools/effective-tool-policy.ts new file mode 100644 index 0000000000..5a800da7b6 --- /dev/null +++ b/src/core/prompts/tools/effective-tool-policy.ts @@ -0,0 +1,354 @@ +import type { ModeConfig, ToolGroup, ModelInfo, GroupEntry } from "@roo-code/types" +import { getModeBySlug, defaultModeSlug, getGroupName, getToolsForMode } from "../../../shared/modes" +import { TOOL_ALIASES, TOOL_GROUPS } from "../../../shared/tools" +import type { CodeIndexManager } from "../../../services/code-index/manager" +import type { McpHub } from "../../../services/mcp/McpHub" +import { isToolAllowedForMode } from "../../../core/tools/validateToolUse" + +/** + * Canonical tool names that participate in the task-completion protocol and must + * remain logically available even when a profile disables them. + * + * The effective tool policy re-adds every one of these after `disabledTools` and + * model-specific exclusions have been applied, so the system prompt and the API's + * logical allowed set always agree that these tools can be called. + * + * `attempt_completion` is the only tool with no coherent prompt state when absent + * (the task loop can only exit through it), so it is the sole protocol guarantee. + */ +export const PROTOCOL_TOOLS: readonly string[] = ["attempt_completion"] + +/** + * Extract the first edit restriction declared by a mode's groups, if any. + * + * A group entry may be either a bare group name (string) or a tuple of + * `[groupName, options]`. Only a tuple entry with a `fileRegex` establishes a + * prompt-visible edit restriction. + * + * Returning only the first restriction is intentional: the mode schema rejects + * duplicate groups (the `rawGroupEntryArraySchema` refine in + * `packages/types/src/mode.ts`), so a mode can declare at most one `edit` group + * with a `fileRegex`; and the runtime validator (`validateToolUse.ts`) likewise + * returns at the first matching group, so the prompt and the validator agree. + * + * @param groups The mode's group entries. + * @returns The first `{ fileRegex, description }` found, or undefined when the + * mode declares no restricted edit group. + */ +function getEditRestriction(groups: readonly GroupEntry[]): + | { + fileRegex: string + description?: string + } + | undefined { + for (const group of groups) { + const groupName = getGroupName(group) + if (groupName !== "edit") { + continue + } + if (Array.isArray(group) && group[1]?.fileRegex) { + return { fileRegex: group[1].fileRegex, description: group[1].description } + } + } + return undefined +} + +/** + * Reverse lookup map - maps alias name to canonical tool name. + * Built once at module load from the central TOOL_ALIASES constant. + */ +const ALIAS_TO_CANONICAL: Map = new Map( + Object.entries(TOOL_ALIASES).map(([alias, canonical]) => [alias, canonical]), +) + +/** + * Resolves a tool name to its canonical name. + * If the tool name is an alias, returns the canonical tool name. + * If it's already a canonical name or unknown, returns as-is. + * + * @param toolName - The tool name to resolve (may be an alias) + * @returns The canonical tool name + */ +export function resolveToolAlias(toolName: string): string { + const canonical = ALIAS_TO_CANONICAL.get(toolName) + return canonical ?? toolName +} + +/** + * Canonical protocol-tool names already warned about. Module-level so the + * protocol-override warning fires at most once per tool per process. + */ +const warnedProtocolOverrides = new Set() + +/** + * Warns once per process, per protocol tool, when `disabledTools` tries to + * disable a protocol tool — which the protocol guarantee step makes a no-op. + * + * Uses `console.warn` (not the shared `logger`, which is a no-op in production) + * so the no-op disable is visible to extension developers. + * + * @param disabledTools The raw disabled-tools list (may contain aliases). + */ +function warnProtocolToolOverrides(disabledTools?: string[]): void { + if (!disabledTools?.length) { + return + } + for (const toolName of disabledTools) { + const canonical = resolveToolAlias(toolName) + if (PROTOCOL_TOOLS.includes(canonical) && !warnedProtocolOverrides.has(canonical)) { + warnedProtocolOverrides.add(canonical) + console.warn( + `[effective-tool-policy] '${canonical}' is a protocol tool: disabling it via disabledTools is a no-op; it remains available.`, + ) + } + } +} + +export interface EffectiveToolPolicyInput { + mode: string + customModes?: ModeConfig[] + mcpHub?: McpHub + disabledTools?: string[] + modelInfo?: ModelInfo + experiments?: Record + todoListEnabled?: boolean + codeIndexManager?: CodeIndexManager + /** + * Optional explicit per-mode MCP server allowlist. When provided it takes + * precedence; when omitted the resolver falls back to the mode config's own + * allowlist (defense in depth), so a restricted mode can never retain + * `access_mcp_resource` based on resources from disallowed servers. + */ + allowedMcpServers?: string[] +} + +export interface EffectiveToolPolicy { + /** Canonical tool names logically available for this request (after all filters, incl. protocol guarantee) */ + tools: ReadonlySet + hasMcpGroup: boolean // mode's groups include "mcp" + hasMcpTools: boolean // ≥1 dynamic MCP tool enabled for allowed servers + hasMcpResources: boolean // ≥1 accessible resource on allowed servers + /** + * The mode's first edit-group file restriction. First-only is intentional: + * the mode schema rejects duplicate groups, so at most one `edit` group can + * carry a `fileRegex`, and the runtime validator likewise stops at the first + * matching group — prompt and validator agree. + */ + editRestriction?: { fileRegex: string; description?: string } +} + +/** + * True when at least one dynamic MCP tool (e.g. `mcp_serverName_toolName`) is + * enabled for the allowed servers. Used both to gate the MCP capability bullet in + * the prompt and to prune `use_mcp_tool` from the policy's tool set, so servers + * whose every tool is `enabledForPrompt: false` do not count. + * + * Cheap existence check: it inspects the MCP server snapshot directly (allowlist + * + `enabledForPrompt !== false`, mirroring the `getMcpServerTools` filter) and + * never materializes or normalizes tool schemas. + * + * @param mcpHub The MCP hub, or undefined when MCP is unavailable (always false). + * @param allowedServers Optional per-mode server allowlist; when provided only + * these servers are considered. + * @returns True when at least one allowed server exposes a prompt-enabled tool. + */ +function resolveHasMcpTools(mcpHub?: McpHub, allowedServers?: string[]): boolean { + if (!mcpHub) { + return false + } + let servers = mcpHub.getServers() + if (allowedServers) { + const allowSet = new Set(allowedServers) + servers = servers.filter((server) => allowSet.has(server.name)) + } + return servers.some((server) => server.tools?.some((tool) => tool.enabledForPrompt !== false)) +} + +/** + * True when `mcpHub` exposes at least one accessible resource on the allowed servers. + * + * When `allowedServers` is provided, only servers whose name is in the allowlist + * are considered, keeping the `access_mcp_resource` availability check consistent + * with the mode's MCP server allowlist. + * + * @param mcpHub The MCP hub whose server snapshot is inspected. + * @param allowedServers Optional per-mode server allowlist; when provided only + * these servers are considered. + * @returns True when at least one allowed server exposes one or more resources. + */ +export function hasAnyMcpResources(mcpHub: McpHub, allowedServers?: string[]): boolean { + let servers = mcpHub.getServers() + if (allowedServers) { + const allowSet = new Set(allowedServers) + servers = servers.filter((server) => allowSet.has(server.name)) + } + return servers.some((server) => server.resources && server.resources.length > 0) +} + +/** + * Computes the request-scoped effective tool policy: the set of tool names + * logically available for a single request, together with the MCP and edit + * metadata the system prompt needs. + * + * This is the single source of truth shared by prompt generation, API tool + * construction, runtime validation, and preview. The numbered steps below (1-10) + * compute the allowed tool set; step 11 adds the protocol guarantee that re-adds + * `PROTOCOL_TOOLS`. + * + * The returned policy is deterministic for a given input. The only side effect + * is an intentional, process-deduplicated `console.warn` when a protocol tool is + * disabled via `disabledTools` (such a disable is a no-op); it fires at most once + * per tool per process, so repeated calls never re-warn and do not affect output. + * + * @param input Mode, custom modes, MCP hub, disabled tools, model customization, + * experiment flags, todo-list enablement, and the code index manager. + * @returns An {@link EffectiveToolPolicy} describing the effective tool set. + */ +export function resolveEffectiveToolPolicy(input: EffectiveToolPolicyInput): EffectiveToolPolicy { + const { + mode, + customModes, + mcpHub, + disabledTools, + modelInfo, + experiments, + todoListEnabled, + codeIndexManager, + allowedMcpServers, + } = input + + // 1. Resolve mode config with default-slug fallback (existing behavior). + const modeSlug = mode ?? defaultModeSlug + const modeConfig = getModeBySlug(modeSlug, customModes) || getModeBySlug(defaultModeSlug, customModes)! + + // 2. Start from all tools granted by the mode's groups (including always-available tools). + const allowedToolNames = new Set(getToolsForMode(modeConfig.groups)) + + // 3. Filter through per-mode permission checks (feature/experiment flags, custom-mode overrides). + for (const tool of Array.from(allowedToolNames)) { + if (!isToolAllowedForMode(tool, modeSlug, customModes ?? [], undefined, undefined, experiments ?? {})) { + allowedToolNames.delete(tool) + } + } + + // 4. Apply model-specific tool customization (excluded tools removed; included tools added only when their group is allowed). + if (modelInfo) { + // Exclusions. + if (modelInfo.excludedTools?.length) { + for (const excluded of modelInfo.excludedTools) { + allowedToolNames.delete(resolveToolAlias(excluded)) + } + } + // Inclusions: only tools belonging to an allowed group are added. + if (modelInfo.includedTools?.length) { + const toolToGroup = new Map() + for (const [groupName, groupConfig] of Object.entries(TOOL_GROUPS)) { + groupConfig.tools.forEach((tool) => toolToGroup.set(tool, groupName as ToolGroup)) + groupConfig.customTools?.forEach((tool) => toolToGroup.set(tool, groupName as ToolGroup)) + } + + const allowedGroups = new Set( + modeConfig.groups.map((groupEntry: GroupEntry) => + Array.isArray(groupEntry) ? groupEntry[0] : groupEntry, + ), + ) + + for (const included of modelInfo.includedTools) { + const resolvedTool = resolveToolAlias(included) + const toolGroup = toolToGroup.get(resolvedTool) + if (toolGroup && allowedGroups.has(toolGroup)) { + allowedToolNames.add(resolvedTool) + } + } + } + } + + // 5. Drop codebase_search unless the code index is enabled, configured, and initialized. + if ( + !codeIndexManager || + !(codeIndexManager.isFeatureEnabled && codeIndexManager.isFeatureConfigured && codeIndexManager.isInitialized) + ) { + allowedToolNames.delete("codebase_search") + } + + // 6. Drop update_todo_list when the todo list is disabled. + if (todoListEnabled === false) { + allowedToolNames.delete("update_todo_list") + } + + // 7. Drop generate_image unless the image-generation experiment is enabled. + if (experiments?.imageGeneration !== true) { + allowedToolNames.delete("generate_image") + } + + // 8. Drop run_slash_command unless the run-slash-command experiment is enabled. + if (experiments?.runSlashCommand !== true) { + allowedToolNames.delete("run_slash_command") + } + + // 9. Drop disabledTools entries (alias-resolved). + if (disabledTools?.length) { + for (const toolName of disabledTools) { + allowedToolNames.delete(resolveToolAlias(toolName)) + } + } + + // 10. Drop the MCP group tools unless allowed servers actually expose them. + // Fall back to the mode config's own allowlist when the caller omits the + // parameter, so the restriction is enforced regardless of call site + // (defense in depth). `getToolsForMode` grants both group tools together, so + // each is pruned independently: `access_mcp_resource` when no allowed server + // exposes resources, and `use_mcp_tool` when no allowed server exposes a + // prompt-enabled tool (mirrors `getMcpServerTools`, which would emit none). + const effectiveAllowedMcpServers = allowedMcpServers ?? modeConfig.allowedMcpServers + const hasMcpResources = !!mcpHub && hasAnyMcpResources(mcpHub, effectiveAllowedMcpServers) + if (!hasMcpResources) { + allowedToolNames.delete("access_mcp_resource") + } + const hasMcpTools = resolveHasMcpTools(mcpHub, effectiveAllowedMcpServers) + if (!hasMcpTools) { + allowedToolNames.delete("use_mcp_tool") + } + + // 11. Protocol guarantee: re-add every protocol tool so the logical set and + // the runtime validator both agree it is callable even when disabled. + warnProtocolToolOverrides(disabledTools) + for (const tool of PROTOCOL_TOOLS) { + allowedToolNames.add(resolveToolAlias(tool)) + } + + const hasMcpGroup = modeConfig.groups.some((groupEntry: GroupEntry) => getGroupName(groupEntry) === "mcp") + + return { + tools: allowedToolNames, + hasMcpGroup, + hasMcpTools, + hasMcpResources, + editRestriction: getEditRestriction(modeConfig.groups), + } +} + +/** + * Builds the runtime `toolRequirements` map (tool name → false) from a list of + * disabled tool names. Protocol tools and their aliases are intentionally + * skipped so that `attempt_completion` (and every call site that disables it) + * can never be marked un-callable at runtime. + * + * @param disabledTools The raw disabled-tools list (may contain aliases). + * @returns A map of disabled canonical/alias names to `false`. + */ +export function buildToolRequirements(disabledTools?: string[]): Record { + const requirements: Record = {} + if (!disabledTools?.length) { + return requirements + } + for (const toolName of disabledTools) { + const canonical = resolveToolAlias(toolName) + if (PROTOCOL_TOOLS.includes(canonical) || PROTOCOL_TOOLS.includes(toolName)) { + continue + } + requirements[toolName] = false + requirements[canonical] = false + } + return requirements +} diff --git a/src/core/prompts/tools/filter-tools-for-mode.ts b/src/core/prompts/tools/filter-tools-for-mode.ts index 2b31714a4c..02311c5d7f 100644 --- a/src/core/prompts/tools/filter-tools-for-mode.ts +++ b/src/core/prompts/tools/filter-tools-for-mode.ts @@ -1,49 +1,15 @@ import type OpenAI from "openai" -import type { ModeConfig, ToolName, ToolGroup, ModelInfo } from "@roo-code/types" -import { getModeBySlug, getToolsForMode } from "../../../shared/modes" -import { TOOL_GROUPS, ALWAYS_AVAILABLE_TOOLS, TOOL_ALIASES } from "../../../shared/tools" +import type { ModeConfig, ModelInfo } from "@roo-code/types" import { defaultModeSlug } from "../../../shared/modes" import type { CodeIndexManager } from "../../../services/code-index/manager" import type { McpHub } from "../../../services/mcp/McpHub" +import { resolveEffectiveToolPolicy, resolveToolAlias } from "./effective-tool-policy" import { isToolAllowedForMode } from "../../../core/tools/validateToolUse" -/** - * Reverse lookup map - maps alias name to canonical tool name. - * Built once at module load from the central TOOL_ALIASES constant. - */ -const ALIAS_TO_CANONICAL: Map = new Map( - Object.entries(TOOL_ALIASES).map(([alias, canonical]) => [alias, canonical]), -) - -/** - * Canonical to aliases map - maps canonical tool name to array of alias names. - * Built once at module load from the central TOOL_ALIASES constant. - */ -const CANONICAL_TO_ALIASES: Map = new Map() - -// Build the reverse mapping (canonical -> aliases) -for (const [alias, canonical] of Object.entries(TOOL_ALIASES)) { - const existing = CANONICAL_TO_ALIASES.get(canonical) ?? [] - existing.push(alias) - CANONICAL_TO_ALIASES.set(canonical, existing) -} - -/** - * Pre-computed alias groups map - maps any tool name (canonical or alias) to its full group. - * Built once at module load for O(1) lookup. - */ -const ALIAS_GROUPS: Map = new Map() - -// Build alias groups for all tools -for (const [canonical, aliases] of CANONICAL_TO_ALIASES.entries()) { - const group = Object.freeze([canonical, ...aliases]) - // Map canonical to group - ALIAS_GROUPS.set(canonical, group) - // Map each alias to the same group - for (const alias of aliases) { - ALIAS_GROUPS.set(alias, group) - } -} +// Re-export the resolver's alias helper so existing importers of this module +// (NativeToolCallParser, presentAssistantMessage, build-tools) keep binding to the +// single canonical implementation in effective-tool-policy.ts. +export { resolveToolAlias } /** * Cache for renamed tool definitions. @@ -85,130 +51,6 @@ function getOrCreateRenamedTool( return renamedTool } -/** - * Resolves a tool name to its canonical name. - * If the tool name is an alias, returns the canonical tool name. - * If it's already a canonical name or unknown, returns as-is. - * - * @param toolName - The tool name to resolve (may be an alias) - * @returns The canonical tool name - */ -export function resolveToolAlias(toolName: string): string { - const canonical = ALIAS_TO_CANONICAL.get(toolName) - return canonical ?? toolName -} - -/** - * Applies tool alias resolution to a set of allowed tools. - * Resolves any aliases to their canonical tool names. - * - * @param allowedTools - Set of tools that may contain aliases - * @returns Set with aliases resolved to canonical names - */ -export function applyToolAliases(allowedTools: Set): Set { - const result = new Set() - - for (const tool of allowedTools) { - // Resolve alias to canonical name - result.add(resolveToolAlias(tool)) - } - - return result -} - -/** - * Gets all tools in an alias group (including the canonical tool). - * Uses pre-computed ALIAS_GROUPS map for O(1) lookup. - * - * @param toolName - Any tool name in the alias group - * @returns Array of all tool names in the alias group, or just the tool if not aliased - */ -export function getToolAliasGroup(toolName: string): readonly string[] { - return ALIAS_GROUPS.get(toolName) ?? [toolName] -} - -/** - * Apply model-specific tool customization to a set of allowed tools. - * - * This function filters tools based on model configuration: - * 1. Removes tools specified in modelInfo.excludedTools - * 2. Adds tools from modelInfo.includedTools (only if they belong to allowed groups) - * - * @param allowedTools - Set of tools already allowed by mode configuration - * @param modeConfig - Current mode configuration to check tool groups - * @param modelInfo - Model configuration with tool customization - * @returns Modified set of tools after applying model customization - */ -/** - * Result of applying model tool customization. - * Contains the set of allowed tools and any alias renames to apply. - */ -interface ModelToolCustomizationResult { - allowedTools: Set - /** Maps canonical tool name to alias name for tools that should be renamed */ - aliasRenames: Map -} - -export function applyModelToolCustomization( - allowedTools: Set, - modeConfig: ModeConfig, - modelInfo?: ModelInfo, -): ModelToolCustomizationResult { - if (!modelInfo) { - return { allowedTools, aliasRenames: new Map() } - } - - const result = new Set(allowedTools) - const aliasRenames = new Map() - - // Apply excluded tools (remove from allowed set) - if (modelInfo.excludedTools && modelInfo.excludedTools.length > 0) { - modelInfo.excludedTools.forEach((tool) => { - const resolvedTool = resolveToolAlias(tool) - result.delete(resolvedTool) - }) - } - - // Apply included tools (add to allowed set, but only if they belong to an allowed group) - if (modelInfo.includedTools && modelInfo.includedTools.length > 0) { - // Build a map of tool -> group for all tools in TOOL_GROUPS (including customTools) - const toolToGroup = new Map() - for (const [groupName, groupConfig] of Object.entries(TOOL_GROUPS)) { - // Add regular tools - groupConfig.tools.forEach((tool) => { - toolToGroup.set(tool, groupName as ToolGroup) - }) - // Add customTools (opt-in only tools) - if (groupConfig.customTools) { - groupConfig.customTools.forEach((tool) => { - toolToGroup.set(tool, groupName as ToolGroup) - }) - } - } - - // Get the list of allowed groups for this mode - const allowedGroups = new Set( - modeConfig.groups.map((groupEntry) => (Array.isArray(groupEntry) ? groupEntry[0] : groupEntry)), - ) - - // Add included tools only if they belong to an allowed group - // If the tool was specified as an alias, track the rename - modelInfo.includedTools.forEach((tool) => { - const resolvedTool = resolveToolAlias(tool) - const toolGroup = toolToGroup.get(resolvedTool) - if (toolGroup && allowedGroups.has(toolGroup)) { - result.add(resolvedTool) - // If the tool was specified as an alias, rename it in the API - if (tool !== resolvedTool) { - aliasRenames.set(resolvedTool, tool) - } - } - }) - } - - return { allowedTools: result, aliasRenames } -} - /** * Filters native tools based on mode restrictions and model customization. * This ensures native tools are filtered consistently with mode/tool permissions. @@ -235,94 +77,38 @@ export function filterNativeToolsForMode( mcpHub?: McpHub, allowedMcpServers?: string[], ): OpenAI.Chat.ChatCompletionTool[] { - // Get mode configuration and all tools for this mode - const modeSlug = mode ?? defaultModeSlug - let modeConfig = getModeBySlug(modeSlug, customModes) - - // Fallback to default mode if current mode config is not found - // This ensures the agent always has functional tools even if a custom mode is deleted - // or configuration becomes corrupted - if (!modeConfig) { - modeConfig = getModeBySlug(defaultModeSlug, customModes)! - } - - // Get all tools for this mode (including always-available tools) - const allToolsForMode = getToolsForMode(modeConfig.groups) - - // Filter to only tools that pass permission checks - let allowedToolNames = new Set( - allToolsForMode.filter((tool) => - isToolAllowedForMode( - tool as ToolName, - modeSlug, - customModes ?? [], - undefined, - undefined, - experiments ?? {}, - ), - ), - ) - - // Apply model-specific tool customization + // Resolve the single, request-scoped effective tool policy. The filter below + // consumes only its `tools` set (plus alias renames from model customization), + // so prompt generation and API tool construction agree on the logical allowed + // set. attempt_completion is always advertised (the protocol guarantee), even + // if it appears in disabledTools. const modelInfo = settings?.modelInfo as ModelInfo | undefined - const { allowedTools: customizedTools, aliasRenames } = applyModelToolCustomization( - allowedToolNames, - modeConfig, - modelInfo, - ) - allowedToolNames = customizedTools - - // Conditionally exclude codebase_search if feature is disabled or not configured - if ( - !codeIndexManager || - !(codeIndexManager.isFeatureEnabled && codeIndexManager.isFeatureConfigured && codeIndexManager.isInitialized) - ) { - allowedToolNames.delete("codebase_search") - } - - // Conditionally exclude update_todo_list if disabled in settings - if (settings?.todoListEnabled === false) { - allowedToolNames.delete("update_todo_list") - } - - // Conditionally exclude generate_image if experiment is not enabled - if (!experiments?.imageGeneration) { - allowedToolNames.delete("generate_image") - } - - // Conditionally exclude run_slash_command if experiment is not enabled - if (!experiments?.runSlashCommand) { - allowedToolNames.delete("run_slash_command") - } - - // Remove tools that are explicitly disabled via the disabledTools setting - if (settings?.disabledTools?.length) { - for (const toolName of settings.disabledTools) { - // Normalize aliases so disabling a legacy alias (e.g. "search_and_replace") - // also disables the canonical tool (e.g. "edit"). - const resolvedToolName = resolveToolAlias(toolName) - allowedToolNames.delete(resolvedToolName) - } - } - - // Conditionally exclude access_mcp_resource if MCP is not enabled or there are no resources. - // When the mode restricts MCP servers via allowedMcpServers, only resources from allowed - // servers count — otherwise a restricted mode could still read resources from disallowed servers. - // Fall back to the mode config's own allowlist when the caller omits the parameter, so the - // restriction is enforced regardless of call site (defense in depth). - const effectiveAllowedMcpServers = allowedMcpServers ?? modeConfig.allowedMcpServers - if (!mcpHub || !hasAnyMcpResources(mcpHub, effectiveAllowedMcpServers)) { - allowedToolNames.delete("access_mcp_resource") - } - // Filter native tools based on allowed tool names and apply alias renames + const policy = resolveEffectiveToolPolicy({ + mode: mode ?? defaultModeSlug, + customModes, + mcpHub, + disabledTools: settings?.disabledTools, + modelInfo, + experiments, + todoListEnabled: settings?.todoListEnabled, + codeIndexManager, + allowedMcpServers, + }) + + // Apply model-specific alias renames (canonical -> alias) to the allowed set. + // Included-tools customization may rename a tool to the alias the caller asked + // for; excluded/always-available semantics are already resolved by the resolver. + const aliasRenames = resolveModelAliasRenames(modelInfo, policy.tools) + + // Filter native tools based on the allowed tool names and apply alias renames const filteredTools: OpenAI.Chat.ChatCompletionTool[] = [] for (const tool of nativeTools) { // Handle both ChatCompletionTool and ChatCompletionCustomTool if ("function" in tool && tool.function) { const toolName = tool.function.name - if (allowedToolNames.has(toolName)) { + if (policy.tools.has(resolveToolAlias(toolName))) { // Check if this tool should be renamed to an alias const aliasName = aliasRenames.get(toolName) if (aliasName) { @@ -339,107 +125,26 @@ export function filterNativeToolsForMode( } /** - * Helper function to check if any MCP server has resources available. - * - * When `allowedServers` is provided, only servers whose name is in the allowlist are considered. - * This keeps the `access_mcp_resource` availability check consistent with the mode's MCP server - * allowlist so a restricted mode cannot retain the tool based on resources from disallowed servers. + * Computes canonical -> alias renames from model-specific included-tools + * customization, but only for tools that remain in the effective policy's allowed + * set (exclusions are already applied by the resolver). An alias listed in + * includedTools renames the canonical tool to that alias. */ -function hasAnyMcpResources(mcpHub: McpHub, allowedServers?: string[]): boolean { - let servers = mcpHub.getServers() - if (allowedServers) { - const allowSet = new Set(allowedServers) - servers = servers.filter((server) => allowSet.has(server.name)) +function resolveModelAliasRenames( + modelInfo: ModelInfo | undefined, + allowedTools: ReadonlySet, +): Map { + const aliasRenames = new Map() + if (!modelInfo?.includedTools?.length) { + return aliasRenames } - return servers.some((server) => server.resources && server.resources.length > 0) -} - -/** - * Checks if a specific tool is allowed in the current mode. - * This is useful for dynamically filtering system prompt content. - * - * @param toolName - Name of the tool to check - * @param mode - Current mode slug - * @param customModes - Custom mode configurations - * @param experiments - Experiment flags - * @param codeIndexManager - Code index manager for codebase_search feature check - * @param settings - Additional settings for tool filtering - * @returns true if the tool is allowed in the mode, false otherwise - */ -export function isToolAllowedInMode( - toolName: ToolName, - mode: string | undefined, - customModes: ModeConfig[] | undefined, - experiments: Record | undefined, - codeIndexManager?: CodeIndexManager, - settings?: Record, -): boolean { - const modeSlug = mode ?? defaultModeSlug - - // Check if it's an always-available tool - if (ALWAYS_AVAILABLE_TOOLS.includes(toolName)) { - // But still check for conditional exclusions - if (toolName === "codebase_search") { - return !!( - codeIndexManager && - codeIndexManager.isFeatureEnabled && - codeIndexManager.isFeatureConfigured && - codeIndexManager.isInitialized - ) + for (const included of modelInfo.includedTools) { + const canonical = resolveToolAlias(included) + if (canonical !== included && allowedTools.has(canonical)) { + aliasRenames.set(canonical, included) } - if (toolName === "update_todo_list") { - return settings?.todoListEnabled !== false - } - if (toolName === "generate_image") { - return experiments?.imageGeneration === true - } - if (toolName === "run_slash_command") { - return experiments?.runSlashCommand === true - } - return true - } - - // Check if the tool is allowed by the mode's groups - // Resolve to canonical name and check that single value - const canonicalTool = resolveToolAlias(toolName) - return isToolAllowedForMode( - canonicalTool as ToolName, - modeSlug, - customModes ?? [], - undefined, - undefined, - experiments ?? {}, - ) -} - -/** - * Gets the list of available tools from a specific tool group for the current mode. - * This is useful for dynamically building system prompt content based on available tools. - * - * @param groupName - Name of the tool group to check - * @param mode - Current mode slug - * @param customModes - Custom mode configurations - * @param experiments - Experiment flags - * @param codeIndexManager - Code index manager for codebase_search feature check - * @param settings - Additional settings for tool filtering - * @returns Array of tool names that are available from the group - */ -export function getAvailableToolsInGroup( - groupName: ToolGroup, - mode: string | undefined, - customModes: ModeConfig[] | undefined, - experiments: Record | undefined, - codeIndexManager?: CodeIndexManager, - settings?: Record, -): ToolName[] { - const toolGroup = TOOL_GROUPS[groupName] - if (!toolGroup) { - return [] } - - return toolGroup.tools.filter((tool) => - isToolAllowedInMode(tool as ToolName, mode, customModes, experiments, codeIndexManager, settings), - ) as ToolName[] + return aliasRenames } /** diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 4f122feefc..f32fc8a8a8 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -4105,6 +4105,8 @@ export class Task extends EventEmitter implements TaskLike { undefined, // todoList this.api.getModel().id, provider.getSkillsManager(), + state?.disabledTools, + modelInfo, ) })() } diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 0376f437cb..6213f5d376 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -583,6 +583,70 @@ describe("Cline", () => { expect(settings).toMatchObject({ todoListEnabled: true }) }) + it("passes undefined disabledTools when provider state becomes unavailable", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + await task.getTaskMode() + + // First getState call: MCP disabled (avoids the MCP hub path). Later + // calls (including the state read feeding `state?.disabledTools`): + // undefined. Dropping the optional chain on that read makes + // getSystemPrompt reject with a TypeError instead of resolving. + vi.spyOn(mockProvider, "getState") + // ProviderState requires all declared fields; the test deliberately supplies a partial state to exercise the fallback path. + .mockResolvedValueOnce({ mcpEnabled: false } as unknown as ProviderState) + // ProviderState requires all declared fields; the test deliberately supplies an absent state to exercise the fallback path. + .mockResolvedValue(undefined as unknown as ProviderState) + vi.mocked(SYSTEM_PROMPT).mockResolvedValueOnce("mock system prompt") + + await expect(getTaskTestAccess(task).getSystemPrompt()).resolves.toBe("mock system prompt") + + const systemPromptCall = requireDefined(vi.mocked(SYSTEM_PROMPT).mock.calls.at(-1)) + // Argument index 16 is the disabledTools parameter fed by `state?.disabledTools`. + expect(systemPromptCall[16]).toBeUndefined() + }) + + it("forwards non-empty disabledTools and modelInfo to the system prompt call", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + await task.getTaskMode() + + // First getState call: MCP disabled (avoids the MCP hub path). Later + // calls supply the state that feeds `state?.disabledTools`. + vi.spyOn(mockProvider, "getState") + // ProviderState requires all declared fields; the test supplies a partial state. + .mockResolvedValueOnce({ mcpEnabled: false } as unknown as ProviderState) + // ProviderState requires all declared fields; the test supplies a partial state. + .mockResolvedValue({ + mcpEnabled: false, + disabledTools: ["execute_command"], + } as unknown as ProviderState) + + const modelInfo: ModelInfo = { + contextWindow: 128_000, + supportsPromptCache: false, + maxTokens: 1234, + } + vi.spyOn(task.api, "getModel").mockReturnValue({ id: "distinctive-model-id", info: modelInfo }) + vi.mocked(SYSTEM_PROMPT).mockResolvedValueOnce("mock system prompt") + + await expect(getTaskTestAccess(task).getSystemPrompt()).resolves.toBe("mock system prompt") + + const systemPromptCall = requireDefined(vi.mocked(SYSTEM_PROMPT).mock.calls.at(-1)) + // Argument index 16 is the disabledTools parameter fed by `state?.disabledTools`; + // index 17 is the modelInfo from `this.api.getModel().info`. + expect(systemPromptCall[16]).toEqual(["execute_command"]) + expect(systemPromptCall[17]).toBe(modelInfo) + }) + it("uses the task mode when manually condensing after focused state changes", async () => { vi.spyOn(mockProvider, "getState").mockResolvedValue({ mode: "architect", diff --git a/src/core/task/__tests__/build-tools.spec.ts b/src/core/task/__tests__/build-tools.spec.ts new file mode 100644 index 0000000000..f2cc1d8757 --- /dev/null +++ b/src/core/task/__tests__/build-tools.spec.ts @@ -0,0 +1,144 @@ +// npx vitest src/core/task/__tests__/build-tools.spec.ts +// +// Gemini `includeAllToolsWithRestrictions` path: with the flag on, `tools` +// contains ALL declarations while `allowedFunctionNames` is derived from the +// resolver-filtered set — so a disabled `attempt_completion` is still allowed +// (protocol guarantee) and `disabledTools`-removed tools are excluded. + +import type OpenAI from "openai" + +import type { ModeConfig, ModelInfo } from "@roo-code/types" + +import type { ClineProvider } from "../../webview/ClineProvider" +import type { McpHub } from "../../../services/mcp/McpHub" + +vi.mock("../../../services/code-index/manager", () => ({ + CodeIndexManager: { + getInstance: () => ({ isFeatureEnabled: false, isFeatureConfigured: false, isInitialized: false }), + }, +})) + +// Keeps the test independent of the bundled @roo-code/core package; the +// customTools experiment stays off in every case below. +vi.mock("@roo-code/core", () => ({ + customToolRegistry: { + loadFromDirectoriesIfStale: vi.fn(), + getAllSerialized: () => [], + }, + formatNative: vi.fn(), +})) + +import { buildNativeToolsArrayWithRestrictions } from "../build-tools" + +/** + * ClineProvider is a heavy class; build-tools only reads `context` and + * `getMcpHub()` from it, so a minimal object literal stands in. The single + * double assertion in this file. + */ +function makeProvider(): ClineProvider { + const provider = { + context: { extensionPath: "/mock", globalStoragePath: "/mock", storagePath: "/mock", logPath: "/mock" }, + getMcpHub: () => ({ getServers: () => [] }) as unknown as McpHub, + } + return provider as unknown as ClineProvider +} + +function toolNames(tools: OpenAI.Chat.ChatCompletionTool[]): string[] { + return tools + .filter((t): t is OpenAI.Chat.ChatCompletionFunctionTool => "function" in t && Boolean(t.function)) + .map((t) => t.function.name) +} + +describe("buildNativeToolsArrayWithRestrictions — Gemini includeAllToolsWithRestrictions", () => { + const provider = makeProvider() + + it("sends all declarations but restricts allowedFunctionNames (protocol tool stays allowed)", async () => { + const result = await buildNativeToolsArrayWithRestrictions({ + provider, + cwd: "/test/path", + mode: "code", + customModes: undefined, + experiments: {}, + apiConfiguration: undefined, + disabledTools: ["execute_command", "attempt_completion"], + includeAllToolsWithRestrictions: true, + }) + + // All tools are still advertised (declarations), including the two + // disabled ones. + expect(toolNames(result.tools)).toContain("execute_command") + expect(toolNames(result.tools)).toContain("attempt_completion") + + // But the logical set (allowedFunctionNames) honors the policy: + // attempt_completion is a protocol tool and stays allowed even though + // disabledTools lists it; execute_command is removed. + expect(result.allowedFunctionNames).toContain("attempt_completion") + expect(result.allowedFunctionNames).not.toContain("execute_command") + }) + + it("flows mode filtering through the resolver into allowedFunctionNames", async () => { + const customModes: ModeConfig[] = [ + { + slug: "arch", + name: "Architect-ish", + roleDefinition: "", + groups: ["read", ["edit", { fileRegex: "\\.md$" }]], + }, + ] + + const result = await buildNativeToolsArrayWithRestrictions({ + provider, + cwd: "/test/path", + mode: "arch", + customModes, + experiments: {}, + apiConfiguration: undefined, + includeAllToolsWithRestrictions: true, + }) + + // The mode's groups do not include "command", so execute_command is not + // in the logical set even though it is advertised in tools. + expect(toolNames(result.tools)).toContain("execute_command") + expect(result.allowedFunctionNames).not.toContain("execute_command") + // Anchor: the mode's read group is still allowed, so the list is populated. + expect(result.allowedFunctionNames).toContain("read_file") + }) + + it("default path (flag omitted) omits disabled tools from the sent declarations", async () => { + const result = await buildNativeToolsArrayWithRestrictions({ + provider, + cwd: "/test/path", + mode: "code", + customModes: undefined, + experiments: {}, + apiConfiguration: undefined, + disabledTools: ["execute_command"], + }) + + // Non-Gemini path: disabled tools are not sent at all. + expect(toolNames(result.tools)).not.toContain("execute_command") + expect(result.allowedFunctionNames).toBeUndefined() + }) + + it("excludes modelInfo.excludedTools from allowedFunctionNames", async () => { + const modelInfo: ModelInfo = { + contextWindow: 100_000, + supportsPromptCache: true, + excludedTools: ["read_file"], + } + + const result = await buildNativeToolsArrayWithRestrictions({ + provider, + cwd: "/test/path", + mode: "code", + customModes: undefined, + experiments: {}, + apiConfiguration: undefined, + modelInfo, + includeAllToolsWithRestrictions: true, + }) + + expect(result.allowedFunctionNames).not.toContain("read_file") + expect(result.allowedFunctionNames).toContain("attempt_completion") + }) +}) diff --git a/src/core/task/build-tools.ts b/src/core/task/build-tools.ts index ebbdc050dc..8e1015e360 100644 --- a/src/core/task/build-tools.ts +++ b/src/core/task/build-tools.ts @@ -51,6 +51,9 @@ interface BuildToolsResult { /** * Extracts the function name from a tool definition. + * + * @param tool A chat-completion tool definition (function tool in practice). + * @returns The tool's function name. */ function getToolName(tool: OpenAI.Chat.ChatCompletionTool): string { return (tool as OpenAI.Chat.ChatCompletionFunctionTool).function.name diff --git a/src/core/webview/__tests__/generateSystemPrompt.spec.ts b/src/core/webview/__tests__/generateSystemPrompt.spec.ts new file mode 100644 index 0000000000..384e564f8c --- /dev/null +++ b/src/core/webview/__tests__/generateSystemPrompt.spec.ts @@ -0,0 +1,558 @@ +// npx vitest src/core/webview/__tests__/generateSystemPrompt.spec.ts +// +// Preview parity: generateSystemPrompt (the webview preview path) must produce +// the same CAPABILITIES / RULES / SYSTEM INFORMATION sections as a direct +// SYSTEM_PROMPT call built from the *same* inputs — including a full ModelInfo, +// so model-level excludedTools/includedTools are honored in the preview exactly +// like the runtime path. The old `{ isStealthModel }`-only typing silently +// allowed the preview to ignore them. + +vi.mock("os", () => ({ + default: { + homedir: () => "/home/user", + platform: () => "linux", + arch: () => "x64", + type: () => "Linux", + release: () => "5.4.0", + hostname: () => "test-host", + tmpdir: () => "/tmp", + endianness: () => "LE", + loadavg: () => [0, 0, 0], + totalmem: () => 8589934592, + freemem: () => 4294967296, + cpus: () => [], + networkInterfaces: () => ({}), + userInfo: () => ({ username: "test", uid: 1000, gid: 1000, shell: "/bin/bash", homedir: "/home/user" }), + }, + homedir: () => "/home/user", + platform: () => "linux", + arch: () => "x64", + type: () => "Linux", + release: () => "5.4.0", + hostname: () => "test-host", + tmpdir: () => "/tmp", + endianness: () => "LE", + loadavg: () => [0, 0, 0], + totalmem: () => 8589934592, + freemem: () => 4294967296, + cpus: () => [], + networkInterfaces: () => ({}), + userInfo: () => ({ username: "test", uid: 1000, gid: 1000, shell: "/bin/bash", homedir: "/home/user" }), +})) + +vi.mock("os-name", () => ({ + default: () => "Linux", +})) + +vi.mock("fs/promises") + +import * as vscode from "vscode" + +import type { ModelInfo } from "@roo-code/types" +import { providerIdentifiers } from "@roo-code/types/provider-identifiers" + +import { SYSTEM_PROMPT } from "../../prompts/system" +import { getCapabilitiesSection } from "../../prompts/sections/capabilities" +import { getRulesSection } from "../../prompts/sections/rules" +import type { EffectiveToolPolicy } from "../../prompts/tools/effective-tool-policy" +import { generateSystemPrompt } from "../generateSystemPrompt" +import type { ClineProvider } from "../ClineProvider" +import "../../../utils/path" + +// Mock vscode — generateSystemPrompt reads env.language and workspace config. +vi.mock("vscode", () => ({ + env: { + language: "en", + }, + workspace: { + workspaceFolders: [{ uri: { fsPath: "/test/path" } }], + getConfiguration: vi.fn().mockReturnValue({ + get: vi.fn().mockReturnValue(undefined), + }), + getWorkspaceFolder: vi.fn().mockReturnValue({ uri: { fsPath: "/test/path" } }), + }, + window: { + activeTextEditor: undefined, + }, + EventEmitter: vi.fn().mockImplementation(function () { + return { + event: vi.fn(), + fire: vi.fn(), + dispose: vi.fn(), + } + }), +})) + +// Mutable shell mock: getShell feeds the command-chaining text in RULES, so the +// fragment-gating describe below can retarget the shell per test without +// re-registering the module mock (which would leak across the parity tests). +const shellMock = vi.hoisted(() => ({ shell: "/bin/zsh" })) + +vi.mock("../../../utils/shell", () => ({ + getShell: () => shellMock.shell, +})) + +// Mock the section builders that touch the filesystem / extension context so the +// parity comparison is stable and independent of workspace state. +vi.mock("../../prompts/sections/modes", () => ({ + getModesSection: vi.fn().mockImplementation(async () => `====\n\nMODES\n\n- Test modes section`), +})) + +vi.mock("../../prompts/sections/custom-instructions", () => ({ + addCustomInstructions: vi.fn().mockImplementation(async () => ""), +})) + +// The preview must consume a *complete* ModelInfo from the API handler. This +// locks in that contract: if generateSystemPrompt ever narrows the local +// modelInfo back down, the excludedTools sub-assertion below fails. +const fullModelInfo: ModelInfo = { + contextWindow: 100_000, + supportsPromptCache: true, + excludedTools: ["read_file"], +} + +// Note: the module under test imports `../../api` from src/core/webview, which +// resolves to src/api — from this spec's directory (one level deeper) that is +// `../../../api`. +vi.mock("../../../api", () => ({ + buildApiHandler: () => ({ + getModel: () => ({ id: "m", info: fullModelInfo }), + }), +})) + +// Minimal mock ExtensionContext, mirroring the pattern in system-prompt.spec.ts. +const mockContext = { + extensionPath: "/mock/extension/path", + globalStoragePath: "/mock/storage/path", + storagePath: "/mock/storage/path", + logPath: "/mock/log/path", + subscriptions: [], + workspaceState: { + get: () => undefined, + update: () => Promise.resolve(), + }, + globalState: { + get: () => undefined, + update: () => Promise.resolve(), + setKeysForSync: () => {}, + }, + extensionUri: { fsPath: "/mock/extension/path" }, + globalStorageUri: { fsPath: "/mock/settings/path" }, + asAbsolutePath: (relativePath: string) => `/mock/extension/path/${relativePath}`, + extension: { + packageJSON: { + version: "1.0.0", + }, + }, +} as unknown as vscode.ExtensionContext + +const fullSettings = { + todoListEnabled: true, + useAgentRules: true, + newTaskRequireTodos: false, +} + +describe("generateSystemPrompt preview parity", () => { + // Section-scoped extraction: capture the text between two "====" headers so + // the comparison is limited to the sections the tool policy drives. + function extractSection(prompt: string, header: string): string { + const marker = `\n\n${header}\n\n` + const idx = prompt.indexOf(marker) + expect(idx).toBeGreaterThan(-1) + const afterHeader = prompt.slice(idx + marker.length) + const nextMarker = afterHeader.indexOf("\n\n====") + return nextMarker === -1 ? afterHeader : afterHeader.slice(0, nextMarker) + } + + /** + * ClineProvider is a heavy class; the preview only touches these members, so + * a minimal object literal stands in for it. This is the single double + * assertion in this spec. + */ + // The preview only destructures a handful of getState() fields, so the mock + // returns that subset instead of a full ExtensionState; keeping the raw + // vi.fn() (rather than vi.mocked) avoids casting the partial doubles. + const getStateMock = vi.fn().mockResolvedValue({ + apiConfiguration: { apiProvider: providerIdentifiers.openai, apiModelId: "gpt-4o" }, + customModePrompts: undefined, + customInstructions: undefined, + mcpEnabled: false, + experiments: {}, + language: undefined, + enableSubfolderRules: false, + disabledTools: undefined, + }) + + const fakeProvider = { + context: mockContext, + cwd: "/test/path", + getState: getStateMock, + getMcpHub: vi.fn(), + getCurrentTask: vi.fn().mockReturnValue(undefined), + getSkillsManager: vi.fn().mockReturnValue(undefined), + customModesManager: { + getCustomModes: vi.fn().mockResolvedValue([]), + }, + } as unknown as ClineProvider + + it("produces identical CAPABILITIES, RULES, and SYSTEM INFORMATION sections for the same inputs", async () => { + const preview = await generateSystemPrompt(fakeProvider, { type: "mode", mode: "code" }) + + // The direct SYSTEM_PROMPT call uses exactly the inputs the webview path + // builds: same disabledTools (undefined), same full modelInfo, same + // settings shape. + const direct = await SYSTEM_PROMPT( + mockContext, + "/test/path", + false, + undefined, // mcpHub + undefined, // diffStrategy + "code", + undefined, // customModePrompts + undefined, // customModes + undefined, // globalCustomInstructions + {}, // experiments + undefined, // language + undefined, // rooIgnoreInstructions + fullSettings, // settings + undefined, // todoList + undefined, // modelId + undefined, // skillsManager + undefined, // disabledTools + fullModelInfo, // modelInfo + ) + + for (const header of ["CAPABILITIES", "RULES", "SYSTEM INFORMATION"]) { + expect(extractSection(preview, header)).toEqual(extractSection(direct, header)) + } + }) + + it("honors the full modelInfo.excludedTools in the preview output", async () => { + const preview = await generateSystemPrompt(fakeProvider, { type: "mode", mode: "code" }) + const capabilities = extractSection(preview, "CAPABILITIES") + + // read_file is excluded by the model info: no "read files" clause. + expect(capabilities).not.toContain("read files") + // Other clauses survive, proving the exclusion is scoped to that tool. + expect(capabilities).toContain("execute CLI commands") + }) + + it("omits command guidance from the preview when execute_command is disabled", async () => { + // The preview must forward state.disabledTools to SYSTEM_PROMPT: with + // execute_command disabled, the CAPABILITIES section drops every + // command-related fragment. The once-value overrides the shared default + // without mutating it for other tests. + getStateMock.mockResolvedValueOnce({ + apiConfiguration: { apiProvider: providerIdentifiers.openai, apiModelId: "gpt-4o" }, + customModePrompts: undefined, + customInstructions: undefined, + mcpEnabled: false, + experiments: {}, + language: undefined, + enableSubfolderRules: false, + disabledTools: ["execute_command"], + }) + + const preview = await generateSystemPrompt(fakeProvider, { type: "mode", mode: "code" }) + const capabilities = extractSection(preview, "CAPABILITIES") + + expect(capabilities).not.toContain("execute CLI commands") + expect(capabilities).not.toContain("You can use the execute_command tool") + // Anchor: the section is still populated, proving only execute_command + // guidance was removed. + expect(capabilities).toContain("list files") + }) + + it("resolves when settings are omitted instead of dereferencing them", async () => { + // generatePrompt reads `settings?.todoListEnabled`; without the optional + // chain this call rejects with a TypeError on the undefined settings object. + const prompt = await SYSTEM_PROMPT( + mockContext, + "/test/path", + false, + undefined, // mcpHub + undefined, // diffStrategy + "code", + undefined, // customModePrompts + undefined, // customModes + undefined, // globalCustomInstructions + {}, // experiments + undefined, // language + undefined, // rooIgnoreInstructions + undefined, // settings -> exercises the `settings?.` optional chain + ) + + expect(prompt).toContain("OBJECTIVE") + }) +}) + +// --------------------------------------------------------------------------- +// Mutation coverage for the CAPABILITIES and RULES fragment builders. These +// sections have no name-matched spec file, so this spec — the gate's direct +// test file for the prompt pipeline — drives every fragment gate, fallback +// sentence, and MCP-availability branch directly against the section builders. +// --------------------------------------------------------------------------- +describe("getCapabilitiesSection / getRulesSection fragment gating", () => { + const cwd = "/test/path" + const settings = { ...fullSettings } + + /** + * Raw policy double: the section builders only read `tools` plus the MCP and + * edit-restriction fields, so a literal captures every branch the resolver + * could produce for these two sections. + */ + function sectionPolicy( + tools: string[], + extra: Partial< + Pick + > = {}, + ): EffectiveToolPolicy { + return { + tools: new Set(tools), + hasMcpGroup: false, + hasMcpTools: false, + hasMcpResources: false, + ...extra, + } + } + + afterEach(() => { + shellMock.shell = "/bin/zsh" + }) + + describe("getCapabilitiesSection", () => { + it("emits every clause and paragraph when all capability tools are advertised", () => { + const result = getCapabilitiesSection( + sectionPolicy( + [ + "execute_command", + "list_files", + "codebase_search", + "search_files", + "read_file", + "write_to_file", + "apply_diff", + ], + { hasMcpGroup: true, hasMcpTools: true }, + ), + ) + + expect(result).toContain("====\n\nCAPABILITIES\n\n") + expect(result).toContain( + "You have access to tools that let you execute CLI commands on the user's computer, list files, view source code definitions, regex search, read files, write and edit files.", + ) + expect(result).toContain("\n- These tools help you accomplish tasks.\n") + expect(result).toContain("you can use the list_files tool") + expect(result).toContain("You can use the execute_command tool to run commands on the user's computer") + expect(result).toContain( + "You have access to MCP servers that may provide additional tools and/or resources", + ) + expect(result).not.toContain("Stryker was here") + // The trailing newline is trimmed; the result must end with the last bullet. + expect(result.endsWith("accomplish tasks more effectively.")).toBe(true) + }) + + it("falls back to the limited-tools sentence and omits every fragment when no capability tools are advertised", () => { + const result = getCapabilitiesSection(sectionPolicy([])) + + expect(result).toContain( + "You have access to a limited set of tools for this mode; only the tools you are provided may be called.", + ) + expect(result).not.toContain("You have access to tools that let you") + expect(result).not.toContain("execute CLI commands") + expect(result).not.toContain("list files") + expect(result).not.toContain("view source code definitions") + expect(result).not.toContain("regex search") + expect(result).not.toContain("read files") + expect(result).not.toContain("write and edit files") + expect(result).not.toContain("you can use the list_files tool") + expect(result).not.toContain("You can use the execute_command tool") + expect(result).not.toContain("MCP servers") + }) + + it("gates each clause on exactly its advertised tool", () => { + expect(getCapabilitiesSection(sectionPolicy(["list_files"]))).toContain( + "You have access to tools that let you list files.", + ) + expect(getCapabilitiesSection(sectionPolicy(["codebase_search"]))).toContain( + "You have access to tools that let you view source code definitions.", + ) + expect(getCapabilitiesSection(sectionPolicy(["search_files"]))).toContain( + "You have access to tools that let you regex search.", + ) + expect(getCapabilitiesSection(sectionPolicy(["search_files"]))).not.toContain( + "view source code definitions", + ) + expect(getCapabilitiesSection(sectionPolicy(["read_file"]))).toContain( + "You have access to tools that let you read files.", + ) + expect(getCapabilitiesSection(sectionPolicy(["write_to_file"]))).toContain("write and edit files") + expect(getCapabilitiesSection(sectionPolicy(["apply_diff"]))).toContain("write and edit files") + expect(getCapabilitiesSection(sectionPolicy(["read_file"]))).not.toContain("write and edit files") + }) + + it("binds the edit-restriction suffix with and without a description", () => { + const withDescription = getCapabilitiesSection( + sectionPolicy(["read_file"], { + editRestriction: { fileRegex: "\\.md$", description: "Markdown files only" }, + }), + ) + expect(withDescription).toContain( + "(in this mode only files matching '\\.md$' can be edited — Markdown files only)", + ) + + const withoutDescription = getCapabilitiesSection( + sectionPolicy(["read_file"], { editRestriction: { fileRegex: "\\.md$" } }), + ) + // "Stryker was here" (no trailing !) covers both the StringLiteral and + // ArrayDeclaration sentinel replacements Stryker injects. + expect(withoutDescription).toContain("(in this mode only files matching '\\.md$' can be edited)") + expect(withoutDescription).not.toContain("Stryker was here") + + const unrestricted = getCapabilitiesSection(sectionPolicy(["read_file"])) + expect(unrestricted).not.toContain("(in this mode only files matching") + expect(unrestricted).not.toContain("Stryker was here") + }) + + it("emits the MCP bullet only when the mcp group is present and tools or resources are effective", () => { + const mcpBullet = "You have access to MCP servers that may provide additional tools" + + // group + effective tools, and group + effective resources -> present + expect(getCapabilitiesSection(sectionPolicy([], { hasMcpGroup: true, hasMcpTools: true }))).toContain( + mcpBullet, + ) + expect(getCapabilitiesSection(sectionPolicy([], { hasMcpGroup: true, hasMcpResources: true }))).toContain( + mcpBullet, + ) + // group but nothing effective -> absent + expect(getCapabilitiesSection(sectionPolicy([], { hasMcpGroup: true }))).not.toContain(mcpBullet) + // effective tools/resources but no group -> absent + expect( + getCapabilitiesSection(sectionPolicy([], { hasMcpTools: true, hasMcpResources: true })), + ).not.toContain(mcpBullet) + }) + }) + + describe("getRulesSection", () => { + it("includes every tool-gated fragment when all relevant tools are advertised", () => { + const result = getRulesSection( + cwd, + settings, + sectionPolicy(["execute_command", "ask_followup_question", "list_files", "read_file"]), + ) + + expect(result).toContain("====\n\nRULES\n\n- ") + expect(result).toContain("The project base directory is: /test/path") + expect(result).toContain( + "All file paths must be relative to this directory. However, commands may change directories in terminals, so respect working directory specified by the response to execute_command.", + ) + expect(result).toContain("You are stuck operating from '/test/path'") + expect(result).toContain("Do not use the ~ character or $HOME to refer to the home directory.") + expect(result).toContain( + "Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context", + ) + expect(result).toContain("Some modes have restrictions on which files they can edit") + expect(result).toContain("Be sure to consider the type of project") + expect(result).toContain("When making changes to code, always consider the context") + expect(result).toContain("Do not ask for more information than necessary") + expect(result).toContain( + "You are only allowed to ask the user questions using the ask_followup_question tool", + ) + expect(result).toContain("you should use the list_files tool to list the files in the Desktop") + expect(result).not.toContain("Provide your best-effort result") + expect(result).toContain("When executing commands, if you don't see the expected output") + expect(result).toContain( + "use the ask_followup_question tool to request the user to copy and paste it back to you", + ) + expect(result).not.toContain("note what you expected and proceed with the task") + expect(result).toContain("The user may provide a file's contents directly") + expect(result).toContain( + "Your goal is to try to accomplish the user's task, NOT engage in a back and forth conversation.", + ) + expect(result).toContain("NEVER end attempt_completion result with a question") + expect(result).toContain("STRICTLY FORBIDDEN from starting your messages") + expect(result).toContain("When presented with images, utilize your vision capabilities") + expect(result).toContain("you will automatically receive environment_details") + expect(result).toContain('"Actively Running Terminals"') + expect(result).toContain("It is critical you wait for the user's response after each tool use") + expect(result).not.toContain("MCP operations should be used one at a time") + expect(result).not.toContain("VENDOR CONFIDENTIALITY") + // join separator: rules are bulleted one per line, not concatenated + expect(result).toContain("/test/path\n- All file paths must be relative") + expect(result).not.toContain("Stryker was here") + }) + + it("uses the fallback fragments when execute_command, ask_followup_question, and read_file are absent", () => { + const result = getRulesSection(cwd, settings, sectionPolicy([])) + + expect(result).toContain("- All file paths must be relative to this directory.\n") + expect(result).not.toContain("However, commands may change directories in terminals") + expect(result).not.toContain("Before using the execute_command tool") + expect(result).toContain("Provide your best-effort result and state your assumptions") + expect(result).not.toContain("You are only allowed to ask the user questions") + expect(result).not.toContain("When executing commands") + expect(result).not.toContain("The user may provide a file's contents directly") + expect(result).not.toContain("Actively Running Terminals") + }) + + it("keeps the ask guidance but drops the list_files example when only ask_followup_question is advertised", () => { + const result = getRulesSection(cwd, settings, sectionPolicy(["ask_followup_question"])) + + expect(result).toContain( + "You are only allowed to ask the user questions using the ask_followup_question tool", + ) + expect(result).not.toContain("the list_files tool") + expect(result).not.toContain("Stryker was here!") + }) + + it("uses the fallback phrasing in the terminal-output rule when ask_followup_question is absent", () => { + const result = getRulesSection(cwd, settings, sectionPolicy(["execute_command"])) + + expect(result).toContain("When executing commands, if you don't see the expected output") + expect(result).toContain("note what you expected and proceed with the task, stating your assumptions") + expect(result).not.toContain("use the ask_followup_question tool to request") + }) + + it("emits the MCP usage rule only when the mcp group is present and tools or resources are effective", () => { + const mcpRule = "MCP operations should be used one at a time" + + expect( + getRulesSection(cwd, settings, sectionPolicy([], { hasMcpGroup: true, hasMcpTools: true })), + ).toContain(mcpRule) + expect( + getRulesSection(cwd, settings, sectionPolicy([], { hasMcpGroup: true, hasMcpResources: true })), + ).toContain(mcpRule) + expect(getRulesSection(cwd, settings, sectionPolicy([], { hasMcpGroup: true }))).not.toContain(mcpRule) + expect( + getRulesSection(cwd, settings, sectionPolicy([], { hasMcpTools: true, hasMcpResources: true })), + ).not.toContain(mcpRule) + }) + + it("tolerates undefined settings and emits vendor confidentiality only for stealth models", () => { + const full = sectionPolicy(["execute_command", "ask_followup_question", "list_files", "read_file"]) + + // The `settings?.isStealthModel` optional chain must survive an undefined settings + // object; dropping the chain throws a TypeError inside getRulesSection. + expect(() => getRulesSection(cwd, undefined, full)).not.toThrow() + expect(getRulesSection(cwd, undefined, full)).not.toContain("VENDOR CONFIDENTIALITY") + expect(getRulesSection(cwd, { ...settings, isStealthModel: true }, full)).toContain( + "VENDOR CONFIDENTIALITY", + ) + }) + + it("appends the PowerShell chain note and omits it for Unix shells", () => { + const full = sectionPolicy(["execute_command"]) + + shellMock.shell = "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" + const powershell = getRulesSection(cwd, settings, full) + expect(powershell).toContain("cd (path to project) ; (command, in this case npm install)") + expect(powershell).toContain(" Note: Using `;` for PowerShell command chaining") + + shellMock.shell = "/bin/bash" + const unix = getRulesSection(cwd, settings, full) + expect(unix).toContain("cd (path to project) && (command, in this case npm install)") + expect(unix).not.toContain("Note: Using") + expect(unix).not.toContain("Stryker was here") + }) + }) +}) diff --git a/src/core/webview/generateSystemPrompt.ts b/src/core/webview/generateSystemPrompt.ts index 8af2f5ff5d..27457c1c7f 100644 --- a/src/core/webview/generateSystemPrompt.ts +++ b/src/core/webview/generateSystemPrompt.ts @@ -1,4 +1,5 @@ import * as vscode from "vscode" +import type { ModelInfo } from "@roo-code/types" import { WebviewMessage } from "../../shared/WebviewMessage" import { defaultModeSlug } from "../../shared/modes" import { buildApiHandler } from "../../api" @@ -18,6 +19,7 @@ export const generateSystemPrompt = async (provider: ClineProvider, message: Web experiments, language, enableSubfolderRules, + disabledTools, } = await provider.getState() const diffStrategy = new MultiSearchReplaceDiffStrategy() @@ -29,9 +31,11 @@ export const generateSystemPrompt = async (provider: ClineProvider, message: Web const rooIgnoreInstructions = provider.getCurrentTask()?.rooIgnoreController?.getInstructions() - // Create a temporary API handler to check model info for stealth mode. + // Create a temporary API handler to fetch the full model info for the preview. // This avoids relying on an active Cline instance which might not exist during preview. - let modelInfo: { isStealthModel?: boolean } | undefined + // The full ModelInfo flows into SYSTEM_PROMPT so the preview honors + // excludedTools/includedTools exactly like the runtime path. + let modelInfo: ModelInfo | undefined try { const tempApiHandler = buildApiHandler(apiConfiguration) modelInfo = tempApiHandler.getModel().info @@ -64,6 +68,8 @@ export const generateSystemPrompt = async (provider: ClineProvider, message: Web undefined, // todoList undefined, // modelId provider.getSkillsManager(), + disabledTools, + modelInfo, ) return systemPrompt diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index 393e108645..60c7586e00 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -761,7 +761,7 @@ }, "core/prompts/tools/filter-tools-for-mode.ts": { "@typescript-eslint/no-explicit-any": { - "count": 3 + "count": 1 } }, "core/prompts/tools/native-tools/__tests__/converters.spec.ts": {