Skip to content

Commit 271d01b

Browse files
daniel-lxsdaniel-lxs
authored
fix: Update tools to return structured JSON for native protocol (#9373)
1 parent 55e9c88 commit 271d01b

34 files changed

+320
-94
lines changed

‎src/core/assistant-message/presentAssistantMessage.ts‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { fetchInstructionsTool } from "../tools/FetchInstructionsTool"
1313
import { listFilesTool } from "../tools/ListFilesTool"
1414
import { readFileTool } from "../tools/ReadFileTool"
1515
import { getSimpleReadFileToolDescription, simpleReadFileTool } from "../tools/simpleReadFileTool"
16-
import { shouldUseSingleFileRead } from "@roo-code/types"
16+
import { shouldUseSingleFileRead, TOOL_PROTOCOL } from "@roo-code/types"
1717
import { writeToFileTool } from "../tools/WriteToFileTool"
1818
import { applyDiffTool } from "../tools/MultiApplyDiffTool"
1919
import { insertContentTool } from "../tools/InsertContentTool"
@@ -284,10 +284,10 @@ export async function presentAssistantMessage(cline: Task) {
284284
// Native protocol tool calls ALWAYS have an ID (set when parsed from tool_call chunks).
285285
// XML protocol tool calls NEVER have an ID (parsed from XML text).
286286
const toolCallId = (block as any).id
287-
const isNative = !!toolCallId
287+
const toolProtocol = toolCallId ? TOOL_PROTOCOL.NATIVE : TOOL_PROTOCOL.XML
288288

289289
const pushToolResult = (content: ToolResponse) => {
290-
if (isNative && toolCallId) {
290+
if (toolProtocol === TOOL_PROTOCOL.NATIVE) {
291291
// For native protocol, only allow ONE tool_result per tool call
292292
if (hasToolResult) {
293293
console.warn(
@@ -360,9 +360,14 @@ export async function presentAssistantMessage(cline: Task) {
360360
// Handle both messageResponse and noButtonClicked with text.
361361
if (text) {
362362
await cline.say("user_feedback", text, images)
363-
pushToolResult(formatResponse.toolResult(formatResponse.toolDeniedWithFeedback(text), images))
363+
pushToolResult(
364+
formatResponse.toolResult(
365+
formatResponse.toolDeniedWithFeedback(text, toolProtocol),
366+
images,
367+
),
368+
)
364369
} else {
365-
pushToolResult(formatResponse.toolDenied())
370+
pushToolResult(formatResponse.toolDenied(toolProtocol))
366371
}
367372
cline.didRejectTool = true
368373
return false
@@ -371,7 +376,9 @@ export async function presentAssistantMessage(cline: Task) {
371376
// Handle yesButtonClicked with text.
372377
if (text) {
373378
await cline.say("user_feedback", text, images)
374-
pushToolResult(formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text), images))
379+
pushToolResult(
380+
formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text, toolProtocol), images),
381+
)
375382
}
376383

377384
return true
@@ -394,7 +401,7 @@ export async function presentAssistantMessage(cline: Task) {
394401
`Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}`,
395402
)
396403

397-
pushToolResult(formatResponse.toolError(errorString))
404+
pushToolResult(formatResponse.toolError(errorString, toolProtocol))
398405
}
399406

400407
// If block is partial, remove partial closing tag so its not
@@ -446,7 +453,7 @@ export async function presentAssistantMessage(cline: Task) {
446453
)
447454
} catch (error) {
448455
cline.consecutiveMistakeCount++
449-
pushToolResult(formatResponse.toolError(error.message))
456+
pushToolResult(formatResponse.toolError(error.message, toolProtocol))
450457
break
451458
}
452459

@@ -485,6 +492,7 @@ export async function presentAssistantMessage(cline: Task) {
485492
pushToolResult(
486493
formatResponse.toolError(
487494
`Tool call repetition limit reached for ${block.name}. Please try a different approach.`,
495+
toolProtocol,
488496
),
489497
)
490498
break
@@ -499,6 +507,7 @@ export async function presentAssistantMessage(cline: Task) {
499507
handleError,
500508
pushToolResult,
501509
removeClosingTag,
510+
toolProtocol,
502511
})
503512
break
504513
case "update_todo_list":
@@ -507,19 +516,21 @@ export async function presentAssistantMessage(cline: Task) {
507516
handleError,
508517
pushToolResult,
509518
removeClosingTag,
519+
toolProtocol,
510520
})
511521
break
512522
case "apply_diff": {
513523
await checkpointSaveAndMark(cline)
514524

515525
// Check if this tool call came from native protocol by checking for ID
516526
// Native calls always have IDs, XML calls never do
517-
if (isNative) {
527+
if (toolProtocol === TOOL_PROTOCOL.NATIVE) {
518528
await applyDiffToolClass.handle(cline, block as ToolUse<"apply_diff">, {
519529
askApproval,
520530
handleError,
521531
pushToolResult,
522532
removeClosingTag,
533+
toolProtocol,
523534
})
524535
break
525536
}
@@ -544,6 +555,7 @@ export async function presentAssistantMessage(cline: Task) {
544555
handleError,
545556
pushToolResult,
546557
removeClosingTag,
558+
toolProtocol,
547559
})
548560
}
549561
break
@@ -555,6 +567,7 @@ export async function presentAssistantMessage(cline: Task) {
555567
handleError,
556568
pushToolResult,
557569
removeClosingTag,
570+
toolProtocol,
558571
})
559572
break
560573
case "read_file":
@@ -568,6 +581,7 @@ export async function presentAssistantMessage(cline: Task) {
568581
handleError,
569582
pushToolResult,
570583
removeClosingTag,
584+
toolProtocol,
571585
)
572586
} else {
573587
// Type assertion is safe here because we're in the "read_file" case
@@ -576,6 +590,7 @@ export async function presentAssistantMessage(cline: Task) {
576590
handleError,
577591
pushToolResult,
578592
removeClosingTag,
593+
toolProtocol,
579594
})
580595
}
581596
break
@@ -585,6 +600,7 @@ export async function presentAssistantMessage(cline: Task) {
585600
handleError,
586601
pushToolResult,
587602
removeClosingTag,
603+
toolProtocol,
588604
})
589605
break
590606
case "list_files":
@@ -593,6 +609,7 @@ export async function presentAssistantMessage(cline: Task) {
593609
handleError,
594610
pushToolResult,
595611
removeClosingTag,
612+
toolProtocol,
596613
})
597614
break
598615
case "codebase_search":
@@ -601,6 +618,7 @@ export async function presentAssistantMessage(cline: Task) {
601618
handleError,
602619
pushToolResult,
603620
removeClosingTag,
621+
toolProtocol,
604622
})
605623
break
606624
case "list_code_definition_names":
@@ -609,6 +627,7 @@ export async function presentAssistantMessage(cline: Task) {
609627
handleError,
610628
pushToolResult,
611629
removeClosingTag,
630+
toolProtocol,
612631
})
613632
break
614633
case "search_files":
@@ -617,6 +636,7 @@ export async function presentAssistantMessage(cline: Task) {
617636
handleError,
618637
pushToolResult,
619638
removeClosingTag,
639+
toolProtocol,
620640
})
621641
break
622642
case "browser_action":
@@ -625,6 +645,7 @@ export async function presentAssistantMessage(cline: Task) {
625645
handleError,
626646
pushToolResult,
627647
removeClosingTag,
648+
toolProtocol,
628649
})
629650
break
630651
case "execute_command":
@@ -633,6 +654,7 @@ export async function presentAssistantMessage(cline: Task) {
633654
handleError,
634655
pushToolResult,
635656
removeClosingTag,
657+
toolProtocol,
636658
})
637659
break
638660
case "use_mcp_tool":
@@ -641,6 +663,7 @@ export async function presentAssistantMessage(cline: Task) {
641663
handleError,
642664
pushToolResult,
643665
removeClosingTag,
666+
toolProtocol,
644667
})
645668
break
646669
case "access_mcp_resource":
@@ -659,6 +682,7 @@ export async function presentAssistantMessage(cline: Task) {
659682
handleError,
660683
pushToolResult,
661684
removeClosingTag,
685+
toolProtocol,
662686
})
663687
break
664688
case "switch_mode":
@@ -667,6 +691,7 @@ export async function presentAssistantMessage(cline: Task) {
667691
handleError,
668692
pushToolResult,
669693
removeClosingTag,
694+
toolProtocol,
670695
})
671696
break
672697
case "new_task":
@@ -675,6 +700,7 @@ export async function presentAssistantMessage(cline: Task) {
675700
handleError,
676701
pushToolResult,
677702
removeClosingTag,
703+
toolProtocol,
678704
})
679705
break
680706
case "attempt_completion": {
@@ -685,6 +711,7 @@ export async function presentAssistantMessage(cline: Task) {
685711
removeClosingTag,
686712
askFinishSubTaskApproval,
687713
toolDescription,
714+
toolProtocol,
688715
}
689716
await attemptCompletionTool.handle(
690717
cline,
@@ -699,6 +726,7 @@ export async function presentAssistantMessage(cline: Task) {
699726
handleError,
700727
pushToolResult,
701728
removeClosingTag,
729+
toolProtocol,
702730
})
703731
break
704732
case "generate_image":
@@ -708,6 +736,7 @@ export async function presentAssistantMessage(cline: Task) {
708736
handleError,
709737
pushToolResult,
710738
removeClosingTag,
739+
toolProtocol,
711740
})
712741
break
713742
}

