Skip to content

Commit 5203873

Browse files
dltechydltechy
committed
refactor: Replace XML-based arg parsing with robust string-based parser
The old `parsePathFromArgsParam` used an XML parser to extract only the `<path>` tag from <args>. This caused tools to break when parameters contained diff markers (`<<<`, `>>>`) or other malformed XML content, especially in apply-diff and code-edit operations. This change replaces that logic with `parseParamsFromArgs`, a simple and safe tag-scanning helper that: - avoids XML parsing entirely (no malformed XML failures) - supports extracting any parameter name, not just `path` - preserves legacy precedence (`params.X` > args-derived values) - matches the old behavior of taking the first matching tag All tools using <args> have been updated to use the new helper for path/content/line/regex/file_pattern queries, improving consistency. Additional changes: - removed deprecated `parsePathFromArgsParam` from xml.ts - added complete test suite for `parseParamsFromArgs`, covering: - multiple <file> entries - root-level tags - missing values - complex parameter names - empty/undefined args This refactor makes the tooling more resilient to real-world input (diff blocks, code snippets, and non-well-formed markup) while expanding support for additional parameters going forward.
1 parent 98bd2e7 commit 5203873

14 files changed

+210
-68
lines changed

‎src/core/tools/ApplyDiffTool.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
66

77
import { ClineSayTool } from "../../shared/ExtensionMessage"
88
import { getReadablePath } from "../../utils/path"
9+
import { parseParamsFromArgs } from "../../utils/pathUtils"
910
import { Task } from "../task/Task"
1011
import { formatResponse } from "../prompts/responses"
1112
import { fileExistsAtPath } from "../../utils/fs"
1213
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
1314
import { unescapeHtmlEntities } from "../../utils/text-normalization"
14-
import { parsePathFromArgsParam } from "../../utils/xml"
1515
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
1616
import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
1717
import { BaseTool, ToolCallbacks } from "./BaseTool"
@@ -26,11 +26,13 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
2626
readonly name = "apply_diff" as const
2727

2828
parseLegacy(params: Partial<Record<string, string>>): ApplyDiffParams {
29-
const relPath = params.path || parsePathFromArgsParam(params) || ""
29+
const args = parseParamsFromArgs(params.args, ["path", "diff"])
30+
const relPath = params.path || args.path || ""
31+
const diff = params.diff || args.diff || ""
3032

3133
return {
3234
path: relPath,
33-
diff: params.diff || "",
35+
diff: diff,
3436
}
3537
}
3638

