Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 155 additions & 0 deletions src/core/tools/__tests__/runSlashCommandTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,22 @@ import { runSlashCommandTool } from "../runSlashCommandTool"
import { Task } from "../../task/Task"
import { formatResponse } from "../../prompts/responses"
import { getCommand, getCommandNames } from "../../../services/command/commands"
import { parseMentions } from "../../mentions"

// Mock dependencies
vi.mock("../../../services/command/commands", () => ({
getCommand: vi.fn(),
getCommandNames: vi.fn(),
}))

vi.mock("../../mentions", () => ({
parseMentions: vi.fn(),
}))

vi.mock("../../../services/browser/UrlContentFetcher", () => ({
UrlContentFetcher: vi.fn().mockImplementation(() => ({})),
}))

describe("runSlashCommandTool", () => {
let mockTask: any
let mockAskApproval: any
Expand All @@ -20,6 +29,9 @@ describe("runSlashCommandTool", () => {
beforeEach(() => {
vi.clearAllMocks()

// By default, mock parseMentions to return the original content unchanged
vi.mocked(parseMentions).mockImplementation((content) => Promise.resolve(content))

mockTask = {
consecutiveMistakeCount: 0,
recordToolError: vi.fn(),
Expand Down Expand Up @@ -377,4 +389,147 @@ Deploy application to production`,

expect(mockTask.consecutiveMistakeCount).toBe(0)
})

it("should process mentions in command content", async () => {
const mockCommand = {
name: "test",
content: "Check @/README.md for details",
source: "project" as const,
filePath: ".roo/commands/test.md",
description: "Test command with file reference",
}

vi.mocked(getCommand).mockResolvedValue(mockCommand)
vi.mocked(parseMentions).mockResolvedValue(
"Check 'README.md' (see below for file content)\n\n<file_content path=\"README.md\">\n# README\nTest content\n</file_content>",
)

mockAskApproval.mockResolvedValue(true)

const block = {
type: "tool_use" as const,
name: "run_slash_command" as const,
params: {
command: "test",
},
partial: false,
}

await runSlashCommandTool(
mockTask as Task,
block,
mockAskApproval,
mockHandleError,
mockPushToolResult,
mockRemoveClosingTag,
)

// Verify parseMentions was called with the command content
expect(vi.mocked(parseMentions)).toHaveBeenCalledWith(
"Check @/README.md for details",
"/test/project",
expect.any(Object), // UrlContentFetcher instance
undefined,
undefined,
false,
true,
50,
undefined,
)

// Verify the processed content is included in the result
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining("Check 'README.md' (see below for file content)"),
)
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining('<file_content path="README.md">'))
})

it("should handle mention processing errors gracefully", async () => {
const mockCommand = {
name: "test",
content: "Check @/README.md for details",
source: "project" as const,
filePath: ".roo/commands/test.md",
description: "Test command with file reference",
}

vi.mocked(getCommand).mockResolvedValue(mockCommand)
vi.mocked(parseMentions).mockRejectedValue(new Error("Failed to process mentions"))

mockAskApproval.mockResolvedValue(true)

const block = {
type: "tool_use" as const,
name: "run_slash_command" as const,
params: {
command: "test",
},
partial: false,
}

// Mock console.warn to verify it's called
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})

await runSlashCommandTool(
mockTask as Task,
block,
mockAskApproval,
mockHandleError,
mockPushToolResult,
mockRemoveClosingTag,
)

// Should log a warning when mention processing fails
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining("Failed to process mentions in slash command content:"),
)

// Should still return the original content when mention processing fails
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Check @/README.md for details"))

consoleWarnSpy.mockRestore()
})

it("should process multiple file references in command content", async () => {
const mockCommand = {
name: "docs",
content: "Review @/README.md and @/CONTRIBUTING.md for guidelines",
source: "project" as const,
filePath: ".roo/commands/docs.md",
description: "Documentation command",
}

vi.mocked(getCommand).mockResolvedValue(mockCommand)
vi.mocked(parseMentions).mockResolvedValue(
"Review 'README.md' (see below for file content) and 'CONTRIBUTING.md' (see below for file content)\n\n" +
'<file_content path="README.md">\n# README\n</file_content>\n\n' +
'<file_content path="CONTRIBUTING.md">\n# Contributing\n</file_content>',
)

mockAskApproval.mockResolvedValue(true)

const block = {
type: "tool_use" as const,
name: "run_slash_command" as const,
params: {
command: "docs",
},
partial: false,
}

await runSlashCommandTool(
mockTask as Task,
block,
mockAskApproval,
mockHandleError,
mockPushToolResult,
mockRemoveClosingTag,
)

// Verify both files are included in the processed content
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining('<file_content path="README.md">'))
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining('<file_content path="CONTRIBUTING.md">'),
)
})
})
27 changes: 26 additions & 1 deletion src/core/tools/runSlashCommandTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } f
import { formatResponse } from "../prompts/responses"
import { getCommand, getCommandNames } from "../../services/command/commands"
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
import { parseMentions } from "../mentions"
import { UrlContentFetcher } from "../../services/browser/UrlContentFetcher"

export async function runSlashCommandTool(
task: Task,
Expand Down Expand Up @@ -78,6 +80,29 @@ export async function runSlashCommandTool(
return
}

// Process mentions in the command content
const provider = task.providerRef.deref()
const urlContentFetcher = new UrlContentFetcher(provider!.context)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical: Potential null pointer dereference. The provider could be undefined if task.providerRef.deref() returns undefined, but you're using the non-null assertion operator (!) on line 85. This will cause a runtime error if provider is undefined.

You should add a null check before using provider.context:

Suggested change
const urlContentFetcher = new UrlContentFetcher(provider!.context)
const provider = task.providerRef.deref()
if (!provider) {
pushToolResult(
formatResponse.toolError("Provider not available for processing mentions in slash command"),
)
return
}
const urlContentFetcher = new UrlContentFetcher(provider.context)

let processedContent = command.content

try {
// Process @/file references and other mentions in the command content
processedContent = await parseMentions(
command.content,
task.cwd,
urlContentFetcher,
undefined, // fileContextTracker - not needed for slash commands
undefined, // rooIgnoreController - will use default
false, // showRooIgnoredFiles
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
undefined, // maxReadFileLine
)
} catch (error) {
// If mention processing fails, log the error but continue with original content
console.warn(`Failed to process mentions in slash command content: ${error}`)
}

// Build the result message
let result = `Command: /${commandName}`

Expand All @@ -94,7 +119,7 @@ export async function runSlashCommandTool(
}

result += `\nSource: ${command.source}`
result += `\n\n--- Command Content ---\n\n${command.content}`
result += `\n\n--- Command Content ---\n\n${processedContent}`

// Return the command content as the tool result
pushToolResult(result)
Expand Down