‎src/core/prompts/responses.ts‎

Lines changed: 96 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,61 @@ import { RooProtectedController } from "../protect/RooProtectedController"
66
import { ToolProtocol, isNativeProtocol, TOOL_PROTOCOL } from "@roo-code/types"
77

88
export const formatResponse = {
9-
toolDenied: () => `The user denied this operation.`,
9+
toolDenied: (protocol?: ToolProtocol) => {
10+
if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
11+
return JSON.stringify({
12+
status: "denied",
13+
message: "The user denied this operation.",
14+
})
15+
}
16+
return `The user denied this operation.`
17+
},
1018

11-
toolDeniedWithFeedback: (feedback?: string) =>
12-
`The user denied this operation and provided the following feedback:\n<feedback>\n${feedback}\n</feedback>`,
19+
toolDeniedWithFeedback: (feedback?: string, protocol?: ToolProtocol) => {
20+
if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
21+
return JSON.stringify({
22+
status: "denied",
23+
message: "The user denied this operation and provided the following feedback",
24+
feedback: feedback,
25+
})
26+
}
27+
return `The user denied this operation and provided the following feedback:\n<feedback>\n${feedback}\n</feedback>`
28+
},
1329

14-
toolApprovedWithFeedback: (feedback?: string) =>
15-
`The user approved this operation and provided the following context:\n<feedback>\n${feedback}\n</feedback>`,
30+
toolApprovedWithFeedback: (feedback?: string, protocol?: ToolProtocol) => {
31+
if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
32+
return JSON.stringify({
33+
status: "approved",
34+
message: "The user approved this operation and provided the following context",
35+
feedback: feedback,
36+
})
37+
}
38+
return `The user approved this operation and provided the following context:\n<feedback>\n${feedback}\n</feedback>`
39+
},
1640

