Skip interpolation for non-existent slash commands (#6475)

Co-authored-by: Roo Code <roomote@roocode.com>
Co-authored-by: Matt Rubens <mrubens@users.noreply.github.com>
This commit is contained in:
roomote[bot] 2025-07-31 01:21:19 -04:00 committed by GitHub
parent 2f8fd889a5
commit e13083e532
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 104 additions and 31 deletions

View file

@ -62,16 +62,31 @@ describe("Command Mentions", () => {
})
it("should handle multiple commands in message", async () => {
const setupContent = "# Setup Environment\n\nRun the following commands:\n```bash\nnpm install\n```"
const deployContent = "# Deploy Environment\n\nRun the following commands:\n```bash\nnpm run deploy\n```"
mockGetCommand
.mockResolvedValueOnce({
name: "setup",
content: "# Setup instructions",
content: setupContent,
source: "project",
filePath: "/project/.roo/commands/setup.md",
})
.mockResolvedValueOnce({
name: "deploy",
content: "# Deploy instructions",
content: deployContent,
source: "project",
filePath: "/project/.roo/commands/deploy.md",
})
.mockResolvedValueOnce({
name: "setup",
content: setupContent,
source: "project",
filePath: "/project/.roo/commands/setup.md",
})
.mockResolvedValueOnce({
name: "deploy",
content: deployContent,
source: "project",
filePath: "/project/.roo/commands/deploy.md",
})
@ -82,33 +97,55 @@ describe("Command Mentions", () => {
expect(mockGetCommand).toHaveBeenCalledWith("/test/cwd", "setup")
expect(mockGetCommand).toHaveBeenCalledWith("/test/cwd", "deploy")
expect(mockGetCommand).toHaveBeenCalledTimes(2) // Both commands called
expect(mockGetCommand).toHaveBeenCalledTimes(2) // Each unique command called once (optimized)
expect(result).toContain('<command name="setup">')
expect(result).toContain("# Setup instructions")
expect(result).toContain("# Setup Environment")
expect(result).toContain('<command name="deploy">')
expect(result).toContain("# Deploy instructions")
expect(result).toContain("# Deploy Environment")
})
it("should handle non-existent command gracefully", async () => {
it("should leave non-existent commands unchanged", async () => {
mockGetCommand.mockReset()
mockGetCommand.mockResolvedValue(undefined)
const input = "/nonexistent command"
const result = await callParseMentions(input)
expect(mockGetCommand).toHaveBeenCalledWith("/test/cwd", "nonexistent")
expect(result).toContain('<command name="nonexistent">')
expect(result).toContain("Command 'nonexistent' not found")
expect(result).toContain("</command>")
// The command should remain unchanged in the text
expect(result).toBe("/nonexistent command")
// Should not contain any command tags
expect(result).not.toContain('<command name="nonexistent">')
expect(result).not.toContain("Command 'nonexistent' not found")
})
it("should handle command loading errors", async () => {
it("should handle command loading errors during existence check", async () => {
mockGetCommand.mockReset()
mockGetCommand.mockRejectedValue(new Error("Failed to load command"))
const input = "/error-command test"
const result = await callParseMentions(input)
// When getCommand throws an error during existence check,
// the command is treated as non-existent and left unchanged
expect(result).toBe("/error-command test")
expect(result).not.toContain('<command name="error-command">')
})
it("should handle command loading errors during processing", async () => {
// With optimization, command is loaded once and cached
mockGetCommand.mockResolvedValue({
name: "error-command",
content: "# Error command",
source: "project",
filePath: "/project/.roo/commands/error-command.md",
})
const input = "/error-command test"
const result = await callParseMentions(input)
expect(result).toContain('<command name="error-command">')
expect(result).toContain("Error loading command")
expect(result).toContain("# Error command")
expect(result).toContain("</command>")
})
@ -246,13 +283,29 @@ npm install
})
describe("command mention text transformation", () => {
it("should transform command mentions at start of message", async () => {
it("should transform existing command mentions at start of message", async () => {
mockGetCommand.mockResolvedValue({
name: "setup",
content: "# Setup instructions",
source: "project",
filePath: "/project/.roo/commands/setup.md",
})
const input = "/setup the project"
const result = await callParseMentions(input)
expect(result).toContain("Command 'setup' (see below for command content)")
})
it("should leave non-existent command mentions unchanged", async () => {
mockGetCommand.mockResolvedValue(undefined)
const input = "/nonexistent the project"
const result = await callParseMentions(input)
expect(result).toBe("/nonexistent the project")
})
it("should process multiple commands in message", async () => {
mockGetCommand
.mockResolvedValueOnce({

View file

@ -18,7 +18,7 @@ import { UrlContentFetcher } from "../../services/browser/UrlContentFetcher"
import { FileContextTracker } from "../context-tracking/FileContextTracker"
import { RooIgnoreController } from "../ignore/RooIgnoreController"
import { getCommand } from "../../services/command/commands"
import { getCommand, type Command } from "../../services/command/commands"
import { t } from "../../i18n"
@ -86,13 +86,38 @@ export async function parseMentions(
maxReadFileLine?: number,
): Promise<string> {
const mentions: Set<string> = new Set()
const commandMentions: Set<string> = new Set()
const validCommands: Map<string, Command> = new Map()
// First pass: extract command mentions (starting with /)
let parsedText = text.replace(commandRegexGlobal, (match, commandName) => {
commandMentions.add(commandName)
return `Command '${commandName}' (see below for command content)`
})
// First pass: check which command mentions exist and cache the results
const commandMatches = Array.from(text.matchAll(commandRegexGlobal))
const uniqueCommandNames = new Set(commandMatches.map(([, commandName]) => commandName))
const commandExistenceChecks = await Promise.all(
Array.from(uniqueCommandNames).map(async (commandName) => {
try {
const command = await getCommand(cwd, commandName)
return { commandName, command }
} catch (error) {
// If there's an error checking command existence, treat it as non-existent
return { commandName, command: undefined }
}
}),
)
// Store valid commands for later use
for (const { commandName, command } of commandExistenceChecks) {
if (command) {
validCommands.set(commandName, command)
}
}
// Only replace text for commands that actually exist
let parsedText = text
for (const [match, commandName] of commandMatches) {
if (validCommands.has(commandName)) {
parsedText = parsedText.replace(match, `Command '${commandName}' (see below for command content)`)
}
}
// Second pass: handle regular mentions
parsedText = parsedText.replace(mentionRegexGlobal, (match, mention) => {
@ -213,20 +238,15 @@ export async function parseMentions(
}
}
// Process command mentions
for (const commandName of commandMentions) {
// Process valid command mentions using cached results
for (const [commandName, command] of validCommands) {
try {
const command = await getCommand(cwd, commandName)
if (command) {
let commandOutput = ""
if (command.description) {
commandOutput += `Description: ${command.description}\n\n`
}
commandOutput += command.content
parsedText += `\n\n<command name="${commandName}">\n${commandOutput}\n</command>`
} else {
parsedText += `\n\n<command name="${commandName}">\nCommand '${commandName}' not found. Available commands can be found in .roo/commands/ or ~/.roo/commands/\n</command>`
let commandOutput = ""
if (command.description) {
commandOutput += `Description: ${command.description}\n\n`
}
commandOutput += command.content
parsedText += `\n\n<command name="${commandName}">\n${commandOutput}\n</command>`
} catch (error) {
parsedText += `\n\n<command name="${commandName}">\nError loading command '${commandName}': ${error.message}\n</command>`
}