mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-23 00:43:14 +00:00
Merge 9fb232b499 into e921f9d21e
This commit is contained in:
commit
ccc15800f3
5 changed files with 6 additions and 68 deletions
|
|
@ -8,7 +8,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 type { ToolUse } from "../../shared/tools"
|
||||
|
|
@ -27,10 +26,6 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
|
|||
const { askApproval, handleError, pushToolResult } = callbacks
|
||||
let { path: relPath, diff: diffContent } = params
|
||||
|
||||
if (diffContent && !task.api.getModel().id.includes("claude")) {
|
||||
diffContent = unescapeHtmlEntities(diffContent)
|
||||
}
|
||||
|
||||
try {
|
||||
if (!relPath) {
|
||||
task.consecutiveMistakeCount++
|
||||
|
|
|
|||
|
|
@ -10,7 +10,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"
|
||||
|
|
@ -52,9 +51,7 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
|
|||
return
|
||||
}
|
||||
|
||||
const canonicalCommand = unescapeHtmlEntities(command)
|
||||
|
||||
const ignoredFileAttemptedToAccess = task.rooIgnoreController?.validateCommand(canonicalCommand)
|
||||
const ignoredFileAttemptedToAccess = task.rooIgnoreController?.validateCommand(command)
|
||||
|
||||
if (ignoredFileAttemptedToAccess) {
|
||||
await task.say("rooignore_error", ignoredFileAttemptedToAccess)
|
||||
|
|
@ -64,7 +61,7 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
|
|||
|
||||
task.consecutiveMistakeCount = 0
|
||||
|
||||
const didApprove = await askApproval("command", canonicalCommand)
|
||||
const didApprove = await askApproval("command", command)
|
||||
|
||||
if (!didApprove) {
|
||||
return
|
||||
|
|
@ -87,9 +84,7 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
|
|||
.get<string[]>("commandTimeoutAllowlist", [])
|
||||
|
||||
// Check if command matches any prefix in the allowlist
|
||||
const isCommandAllowlisted = commandTimeoutAllowlist.some((prefix) =>
|
||||
canonicalCommand.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
|
||||
|
|
@ -99,7 +94,7 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
|
|||
|
||||
const options: ExecuteCommandOptions = {
|
||||
executionId,
|
||||
command: canonicalCommand,
|
||||
command: command,
|
||||
customCwd,
|
||||
terminalShellIntegrationDisabled,
|
||||
commandExecutionTimeout,
|
||||
|
|
|
|||
|
|
@ -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 { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
|
||||
import { convertNewFileToUnifiedDiff, computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
|
||||
import type { ToolUse } from "../../shared/tools"
|
||||
|
|
@ -81,10 +80,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, relPath) : ""
|
||||
const isOutsideWorkspace = isPathOutsideWorkspace(fullPath)
|
||||
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@ import * as vscode from "vscode"
|
|||
import { Task } from "../../task/Task"
|
||||
import { formatResponse } from "../../prompts/responses"
|
||||
import { ToolUse, AskApproval, HandleError, PushToolResult } from "../../../shared/tools"
|
||||
import { unescapeHtmlEntities } from "../../../utils/text-normalization"
|
||||
|
||||
// Mock dependencies
|
||||
vitest.mock("execa", () => ({
|
||||
|
|
@ -111,37 +110,6 @@ describe("executeCommandTool", () => {
|
|||
process.env.ROO_CLI_RUNTIME = originalCliRuntime
|
||||
})
|
||||
|
||||
/**
|
||||
* 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 <test>"
|
||||
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' <file.txt >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 () => {
|
||||
// Setup
|
||||
|
|
|
|||
|
|
@ -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<typeof createDirectoriesForFile>
|
||||
const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction<typeof isPathOutsideWorkspace>
|
||||
const mockedGetReadablePath = getReadablePath as MockedFunction<typeof getReadablePath>
|
||||
const mockedUnescapeHtmlEntities = unescapeHtmlEntities as MockedFunction<typeof unescapeHtmlEntities>
|
||||
const mockedEveryLineHasLineNumbers = everyLineHasLineNumbers as MockedFunction<typeof everyLineHasLineNumbers>
|
||||
const mockedStripLineNumbers = stripLineNumbers as MockedFunction<typeof stripLineNumbers>
|
||||
const mockedPathResolve = path.resolve as MockedFunction<typeof path.resolve>
|
||||
|
|
@ -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)
|
||||
|
||||
|
|
@ -327,20 +320,12 @@ describe("writeToFileTool", () => {
|
|||
expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("", true)
|
||||
})
|
||||
|
||||
it("unescapes HTML entities for non-Claude models", async () => {
|
||||
it("preserves HTML entities in content (no unescaping)", 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()
|
||||
expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("<test>", true)
|
||||
})
|
||||
|
||||
it("strips line numbers from numbered content", async () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue