Skip to content

Commit e8ac3bf

Browse files
authored
Gate XML out when native tool protocol is ON (#9107)
1 parent fefec6c commit e8ac3bf

File tree

14 files changed

+358
-38
lines changed

14 files changed

+358
-38
lines changed

packages/types/src/tool.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,17 @@ export const toolUsageSchema = z.record(
5454
)
5555

5656
export type ToolUsage = z.infer<typeof toolUsageSchema>
57+
58+
/**
59+
* Tool protocol constants
60+
*/
61+
export const TOOL_PROTOCOL = {
62+
XML: "xml",
63+
NATIVE: "native",
64+
} as const
65+
66+
/**
67+
* Tool protocol type for system prompt generation
68+
* Derived from TOOL_PROTOCOL constants to ensure type safety
69+
*/
70+
export type ToolProtocol = (typeof TOOL_PROTOCOL)[keyof typeof TOOL_PROTOCOL]

src/core/prompts/__tests__/system-prompt.spec.ts

Lines changed: 178 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,11 @@ __setMockImplementation(
112112
}
113113

114114
const joinedSections = sections.join("\n\n")
115+
const effectiveProtocol = options?.settings?.toolProtocol || "xml"
116+
const skipXmlReferences = effectiveProtocol === "native"
117+
const toolUseRef = skipXmlReferences ? "." : " without interfering with the TOOL USE guidelines."
115118
return joinedSections
116-
? `\n====\n\nUSER'S CUSTOM INSTRUCTIONS\n\nThe following additional instructions are provided by the user, and should be followed to the best of your ability without interfering with the TOOL USE guidelines.\n\n${joinedSections}`
119+
? `\n====\n\nUSER'S CUSTOM INSTRUCTIONS\n\nThe following additional instructions are provided by the user, and should be followed to the best of your ability${toolUseRef}\n\n${joinedSections}`
117120
: ""
118121
},
119122
)
@@ -581,6 +584,7 @@ describe("SYSTEM_PROMPT", () => {
581584
todoListEnabled: false,
582585
useAgentRules: true,
583586
newTaskRequireTodos: false,
587+
toolProtocol: "xml" as const,
584588
}
585589

586590
const prompt = await SYSTEM_PROMPT(
@@ -614,6 +618,7 @@ describe("SYSTEM_PROMPT", () => {
614618
todoListEnabled: true,
615619
useAgentRules: true,
616620
newTaskRequireTodos: false,
621+
toolProtocol: "xml" as const,
617622
}
618623

619624
const prompt = await SYSTEM_PROMPT(
@@ -646,6 +651,7 @@ describe("SYSTEM_PROMPT", () => {
646651
todoListEnabled: true,
647652
useAgentRules: true,
648653
newTaskRequireTodos: false,
654+
toolProtocol: "xml" as const,
649655
}
650656

651657
const prompt = await SYSTEM_PROMPT(
@@ -672,6 +678,177 @@ describe("SYSTEM_PROMPT", () => {
672678
expect(prompt).toContain("## update_todo_list")
673679
})
674680

681+
it("should include XML tool instructions when disableXmlToolInstructions is false (default)", async () => {
682+
const settings = {
683+
maxConcurrentFileReads: 5,
684+
todoListEnabled: true,
685+
useAgentRules: true,
686+
newTaskRequireTodos: false,
687+
toolProtocol: "xml" as const, // explicitly xml
688+
}
689+
690+
const prompt = await SYSTEM_PROMPT(
691+
mockContext,
692+
"/test/path",
693+
false,
694+
undefined, // mcpHub
695+
undefined, // diffStrategy
696+
undefined, // browserViewportSize
697+
defaultModeSlug, // mode
698+
undefined, // customModePrompts
699+
undefined, // customModes
700+
undefined, // globalCustomInstructions
701+
undefined, // diffEnabled
702+
experiments,
703+
true, // enableMcpServerCreation
704+
undefined, // language
705+
undefined, // rooIgnoreInstructions
706+
undefined, // partialReadsEnabled
707+
settings, // settings
708+
)
709+
710+
// Should contain XML guidance sections
711+
expect(prompt).toContain("TOOL USE")
712+
expect(prompt).toContain("XML-style tags")
713+
expect(prompt).toContain("<actual_tool_name>")
714+
expect(prompt).toContain("</actual_tool_name>")
715+
expect(prompt).toContain("Tool Use Guidelines")
716+
expect(prompt).toContain("# Tools")
717+
718+
// Should contain tool descriptions with XML examples
719+
expect(prompt).toContain("## read_file")
720+
expect(prompt).toContain("<read_file>")
721+
expect(prompt).toContain("<path>")
722+
723+
// Should be byte-for-byte compatible with default behavior
724+
const defaultPrompt = await SYSTEM_PROMPT(
725+
mockContext,
726+
"/test/path",
727+
false,
728+
undefined,
729+
undefined,
730+
undefined,
731+
defaultModeSlug,
732+
undefined,
733+
undefined,
734+
undefined,
735+
undefined,
736+
experiments,
737+
true,
738+
undefined,
739+
undefined,
740+
undefined,
741+
{
742+
maxConcurrentFileReads: 5,
743+
todoListEnabled: true,
744+
useAgentRules: true,
745+
newTaskRequireTodos: false,
746+
toolProtocol: "xml" as const,
747+
},
748+
)
749+
750+
expect(prompt).toBe(defaultPrompt)
751+
})
752+
753+
it("should include native tool instructions when toolProtocol is native", async () => {
754+
const settings = {
755+
maxConcurrentFileReads: 5,
756+
todoListEnabled: true,
757+
useAgentRules: true,
758+
newTaskRequireTodos: false,
759+
toolProtocol: "native" as const, // native protocol
760+
}
761+
762+
const prompt = await SYSTEM_PROMPT(
763+
mockContext,
764+
"/test/path",
765+
false,
766+
undefined, // mcpHub
767+
undefined, // diffStrategy
768+
undefined, // browserViewportSize
769+
defaultModeSlug, // mode
770+
undefined, // customModePrompts
771+
undefined, // customModes
772+
undefined, // globalCustomInstructions
773+
undefined, // diffEnabled
774+
experiments,
775+
true, // enableMcpServerCreation
776+
undefined, // language
777+
undefined, // rooIgnoreInstructions
778+
undefined, // partialReadsEnabled
779+
settings, // settings
780+
)
781+
782+
// Should contain TOOL USE section with native note
783+
expect(prompt).toContain("TOOL USE")
784+
expect(prompt).toContain("provider-native tool-calling mechanism")
785+
expect(prompt).toContain("Do not include XML markup or examples")
786+
787+
// Should NOT contain XML-style tags or examples
788+
expect(prompt).not.toContain("XML-style tags")
789+
expect(prompt).not.toContain("<actual_tool_name>")
790+
expect(prompt).not.toContain("</actual_tool_name>")
791+
792+
// Should contain Tool Use Guidelines section without format-specific guidance
793+
expect(prompt).toContain("Tool Use Guidelines")
794+
// Should NOT contain any protocol-specific formatting instructions
795+
expect(prompt).not.toContain("provider's native tool-calling mechanism")
796+
expect(prompt).not.toContain("XML format specified for each tool")
797+
798+
// Should NOT contain # Tools catalog at all in native mode
799+
expect(prompt).not.toContain("# Tools")
800+
expect(prompt).not.toContain("## read_file")
801+
expect(prompt).not.toContain("## execute_command")
802+
expect(prompt).not.toContain("<read_file>")
803+
expect(prompt).not.toContain("<path>")
804+
expect(prompt).not.toContain("Usage:")
805+
expect(prompt).not.toContain("Examples:")
806+
807+
// Should still contain role definition and other non-XML sections
808+
expect(prompt).toContain(modes[0].roleDefinition)
809+
expect(prompt).toContain("CAPABILITIES")
810+
expect(prompt).toContain("RULES")
811+
expect(prompt).toContain("SYSTEM INFORMATION")
812+
expect(prompt).toContain("OBJECTIVE")
813+
})
814+
815+
it("should default to XML tool instructions when toolProtocol is undefined", async () => {
816+
const settings = {
817+
maxConcurrentFileReads: 5,
818+
todoListEnabled: true,
819+
useAgentRules: true,
820+
newTaskRequireTodos: false,
821+
toolProtocol: "xml" as const,
822+
}
823+
824+
const prompt = await SYSTEM_PROMPT(
825+
mockContext,
826+
"/test/path",
827+
false,
828+
undefined, // mcpHub
829+
undefined, // diffStrategy
830+
undefined, // browserViewportSize
831+
defaultModeSlug, // mode
832+
undefined, // customModePrompts
833+
undefined, // customModes
834+
undefined, // globalCustomInstructions
835+
undefined, // diffEnabled
836+
experiments,
837+
true, // enableMcpServerCreation
838+
undefined, // language
839+
undefined, // rooIgnoreInstructions
840+
undefined, // partialReadsEnabled
841+
settings, // settings
842+
)
843+
844+
// Should contain XML guidance (default behavior)
845+
expect(prompt).toContain("TOOL USE")
846+
expect(prompt).toContain("XML-style tags")
847+
expect(prompt).toContain("<actual_tool_name>")
848+
expect(prompt).toContain("Tool Use Guidelines")
849+
expect(prompt).toContain("# Tools")
850+
})
851+
675852
afterAll(() => {
676853
vi.restoreAllMocks()
677854
})
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// npx vitest core/prompts/__tests__/toolProtocolResolver.spec.ts
2+
3+
import { describe, it, expect } from "vitest"
4+
import { resolveToolProtocol } from "../toolProtocolResolver"
5+
6+
describe("toolProtocolResolver", () => {
7+
it("should default to xml protocol", () => {
8+
expect(resolveToolProtocol()).toBe("xml")
9+
})
10+
})

src/core/prompts/responses.ts

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import * as path from "path"
33
import * as diff from "diff"
44
import { RooIgnoreController, LOCK_TEXT_SYMBOL } from "../ignore/RooIgnoreController"
55
import { RooProtectedController } from "../protect/RooProtectedController"
6+
import { resolveToolProtocol, isNativeProtocol } from "./toolProtocolResolver"
7+
import { ToolProtocol } from "@roo-code/types"
68

79
export const formatResponse = {
810
toolDenied: () => `The user denied this operation.`,
@@ -18,25 +20,36 @@ export const formatResponse = {
1820
rooIgnoreError: (path: string) =>
1921
`Access to ${path} is blocked by the .rooignore file settings. You must try to continue in the task without using this file, or ask the user to update the .rooignore file.`,
2022

21-
noToolsUsed: () =>
22-
`[ERROR] You did not use a tool in your previous response! Please retry with a tool use.
23+
noToolsUsed: (protocol?: ToolProtocol) => {
24+
const instructions = getToolInstructionsReminder(protocol)
2325

24-
${toolUseInstructionsReminder}
26+
return `[ERROR] You did not use a tool in your previous response! Please retry with a tool use.
27+
28+
${instructions}
2529
2630
# Next Steps
2731
28-
If you have completed the user's task, use the attempt_completion tool.
29-
If you require additional information from the user, use the ask_followup_question tool.
30-
Otherwise, if you have not completed the task and do not need additional information, then proceed with the next step of the task.
31-
(This is an automated message, so do not respond to it conversationally.)`,
32+
If you have completed the user's task, use the attempt_completion tool.
33+
If you require additional information from the user, use the ask_followup_question tool.
34+
Otherwise, if you have not completed the task and do not need additional information, then proceed with the next step of the task.
35+
(This is an automated message, so do not respond to it conversationally.)`
36+
},
3237

3338
tooManyMistakes: (feedback?: string) =>
3439
`You seem to be having trouble proceeding. The user has provided the following feedback to help guide you:\n<feedback>\n${feedback}\n</feedback>`,
3540

36-
missingToolParameterError: (paramName: string) =>
37-
`Missing value for required parameter '${paramName}'. Please retry with complete response.\n\n${toolUseInstructionsReminder}`,
41+
missingToolParameterError: (paramName: string, protocol?: ToolProtocol) => {
42+
const instructions = getToolInstructionsReminder(protocol)
3843

39-
lineCountTruncationError: (actualLineCount: number, isNewFile: boolean, diffStrategyEnabled: boolean = false) => {
44+
return `Missing value for required parameter '${paramName}'. Please retry with complete response.\n\n${instructions}`
45+
},
46+
47+
lineCountTruncationError: (
48+
actualLineCount: number,
49+
isNewFile: boolean,
50+
diffStrategyEnabled: boolean = false,
51+
protocol?: ToolProtocol,
52+
) => {
4053
const truncationMessage = `Note: Your response may have been truncated because it exceeded your output limit. You wrote ${actualLineCount} lines of content, but the line_count parameter was either missing or not included in your response.`
4154

4255
const newFileGuidance =
@@ -65,7 +78,9 @@ Otherwise, if you have not completed the task and do not need additional informa
6578
`RECOMMENDED APPROACH:\n` +
6679
`${existingFileApproaches.join("\n")}\n`
6780

68-
return `${isNewFile ? newFileGuidance : existingFileGuidance}\n${toolUseInstructionsReminder}`
81+
const instructions = getToolInstructionsReminder(protocol)
82+
83+
return `${isNewFile ? newFileGuidance : existingFileGuidance}\n${instructions}`
6984
},
7085

7186
invalidMcpToolArgumentError: (serverName: string, toolName: string) =>
@@ -220,3 +235,20 @@ I have completed the task...
220235
</attempt_completion>
221236
222237
Always use the actual tool name as the XML tag name for proper parsing and execution.`
238+
239+
const toolUseInstructionsReminderNative = `# Reminder: Instructions for Tool Use
240+
241+
Tools are invoked using the platform's native tool calling mechanism. Each tool requires specific parameters as defined in the tool descriptions. Refer to the tool definitions provided in your system instructions for the correct parameter structure and usage examples.
242+
243+
Always ensure you provide all required parameters for the tool you wish to use.`
244+
245+
/**
246+
* Gets the appropriate tool use instructions reminder based on the protocol.
247+
*
248+
* @param protocol - Optional tool protocol, falls back to default if not provided
249+
* @returns The tool use instructions reminder text
250+
*/
251+
function getToolInstructionsReminder(protocol?: ToolProtocol): string {
252+
const effectiveProtocol = protocol ?? resolveToolProtocol()
253+
return isNativeProtocol(effectiveProtocol) ? toolUseInstructionsReminderNative : toolUseInstructionsReminder
254+
}

src/core/prompts/sections/custom-instructions.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Dirent } from "fs"
66
import { isLanguage } from "@roo-code/types"
77

88
import type { SystemPromptSettings } from "../types"
9+
import { getEffectiveProtocol, isNativeProtocol } from "../toolProtocolResolver"
910

1011
import { LANGUAGES } from "../../../shared/language"
1112
import { getRooDirectoriesForCwd, getGlobalRooDirectory } from "../../../services/roo-config"
@@ -368,15 +369,20 @@ export async function addCustomInstructions(
368369

369370
const joinedSections = sections.join("\n\n")
370371

372+
const effectiveProtocol = getEffectiveProtocol(options.settings)
373+
371374
return joinedSections
372375
? `
373376
====
374377
375378
USER'S CUSTOM INSTRUCTIONS
376379
377-
The following additional instructions are provided by the user, and should be followed to the best of your ability without interfering with the TOOL USE guidelines.
380+
The following additional instructions are provided by the user, and should be followed to the best of your ability${
381+
isNativeProtocol(effectiveProtocol) ? "." : " without interfering with the TOOL USE guidelines."
382+
}
378383
379-
${joinedSections}`
384+
${joinedSections}
385+
`
380386
: ""
381387
}
382388

src/core/prompts/sections/rules.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { DiffStrategy } from "../../../shared/tools"
22
import { CodeIndexManager } from "../../../services/code-index/manager"
3+
import type { SystemPromptSettings } from "../types"
4+
import { getEffectiveProtocol, isNativeProtocol } from "../toolProtocolResolver"
35

46
function getEditingInstructions(diffStrategy?: DiffStrategy): string {
57
const instructions: string[] = []
@@ -45,6 +47,7 @@ export function getRulesSection(
4547
supportsComputerUse: boolean,
4648
diffStrategy?: DiffStrategy,
4749
codeIndexManager?: CodeIndexManager,
50+
settings?: SystemPromptSettings,
4851
): string {
4952
const isCodebaseSearchAvailable =
5053
codeIndexManager &&
@@ -56,12 +59,15 @@ export function getRulesSection(
5659
? "- **CRITICAL: For ANY exploration of code you haven't examined yet in this conversation, you MUST use the `codebase_search` tool FIRST before using search_files or other file exploration tools.** This requirement applies throughout the entire conversation, not just when starting a task. The codebase_search tool uses semantic search to find relevant code based on meaning, not just keywords, making it much more effective for understanding how features are implemented. Even if you've already explored some parts of the codebase, any new area or functionality you need to understand requires using codebase_search first.\n"
5760
: ""
5861

62+
// Determine whether to use XML tool references based on protocol
63+
const effectiveProtocol = getEffectiveProtocol(settings)
64+
5965
return `====
6066
6167
RULES
6268
6369
- The project base directory is: ${cwd.toPosix()}
64-
- 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>.
70+
- 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 ${isNativeProtocol(effectiveProtocol) ? "execute_command" : "<execute_command>"}.
6571
- 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.
6672
- Do not use the ~ character or $HOME to refer to the home directory.
6773
- 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 && 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) && (command, in this case npm install)\`.

0 commit comments

Comments
 (0)