Skip to content
Merged
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
77 changes: 65 additions & 12 deletions src/__tests__/command-mentions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
})
Expand All @@ -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>")
})

Expand Down Expand Up @@ -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({
Expand Down
58 changes: 39 additions & 19 deletions src/core/mentions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Error handling inconsistency: If getCommand() fails here after passing the existence check, we show an error message. This creates a scenario where a 'verified existing' command can still fail. Is this the intended behavior?

parsedText += `\n\n<command name="${commandName}">\nError loading command '${commandName}': ${error.message}\n</command>`
}
Expand Down