17-
toolError: (error?: string) => `The tool execution failed with the following error:\n<error>\n${error}\n</error>`,
41+
toolError: (error?: string, protocol?: ToolProtocol) => {
42+
if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
43+
return JSON.stringify({
44+
status: "error",
45+
message: "The tool execution failed",
46+
error: error,
47+
})
48+
}
49+
return `The tool execution failed with the following error:\n<error>\n${error}\n</error>`
50+
},
1851

19-
rooIgnoreError: (path: string) =>
20-
`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.`,
52+
rooIgnoreError: (path: string, protocol?: ToolProtocol) => {
53+
if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
54+
return JSON.stringify({
55+
status: "error",
56+
type: "access_denied",
57+
message: "Access blocked by .rooignore",
58+
path: path,
59+
suggestion: "Try to continue without this file, or ask the user to update the .rooignore file",
60+
})
61+
}
62+
return `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.`
63+
},
2164

2265
noToolsUsed: (protocol?: ToolProtocol) => {
2366
const instructions = getToolInstructionsReminder(protocol)
@@ -34,8 +77,16 @@ Otherwise, if you have not completed the task and do not need additional informa
3477
(This is an automated message, so do not respond to it conversationally.)`
3578
},
3679

37-
tooManyMistakes: (feedback?: string) =>
38-
`You seem to be having trouble proceeding. The user has provided the following feedback to help guide you:\n<feedback>\n${feedback}\n</feedback>`,
80+
tooManyMistakes: (feedback?: string, protocol?: ToolProtocol) => {
81+
if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
82+
return JSON.stringify({
83+
status: "guidance",
84+
message: "You seem to be having trouble proceeding",
85+
feedback: feedback,
86+
})
87+
}
88+
return `You seem to be having trouble proceeding. The user has provided the following feedback to help guide you:\n<feedback>\n${feedback}\n</feedback>`
89+
},
3990

4091
missingToolParameterError: (paramName: string, protocol?: ToolProtocol) => {
4192
const instructions = getToolInstructionsReminder(protocol)
@@ -82,15 +133,46 @@ Otherwise, if you have not completed the task and do not need additional informa
82133
return `${isNewFile ? newFileGuidance : existingFileGuidance}\n${instructions}`
83134
},
84135

85-
invalidMcpToolArgumentError: (serverName: string, toolName: string) =>
86-
`Invalid JSON argument used with ${serverName} for ${toolName}. Please retry with a properly formatted JSON argument.`,
136+
invalidMcpToolArgumentError: (serverName: string, toolName: string, protocol?: ToolProtocol) => {
137+
if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
138+
return JSON.stringify({
139+
status: "error",
140+
type: "invalid_argument",
141+
message: "Invalid JSON argument",
142+
server: serverName,
143+
tool: toolName,
144+
suggestion: "Please retry with a properly formatted JSON argument",
145+
})
146+
}
147+
return `Invalid JSON argument used with ${serverName} for ${toolName}. Please retry with a properly formatted JSON argument.`
148+
},
87149

88-
unknownMcpToolError: (serverName: string, toolName: string, availableTools: string[]) => {
150+
unknownMcpToolError: (serverName: string, toolName: string, availableTools: string[], protocol?: ToolProtocol) => {
151+
if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
152+
return JSON.stringify({
153+
status: "error",
154+
type: "unknown_tool",
155+
message: "Tool does not exist on server",
156+
server: serverName,
157+
tool: toolName,
158+
available_tools: availableTools.length > 0 ? availableTools : [],
159+
suggestion: "Please use one of the available tools or check if the server is properly configured",
160+
})
161+
}
89162
const toolsList = availableTools.length > 0 ? availableTools.join(", ") : "No tools available"
90163
return `Tool '${toolName}' does not exist on server '${serverName}'.\n\nAvailable tools on this server: ${toolsList}\n\nPlease use one of the available tools or check if the server is properly configured.`
91164
},
92165

93-
unknownMcpServerError: (serverName: string, availableServers: string[]) => {
166+
unknownMcpServerError: (serverName: string, availableServers: string[], protocol?: ToolProtocol) => {
167+
if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
168+
return JSON.stringify({
169+
status: "error",
170+
type: "unknown_server",
171+
message: "Server is not configured",
172+
server: serverName,
173+
available_servers: availableServers.length > 0 ? availableServers : [],
174+
})
175+
}
94176
const serversList = availableServers.length > 0 ? availableServers.join(", ") : "No servers available"
95177
return `Server '${serverName}' is not configured. Available servers: ${serversList}`
96178
},

‎src/core/prompts/system.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ async function generatePrompt(
8686
const effectiveProtocol = getEffectiveProtocol(settings?.toolProtocol)
8787

8888
const [modesSection, mcpServersSection] = await Promise.all([
89-
getModesSection(context, isNativeProtocol(effectiveProtocol)),
89+
getModesSection(context),
9090
shouldIncludeMcp
9191
? getMcpServersSection(
9292
mcpHub,

0 commit comments

Comments
 (0)