diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index 7161c7c08e..e3a5872bee 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -10,7 +10,6 @@ import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { fileExistsAtPath } from "../../utils/fs" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" -import { unescapeHtmlEntities } from "../../utils/text-normalization" import { EXPERIMENT_IDS, experiments } from "../../shared/experiments" import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats" import { BaseTool, ToolCallbacks } from "./BaseTool" @@ -35,10 +34,6 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks let { path: relPath, diff: diffContent } = params - if (diffContent && !task.api.getModel().id.includes("claude")) { - diffContent = unescapeHtmlEntities(diffContent) - } - try { if (!relPath) { task.consecutiveMistakeCount++ diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index f7271bffe9..61a516fb5b 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -11,7 +11,6 @@ import { Task } from "../task/Task" import { ToolUse, ToolResponse } from "../../shared/tools" import { formatResponse } from "../prompts/responses" -import { unescapeHtmlEntities } from "../../utils/text-normalization" import { ExitCodeDetails, RooTerminalCallbacks, RooTerminalProcess } from "../../integrations/terminal/types" import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry" import { Terminal } from "../../integrations/terminal/Terminal" @@ -58,8 +57,7 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> { task.consecutiveMistakeCount = 0 - const unescapedCommand = unescapeHtmlEntities(command) - const didApprove = await askApproval("command", unescapedCommand) + const didApprove = await askApproval("command", command) if (!didApprove) { return @@ -86,16 +84,14 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> { .get("commandTimeoutAllowlist", []) // Check if command matches any prefix in the allowlist - const isCommandAllowlisted = commandTimeoutAllowlist.some((prefix) => - unescapedCommand.startsWith(prefix.trim()), - ) + const isCommandAllowlisted = commandTimeoutAllowlist.some((prefix) => command.startsWith(prefix.trim())) // Convert seconds to milliseconds for internal use, but skip timeout if command is allowlisted const commandExecutionTimeout = isCommandAllowlisted ? 0 : commandExecutionTimeoutSeconds * 1000 const options: ExecuteCommandOptions = { executionId, - command: unescapedCommand, + command, customCwd, terminalShellIntegrationDisabled, terminalOutputLineLimit, diff --git a/src/core/tools/MultiApplyDiffTool.ts b/src/core/tools/MultiApplyDiffTool.ts index 94cdb3fd49..9756cf963a 100644 --- a/src/core/tools/MultiApplyDiffTool.ts +++ b/src/core/tools/MultiApplyDiffTool.ts @@ -11,7 +11,6 @@ import { ToolUse, RemoveClosingTag, AskApproval, HandleError, PushToolResult } f import { formatResponse } from "../prompts/responses" import { fileExistsAtPath } from "../../utils/fs" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" -import { unescapeHtmlEntities } from "../../utils/text-normalization" import { parseXmlForDiff } from "../../utils/xml" import { EXPERIMENT_IDS, experiments } from "../../shared/experiments" import { applyDiffTool as applyDiffToolClass } from "./ApplyDiffTool" @@ -204,7 +203,7 @@ Original error: ${errorMessage}` path: legacyPath, diff: [ { - content: legacyDiffContent, // Unescaping will be handled later like new diffs + content: legacyDiffContent, startLine: legacyStartLineStr ? parseInt(legacyStartLineStr) : undefined, }, ], @@ -322,12 +321,7 @@ Original error: ${errorMessage}` let unified = "" try { const original = await fs.readFile(opResult.absolutePath!, "utf-8") - const processed = !cline.api.getModel().id.includes("claude") - ? (opResult.diffItems || []).map((item) => ({ - ...item, - content: item.content ? unescapeHtmlEntities(item.content) : item.content, - })) - : opResult.diffItems || [] + const processed = opResult.diffItems || [] const applyRes = (await cline.diffStrategy?.applyDiff(original, processed)) ?? ({ success: false } as any) @@ -484,12 +478,8 @@ Original error: ${errorMessage}` let formattedError = "" // Pre-process all diff items for HTML entity unescaping if needed - const processedDiffItems = !cline.api.getModel().id.includes("claude") - ? diffItems.map((item) => ({ - ...item, - content: item.content ? unescapeHtmlEntities(item.content) : item.content, - })) - : diffItems + // Use diff items directly without HTML entity unescaping + const processedDiffItems = diffItems // Apply all diffs at once with the array-based method const diffResult = (await cline.diffStrategy?.applyDiff(originalContent, processedDiffItems)) ?? { diff --git a/src/core/tools/WriteToFileTool.ts b/src/core/tools/WriteToFileTool.ts index 7caaeb6d55..682f8216a0 100644 --- a/src/core/tools/WriteToFileTool.ts +++ b/src/core/tools/WriteToFileTool.ts @@ -11,7 +11,6 @@ import { fileExistsAtPath, createDirectoriesForFile } from "../../utils/fs" import { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text" import { getReadablePath } from "../../utils/path" import { isPathOutsideWorkspace } from "../../utils/pathUtils" -import { unescapeHtmlEntities } from "../../utils/text-normalization" import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" import { EXPERIMENT_IDS, experiments } from "../../shared/experiments" import { convertNewFileToUnifiedDiff, computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats" @@ -88,10 +87,6 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> { newContent = newContent.split("\n").slice(0, -1).join("\n") } - if (!task.api.getModel().id.includes("claude")) { - newContent = unescapeHtmlEntities(newContent) - } - const fullPath = relPath ? path.resolve(task.cwd, removeClosingTag("path", relPath)) : "" const isOutsideWorkspace = isPathOutsideWorkspace(fullPath) diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index 0406a83d2a..4a3482a42c 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -6,7 +6,6 @@ import * as vscode from "vscode" import { Task } from "../../task/Task" import { formatResponse } from "../../prompts/responses" import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../../shared/tools" -import { unescapeHtmlEntities } from "../../../utils/text-normalization" // Mock dependencies vitest.mock("execa", () => ({ @@ -105,36 +104,6 @@ describe("executeCommandTool", () => { } }) - /** - * Tests for HTML entity unescaping in commands - * This verifies that HTML entities are properly converted to their actual characters - */ - describe("HTML entity unescaping", () => { - it("should unescape < to < character", () => { - const input = "echo <test>" - const expected = "echo " - expect(unescapeHtmlEntities(input)).toBe(expected) - }) - - it("should unescape > to > character", () => { - const input = "echo test > output.txt" - const expected = "echo test > output.txt" - expect(unescapeHtmlEntities(input)).toBe(expected) - }) - - it("should unescape & to & character", () => { - const input = "echo foo && echo bar" - const expected = "echo foo && echo bar" - expect(unescapeHtmlEntities(input)).toBe(expected) - }) - - it("should handle multiple mixed HTML entities", () => { - const input = "grep -E 'pattern' <file.txt >output.txt 2>&1" - const expected = "grep -E 'pattern' output.txt 2>&1" - expect(unescapeHtmlEntities(input)).toBe(expected) - }) - }) - // Now we can run these tests describe("Basic functionality", () => { it("should execute a command normally", async () => { diff --git a/src/core/tools/__tests__/writeToFileTool.spec.ts b/src/core/tools/__tests__/writeToFileTool.spec.ts index 3970a99f06..6cf8f51ce2 100644 --- a/src/core/tools/__tests__/writeToFileTool.spec.ts +++ b/src/core/tools/__tests__/writeToFileTool.spec.ts @@ -5,7 +5,6 @@ import type { MockedFunction } from "vitest" import { fileExistsAtPath, createDirectoriesForFile } from "../../../utils/fs" import { isPathOutsideWorkspace } from "../../../utils/pathUtils" import { getReadablePath } from "../../../utils/path" -import { unescapeHtmlEntities } from "../../../utils/text-normalization" import { everyLineHasLineNumbers, stripLineNumbers } from "../../../integrations/misc/extract-text" import { ToolUse, ToolResponse } from "../../../shared/tools" import { writeToFileTool } from "../WriteToFileTool" @@ -47,10 +46,6 @@ vi.mock("../../../utils/path", () => ({ getReadablePath: vi.fn().mockReturnValue("test/path.txt"), })) -vi.mock("../../../utils/text-normalization", () => ({ - unescapeHtmlEntities: vi.fn().mockImplementation((content) => content), -})) - vi.mock("../../../integrations/misc/extract-text", () => ({ everyLineHasLineNumbers: vi.fn().mockReturnValue(false), stripLineNumbers: vi.fn().mockImplementation((content) => content), @@ -97,7 +92,6 @@ describe("writeToFileTool", () => { const mockedCreateDirectoriesForFile = createDirectoriesForFile as MockedFunction const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction const mockedGetReadablePath = getReadablePath as MockedFunction - const mockedUnescapeHtmlEntities = unescapeHtmlEntities as MockedFunction const mockedEveryLineHasLineNumbers = everyLineHasLineNumbers as MockedFunction const mockedStripLineNumbers = stripLineNumbers as MockedFunction const mockedPathResolve = path.resolve as MockedFunction @@ -116,7 +110,6 @@ describe("writeToFileTool", () => { mockedFileExistsAtPath.mockResolvedValue(false) mockedIsPathOutsideWorkspace.mockReturnValue(false) mockedGetReadablePath.mockReturnValue("test/path.txt") - mockedUnescapeHtmlEntities.mockImplementation((content) => content) mockedEveryLineHasLineNumbers.mockReturnValue(false) mockedStripLineNumbers.mockImplementation((content) => content) @@ -322,22 +315,6 @@ describe("writeToFileTool", () => { expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("", true) }) - it("unescapes HTML entities for non-Claude models", async () => { - mockCline.api.getModel.mockReturnValue({ id: "gpt-4" }) - - await executeWriteFileTool({ content: "<test>" }) - - expect(mockedUnescapeHtmlEntities).toHaveBeenCalledWith("<test>") - }) - - it("skips HTML unescaping for Claude models", async () => { - mockCline.api.getModel.mockReturnValue({ id: "claude-3" }) - - await executeWriteFileTool({ content: "<test>" }) - - expect(mockedUnescapeHtmlEntities).not.toHaveBeenCalled() - }) - it("strips line numbers from numbered content", async () => { const contentWithLineNumbers = "1 | line one\n2 | line two" mockedEveryLineHasLineNumbers.mockReturnValue(true)