diff --git a/src/__tests__/command-mentions.spec.ts b/src/__tests__/command-mentions.spec.ts index d4de0bbba7..b120f3720c 100644 --- a/src/__tests__/command-mentions.spec.ts +++ b/src/__tests__/command-mentions.spec.ts @@ -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('') - expect(result).toContain("# Setup instructions") + expect(result).toContain("# Setup Environment") expect(result).toContain('') - 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('') - expect(result).toContain("Command 'nonexistent' not found") - expect(result).toContain("") + // The command should remain unchanged in the text + expect(result).toBe("/nonexistent command") + // Should not contain any command tags + expect(result).not.toContain('') + 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('') + }) + + 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('') - expect(result).toContain("Error loading command") + expect(result).toContain("# Error command") expect(result).toContain("") }) @@ -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({ diff --git a/src/core/mentions/index.ts b/src/core/mentions/index.ts index b6a9dd4d0d..ed3060859b 100644 --- a/src/core/mentions/index.ts +++ b/src/core/mentions/index.ts @@ -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 { const mentions: Set = new Set() - const commandMentions: Set = new Set() + const validCommands: Map = 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\n${commandOutput}\n` - } else { - parsedText += `\n\n\nCommand '${commandName}' not found. Available commands can be found in .roo/commands/ or ~/.roo/commands/\n` + let commandOutput = "" + if (command.description) { + commandOutput += `Description: ${command.description}\n\n` } + commandOutput += command.content + parsedText += `\n\n\n${commandOutput}\n` } catch (error) { parsedText += `\n\n\nError loading command '${commandName}': ${error.message}\n` }