‎src/core/tools/CodebaseSearchTool.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import path from "path"
44
import { Task } from "../task/Task"
55
import { CodeIndexManager } from "../../services/code-index/manager"
66
import { getWorkspacePath } from "../../utils/path"
7-
import { parsePathFromArgsParam } from "../../utils/xml"
7+
import { parseParamsFromArgs } from "../../utils/pathUtils"
88
import { formatResponse } from "../prompts/responses"
99
import { VectorStoreSearchResult } from "../../services/code-index/interfaces"
1010
import { BaseTool, ToolCallbacks } from "./BaseTool"
@@ -23,15 +23,16 @@ export class CodebaseSearchTool extends BaseTool<"codebase_search"> {
2323
readonly name = "codebase_search" as const
2424

2525
parseLegacy(params: Partial<Record<string, string>>): CodebaseSearchParams {
26-
let query = params.query
27-
let directoryPrefix = params.path || parsePathFromArgsParam(params)
26+
const args = parseParamsFromArgs(params.args, ["query", "path"])
27+
let query = params.query || args.query || ""
28+
let directoryPrefix = params.path || args.path
2829

2930
if (directoryPrefix) {
3031
directoryPrefix = path.normalize(directoryPrefix)
3132
}
3233

3334
return {
34-
query: query || "",
35+
query: query,
3536
path: directoryPrefix,
3637
}
3738
}

‎src/core/tools/InsertContentTool.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ import fs from "fs/promises"
22
import path from "path"
33

44
import { getReadablePath } from "../../utils/path"
5+
import { parseParamsFromArgs } from "../../utils/pathUtils"
56
import { Task } from "../task/Task"
67
import { formatResponse } from "../prompts/responses"
78
import { ClineSayTool } from "../../shared/ExtensionMessage"
89
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
910
import { fileExistsAtPath } from "../../utils/fs"
10-
import { parsePathFromArgsParam } from "../../utils/xml"
1111
import { insertGroups } from "../diff/insert-groups"
1212
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
1313
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
@@ -25,9 +25,10 @@ export class InsertContentTool extends BaseTool<"insert_content"> {
2525
readonly name = "insert_content" as const
2626

2727
parseLegacy(params: Partial<Record<string, string>>): InsertContentParams {
28-
const relPath = params.path || parsePathFromArgsParam(params) || ""
29-
const lineStr = params.line || ""
30-
const content = params.content || ""
28+
const args = parseParamsFromArgs(params.args, ["path", "line", "content"])
29+
const relPath = params.path || args.path || ""
30+
const lineStr = params.line || args.line || ""
31+
const content = params.content || args.content || ""
3132

3233
const lineNumber = parseInt(lineStr, 10)
3334

‎src/core/tools/ListCodeDefinitionNamesTool.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ import fs from "fs/promises"
44
import { Task } from "../task/Task"
55
import { ClineSayTool } from "../../shared/ExtensionMessage"
66
import { getReadablePath } from "../../utils/path"
7-
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
8-
import { parsePathFromArgsParam } from "../../utils/xml"
7+
import { isPathOutsideWorkspace, parseParamsFromArgs } from "../../utils/pathUtils"
98
import { parseSourceCodeForDefinitionsTopLevel, parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter"
109
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
1110
import { truncateDefinitionsToLineLimit } from "./helpers/truncateDefinitions"
@@ -20,7 +19,8 @@ export class ListCodeDefinitionNamesTool extends BaseTool<"list_code_definition_
2019
readonly name = "list_code_definition_names" as const
2120

2221
parseLegacy(params: Partial<Record<string, string>>): ListCodeDefinitionNamesParams {
23-
const relPath = params.path || parsePathFromArgsParam(params) || ""
22+
const args = parseParamsFromArgs(params.args, ["path"])
23+
const relPath = params.path || args.path || ""
2424

2525
return {
2626
path: relPath,

‎src/core/tools/ListFilesTool.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ import { ClineSayTool } from "../../shared/ExtensionMessage"
55
import { formatResponse } from "../prompts/responses"
66
import { listFiles } from "../../services/glob/list-files"
77
import { getReadablePath } from "../../utils/path"
8-
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
9-
import { parsePathFromArgsParam } from "../../utils/xml"
8+
import { isPathOutsideWorkspace, parseParamsFromArgs } from "../../utils/pathUtils"
109
import { BaseTool, ToolCallbacks } from "./BaseTool"
1110
import type { ToolUse } from "../../shared/tools"
1211

@@ -19,8 +18,9 @@ export class ListFilesTool extends BaseTool<"list_files"> {
1918
readonly name = "list_files" as const
2019

2120
parseLegacy(params: Partial<Record<string, string>>): ListFilesParams {
22-
const relPath = params.path || parsePathFromArgsParam(params) || ""
23-
const recursiveRaw: string | undefined = params.recursive
21+
const args = parseParamsFromArgs(params.args, ["path", "recursive"])
22+
const relPath = params.path || args.path || ""
23+
const recursiveRaw: string | undefined = params.recursive || args.recursive
2424
const recursive = recursiveRaw?.toLowerCase() === "true"
2525

2626
return {

‎src/core/tools/SearchFilesTool.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ import path from "path"
33
import { Task } from "../task/Task"
44
import { ClineSayTool } from "../../shared/ExtensionMessage"
55
import { getReadablePath } from "../../utils/path"
6-
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
7-
import { parsePathFromArgsParam } from "../../utils/xml"
6+
import { isPathOutsideWorkspace, parseParamsFromArgs } from "../../utils/pathUtils"
87
import { regexSearchFiles } from "../../services/ripgrep"
98
import { BaseTool, ToolCallbacks } from "./BaseTool"
109
import type { ToolUse } from "../../shared/tools"
@@ -19,12 +18,15 @@ export class SearchFilesTool extends BaseTool<"search_files"> {
1918
readonly name = "search_files" as const
2019

2120
parseLegacy(params: Partial<Record<string, string>>): SearchFilesParams {
22-
const relPath = params.path || parsePathFromArgsParam(params) || ""
21+
const args = parseParamsFromArgs(params.args, ["path", "regex", "file_pattern"])
22+
const relPath = params.path || args.path || ""
23+
const regex = params.regex || args.regex || ""
24+
const filePattern = params.file_pattern || args.file_pattern
2325

2426
return {
2527
path: relPath,
26-
regex: params.regex || "",
27-
file_pattern: params.file_pattern || undefined,
28+
regex: regex,
29+
file_pattern: filePattern,
2830
}
2931
}
3032

‎src/core/tools/WriteToFileTool.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,9 @@ import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
1010
import { fileExistsAtPath } from "../../utils/fs"
1111
import { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
1212
import { getReadablePath } from "../../utils/path"
13-
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
13+
import { isPathOutsideWorkspace, parseParamsFromArgs } from "../../utils/pathUtils"
1414
import { detectCodeOmission } from "../../integrations/editor/detect-omission"
1515
import { unescapeHtmlEntities } from "../../utils/text-normalization"
16-
import { parsePathFromArgsParam } from "../../utils/xml"
1716
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
1817
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
1918
import { convertNewFileToUnifiedDiff, computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
@@ -30,12 +29,15 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> {
3029
readonly name = "write_to_file" as const
3130

3231
parseLegacy(params: Partial<Record<string, string>>): WriteToFileParams {
33-
const relPath = params.path || parsePathFromArgsParam(params) || ""
32+
const args = parseParamsFromArgs(params.args, ["path", "content", "line_count"])
33+
const relPath = params.path || args.path || ""
34+
const content = params.content || args.content || ""
35+
const lineCount = parseInt(params.line_count || args.line_count || "0", 10)
3436

3537
return {
3638
path: relPath,
37-
content: params.content || "",
38-
line_count: parseInt(params.line_count ?? "0", 10),
39+
content: content,
40+
line_count: lineCount,
3941
}
4042
}
4143

‎src/core/tools/kilocode/deleteFileTool.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ import { Task } from "../../task/Task"
55
import { ClineSayTool } from "../../../shared/ExtensionMessage"
66
import { formatResponse } from "../../prompts/responses"
77
import { getReadablePath } from "../../../utils/path"
8-
import { isPathOutsideWorkspace } from "../../../utils/pathUtils"
9-
import { parsePathFromArgsParam } from "../../../utils/xml"
8+
import { isPathOutsideWorkspace, parseParamsFromArgs } from "../../../utils/pathUtils"
109
import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../../shared/tools"
1110

1211
interface DirectoryStats {
@@ -100,7 +99,8 @@ export async function deleteFileTool(
10099
return
101100
}
102101

103-
const relPath: string | undefined = block.params.path || parsePathFromArgsParam(block.params)
102+
const args = parseParamsFromArgs(block.params.args, ["path"])
103+
const relPath: string | undefined = block.params.path || args.path
104104

105105
try {
106106
if (!relPath) {

‎src/core/tools/kilocode/editFileTool.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { formatResponse } from "../../prompts/responses"
77
import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../../shared/tools"
88
import { fileExistsAtPath } from "../../../utils/fs"
99
import { getReadablePath } from "../../../utils/path"
10-
import { parsePathFromArgsParam } from "../../../utils/xml"
10+
import { parseParamsFromArgs } from "../../../utils/pathUtils"
1111
import { FastApplyApiProvider, fastApplyApiProviderSchema, getKiloUrlFromToken } from "@roo-code/types"
1212
import { DEFAULT_HEADERS } from "../../../api/providers/constants"
1313
import { TelemetryService } from "@roo-code/telemetry"
@@ -85,9 +85,10 @@ export async function editFileTool(
8585
pushToolResult: PushToolResult,
8686
removeClosingTag: RemoveClosingTag,
8787
): Promise<void> {
88-
const target_file: string | undefined = block.params.target_file || parsePathFromArgsParam(block.params)
89-
const instructions: string | undefined = block.params.instructions
90-
const code_edit: string | undefined = block.params.code_edit
88+
const args = parseParamsFromArgs(block.params.args, ["target_file", "instructions", "code_edit"])
89+
const target_file: string | undefined = block.params.target_file || args.target_file
90+
const instructions: string | undefined = block.params.instructions || args.instructions
91+
const code_edit: string | undefined = block.params.code_edit || args.code_edit
9192

9293
let fileExists = true
9394
try {

‎src/core/tools/kilocode/searchAndReplaceTool.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import { AskApproval, HandleError, PushToolResult, ToolUse } from "../../../shar
99
import { formatResponse } from "../../prompts/responses"
1010
import { ClineSayTool } from "../../../shared/ExtensionMessage"
1111
import { getReadablePath } from "../../../utils/path"
12+
import { parseParamsFromArgs } from "../../../utils/pathUtils"
1213
import { fileExistsAtPath } from "../../../utils/fs"
13-
import { parsePathFromArgsParam } from "../../../utils/xml"
1414
import { RecordSource } from "../../context-tracking/FileContextTrackerTypes"
1515
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
1616
import { EXPERIMENT_IDS, experiments } from "../../../shared/experiments"
@@ -71,9 +71,12 @@ export async function searchAndReplaceTool(
7171
return
7272
}
7373

74-
// Fix `path` param first to prevent `validateParams` from
75-
// failing if `path` exists in `args`
76-
block.params.path ||= parsePathFromArgsParam(block.params)
74+
// Fix params first to prevent `validateParams` from
75+
// failing if they exist in `args`
76+
const args = parseParamsFromArgs(block.params.args, ["path", "old_str", "new_str"])
77+
block.params.path ||= args.path
78+
block.params.old_str ||= args.old_str
79+
block.params.new_str ||= args.new_str
7780

7881
// Validate required parameters
7982
const params = await validateParams(cline, block, pushToolResult)

0 commit comments

Comments
 (0)