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

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: Matt Rubens <[email protected]>

Author: 3 people

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('<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({

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<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>`
 		}