Skip to content

Commit 8d9973c

Browse files
committed
feat: skip interpolation for non-existent slash commands
- Modified parseMentions to check command existence before text replacement - Non-existent commands are now left unchanged in the original text - Updated tests to reflect new behavior where non-existent commands remain as-is - Added error handling for command existence checks
1 parent 2f8fd88 commit 8d9973c

File tree

2 files changed

+91
-21
lines changed

2 files changed

+91
-21
lines changed

src/__tests__/command-mentions.spec.ts

Lines changed: 64 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,31 @@ describe("Command Mentions", () => {
6262
})
6363

6464
it("should handle multiple commands in message", async () => {
65+
const setupContent = "# Setup Environment\n\nRun the following commands:\n```bash\nnpm install\n```"
66+
const deployContent = "# Deploy Environment\n\nRun the following commands:\n```bash\nnpm run deploy\n```"
67+
6568
mockGetCommand
6669
.mockResolvedValueOnce({
6770
name: "setup",
68-
content: "# Setup instructions",
71+
content: setupContent,
6972
source: "project",
7073
filePath: "/project/.roo/commands/setup.md",
7174
})
7275
.mockResolvedValueOnce({
7376
name: "deploy",
74-
content: "# Deploy instructions",
77+
content: deployContent,
78+
source: "project",
79+
filePath: "/project/.roo/commands/deploy.md",
80+
})
81+
.mockResolvedValueOnce({
82+
name: "setup",
83+
content: setupContent,
84+
source: "project",
85+
filePath: "/project/.roo/commands/setup.md",
86+
})
87+
.mockResolvedValueOnce({
88+
name: "deploy",
89+
content: deployContent,
7590
source: "project",
7691
filePath: "/project/.roo/commands/deploy.md",
7792
})
@@ -82,31 +97,53 @@ describe("Command Mentions", () => {
8297

8398
expect(mockGetCommand).toHaveBeenCalledWith("/test/cwd", "setup")
8499
expect(mockGetCommand).toHaveBeenCalledWith("/test/cwd", "deploy")
85-
expect(mockGetCommand).toHaveBeenCalledTimes(2) // Both commands called
100+
expect(mockGetCommand).toHaveBeenCalledTimes(4) // Each command called twice (existence check + processing)
86101
expect(result).toContain('<command name="setup">')
87-
expect(result).toContain("# Setup instructions")
102+
expect(result).toContain("# Setup Environment")
88103
expect(result).toContain('<command name="deploy">')
89-
expect(result).toContain("# Deploy instructions")
104+
expect(result).toContain("# Deploy Environment")
90105
})
91106

92-
it("should handle non-existent command gracefully", async () => {
107+
it("should leave non-existent commands unchanged", async () => {
93108
mockGetCommand.mockResolvedValue(undefined)
94109

95110
const input = "/nonexistent command"
96111
const result = await callParseMentions(input)
97112

98113
expect(mockGetCommand).toHaveBeenCalledWith("/test/cwd", "nonexistent")
99-
expect(result).toContain('<command name="nonexistent">')
100-
expect(result).toContain("Command 'nonexistent' not found")
101-
expect(result).toContain("</command>")
114+
// The command should remain unchanged in the text
115+
expect(result).toBe("/nonexistent command")
116+
// Should not contain any command tags
117+
expect(result).not.toContain('<command name="nonexistent">')
118+
expect(result).not.toContain("Command 'nonexistent' not found")
102119
})
103120

104-
it("should handle command loading errors", async () => {
121+
it("should handle command loading errors during existence check", async () => {
105122
mockGetCommand.mockRejectedValue(new Error("Failed to load command"))
106123

107124
const input = "/error-command test"
108125
const result = await callParseMentions(input)
109126

127+
// When getCommand throws an error during existence check,
128+
// the command is treated as non-existent and left unchanged
129+
expect(result).toBe("/error-command test")
130+
expect(result).not.toContain('<command name="error-command">')
131+
})
132+
133+
it("should handle command loading errors during processing", async () => {
134+
// First call (existence check) succeeds, second call (processing) fails
135+
mockGetCommand
136+
.mockResolvedValueOnce({
137+
name: "error-command",
138+
content: "# Error command",
139+
source: "project",
140+
filePath: "/project/.roo/commands/error-command.md",
141+
})
142+
.mockRejectedValueOnce(new Error("Failed to load command"))
143+
144+
const input = "/error-command test"
145+
const result = await callParseMentions(input)
146+
110147
expect(result).toContain('<command name="error-command">')
111148
expect(result).toContain("Error loading command")
112149
expect(result).toContain("</command>")
@@ -246,13 +283,29 @@ npm install
246283
})
247284

248285
describe("command mention text transformation", () => {
249-
it("should transform command mentions at start of message", async () => {
286+
it("should transform existing command mentions at start of message", async () => {
287+
mockGetCommand.mockResolvedValue({
288+
name: "setup",
289+
content: "# Setup instructions",
290+
source: "project",
291+
filePath: "/project/.roo/commands/setup.md",
292+
})
293+
250294
const input = "/setup the project"
251295
const result = await callParseMentions(input)
252296

253297
expect(result).toContain("Command 'setup' (see below for command content)")
254298
})
255299

300+
it("should leave non-existent command mentions unchanged", async () => {
301+
mockGetCommand.mockResolvedValue(undefined)
302+
303+
const input = "/nonexistent the project"
304+
const result = await callParseMentions(input)
305+
306+
expect(result).toBe("/nonexistent the project")
307+
})
308+
256309
it("should process multiple commands in message", async () => {
257310
mockGetCommand
258311
.mockResolvedValueOnce({

src/core/mentions/index.ts

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,30 @@ export async function parseMentions(
8686
maxReadFileLine?: number,
8787
): Promise<string> {
8888
const mentions: Set<string> = new Set()
89-
const commandMentions: Set<string> = new Set()
89+
const validCommandMentions: Set<string> = new Set()
9090

91-
// First pass: extract command mentions (starting with /)
92-
let parsedText = text.replace(commandRegexGlobal, (match, commandName) => {
93-
commandMentions.add(commandName)
94-
return `Command '${commandName}' (see below for command content)`
95-
})
91+
// First pass: check which command mentions exist before replacing text
92+
const commandMatches = Array.from(text.matchAll(commandRegexGlobal))
93+
const commandExistenceChecks = await Promise.all(
94+
commandMatches.map(async ([match, commandName]) => {
95+
try {
96+
const command = await getCommand(cwd, commandName)
97+
return { match, commandName, exists: !!command }
98+
} catch (error) {
99+
// If there's an error checking command existence, treat it as non-existent
100+
return { match, commandName, exists: false }
101+
}
102+
}),
103+
)
104+
105+
// Only replace text for commands that actually exist
106+
let parsedText = text
107+
for (const { match, commandName, exists } of commandExistenceChecks) {
108+
if (exists) {
109+
validCommandMentions.add(commandName)
110+
parsedText = parsedText.replace(match, `Command '${commandName}' (see below for command content)`)
111+
}
112+
}
96113

97114
// Second pass: handle regular mentions
98115
parsedText = parsedText.replace(mentionRegexGlobal, (match, mention) => {
@@ -213,8 +230,8 @@ export async function parseMentions(
213230
}
214231
}
215232

216-
// Process command mentions
217-
for (const commandName of commandMentions) {
233+
// Process valid command mentions only
234+
for (const commandName of validCommandMentions) {
218235
try {
219236
const command = await getCommand(cwd, commandName)
220237
if (command) {
@@ -224,9 +241,9 @@ export async function parseMentions(
224241
}
225242
commandOutput += command.content
226243
parsedText += `\n\n<command name="${commandName}">\n${commandOutput}\n</command>`
227-
} else {
228-
parsedText += `\n\n<command name="${commandName}">\nCommand '${commandName}' not found. Available commands can be found in .roo/commands/ or ~/.roo/commands/\n</command>`
229244
}
245+
// Note: We don't add error messages for non-existent commands anymore
246+
// since we only process commands that we've already verified exist
230247
} catch (error) {
231248
parsedText += `\n\n<command name="${commandName}">\nError loading command '${commandName}': ${error.message}\n</command>`
232249
}

0 commit comments

Comments
 (0)