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
1 change: 1 addition & 0 deletions .roo/commands/release.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
description: "Create a new release of the Roo Code extension"
argument-hint: patch | minor | major
mode: code
---

1. Identify the SHA corresponding to the most recent release using GitHub CLI: `gh release view --json tagName,targetCommitish,publishedAt`
Expand Down
4 changes: 3 additions & 1 deletion src/__tests__/command-mentions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe("Command Mentions", () => {

// Helper function to call parseMentions with required parameters
const callParseMentions = async (text: string) => {
return await parseMentions(
const result = await parseMentions(
text,
"/test/cwd", // cwd
mockUrlContentFetcher, // urlContentFetcher
Expand All @@ -38,6 +38,8 @@ describe("Command Mentions", () => {
50, // maxDiagnosticMessages
undefined, // maxReadFileLine
)
// Return just the text for backward compatibility with existing tests
return result.text
}

describe("parseMentions with command support", () => {
Expand Down
32 changes: 16 additions & 16 deletions src/core/mentions/__tests__/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe("parseMentions - URL error handling", () => {

expect(consoleErrorSpy).toHaveBeenCalledWith("Error fetching URL https://example.com:", timeoutError)
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: Navigation timeout of 30000 ms exceeded")
expect(result.text).toContain("Error fetching content: Navigation timeout of 30000 ms exceeded")
})

it("should handle DNS resolution errors", async () => {
Expand All @@ -50,7 +50,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://nonexistent.example", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: net::ERR_NAME_NOT_RESOLVED")
expect(result.text).toContain("Error fetching content: net::ERR_NAME_NOT_RESOLVED")
})

it("should handle network disconnection errors", async () => {
Expand All @@ -60,7 +60,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: net::ERR_INTERNET_DISCONNECTED")
expect(result.text).toContain("Error fetching content: net::ERR_INTERNET_DISCONNECTED")
})

it("should handle 403 Forbidden errors", async () => {
Expand All @@ -70,7 +70,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: 403 Forbidden")
expect(result.text).toContain("Error fetching content: 403 Forbidden")
})

it("should handle 404 Not Found errors", async () => {
Expand All @@ -80,7 +80,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com/missing", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: 404 Not Found")
expect(result.text).toContain("Error fetching content: 404 Not Found")
})

it("should handle generic errors with fallback message", async () => {
Expand All @@ -90,7 +90,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: Some unexpected error")
expect(result.text).toContain("Error fetching content: Some unexpected error")
})

it("should handle non-Error objects thrown", async () => {
Expand All @@ -100,7 +100,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content:")
expect(result.text).toContain("Error fetching content:")
})

it("should handle browser launch errors correctly", async () => {
Expand All @@ -112,7 +112,7 @@ describe("parseMentions - URL error handling", () => {
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
"Error fetching content for https://example.com: Failed to launch browser",
)
expect(result).toContain("Error fetching content: Failed to launch browser")
expect(result.text).toContain("Error fetching content: Failed to launch browser")
// Should not attempt to fetch URL if browser launch failed
expect(mockUrlContentFetcher.urlToMarkdown).not.toHaveBeenCalled()
})
Expand All @@ -126,7 +126,7 @@ describe("parseMentions - URL error handling", () => {
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
"Error fetching content for https://example.com: String error",
)
expect(result).toContain("Error fetching content: String error")
expect(result.text).toContain("Error fetching content: String error")
})

it("should successfully fetch URL content when no errors occur", async () => {
Expand All @@ -135,9 +135,9 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).not.toHaveBeenCalled()
expect(result).toContain('<url_content url="https://example.com">')
expect(result).toContain("# Example Content\n\nThis is the content.")
expect(result).toContain("</url_content>")
expect(result.text).toContain('<url_content url="https://example.com">')
expect(result.text).toContain("# Example Content\n\nThis is the content.")
expect(result.text).toContain("</url_content>")
})

it("should handle multiple URLs with mixed success and failure", async () => {
Expand All @@ -151,9 +151,9 @@ describe("parseMentions - URL error handling", () => {
mockUrlContentFetcher,
)

expect(result).toContain('<url_content url="https://example1.com">')
expect(result).toContain("# First Site")
expect(result).toContain('<url_content url="https://example2.com">')
expect(result).toContain("Error fetching content: timeout")
expect(result.text).toContain('<url_content url="https://example1.com">')
expect(result.text).toContain("# First Site")
expect(result.text).toContain('<url_content url="https://example2.com">')
expect(result.text).toContain("Error fetching content: timeout")
})
})
31 changes: 20 additions & 11 deletions src/core/mentions/__tests__/processUserContentMentions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ describe("processUserContentMentions", () => {
mockFileContextTracker = {} as FileContextTracker
mockRooIgnoreController = {}

// Default mock implementation
vi.mocked(parseMentions).mockImplementation(async (text) => `parsed: ${text}`)
// Default mock implementation - returns ParseMentionsResult object
vi.mocked(parseMentions).mockImplementation(async (text) => ({
text: `parsed: ${text}`,
mode: undefined,
}))
})

describe("maxReadFileLine parameter", () => {
Expand Down Expand Up @@ -134,10 +137,11 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).toHaveBeenCalled()
expect(result[0]).toEqual({
expect(result.content[0]).toEqual({
type: "text",
text: "parsed: <task>Do something</task>",
})
expect(result.mode).toBeUndefined()
})

it("should process text blocks with <feedback> tags", async () => {
Expand All @@ -156,10 +160,11 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).toHaveBeenCalled()
expect(result[0]).toEqual({
expect(result.content[0]).toEqual({
type: "text",
text: "parsed: <feedback>Fix this issue</feedback>",
})
expect(result.mode).toBeUndefined()
})

it("should not process text blocks without task or feedback tags", async () => {
Expand All @@ -178,7 +183,8 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).not.toHaveBeenCalled()
expect(result[0]).toEqual(userContent[0])
expect(result.content[0]).toEqual(userContent[0])
expect(result.mode).toBeUndefined()
})

it("should process tool_result blocks with string content", async () => {
Expand All @@ -198,11 +204,12 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).toHaveBeenCalled()
expect(result[0]).toEqual({
expect(result.content[0]).toEqual({
type: "tool_result",
tool_use_id: "123",
content: "parsed: <feedback>Tool feedback</feedback>",
})
expect(result.mode).toBeUndefined()
})

it("should process tool_result blocks with array content", async () => {
Expand Down Expand Up @@ -231,7 +238,7 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).toHaveBeenCalledTimes(1)
expect(result[0]).toEqual({
expect(result.content[0]).toEqual({
type: "tool_result",
tool_use_id: "123",
content: [
Expand All @@ -245,6 +252,7 @@ describe("processUserContentMentions", () => {
},
],
})
expect(result.mode).toBeUndefined()
})

it("should handle mixed content types", async () => {
Expand Down Expand Up @@ -277,17 +285,18 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).toHaveBeenCalledTimes(2)
expect(result).toHaveLength(3)
expect(result[0]).toEqual({
expect(result.content).toHaveLength(3)
expect(result.content[0]).toEqual({
type: "text",
text: "parsed: <task>First task</task>",
})
expect(result[1]).toEqual(userContent[1]) // Image block unchanged
expect(result[2]).toEqual({
expect(result.content[1]).toEqual(userContent[1]) // Image block unchanged
expect(result.content[2]).toEqual({
type: "tool_result",
tool_use_id: "456",
content: "parsed: <feedback>Feedback</feedback>",
})
expect(result.mode).toBeUndefined()
})
})

Expand Down
17 changes: 14 additions & 3 deletions src/core/mentions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ export async function openMention(cwd: string, mention?: string): Promise<void>
}
}

export interface ParseMentionsResult {
text: string
mode?: string // Mode from the first slash command that has one
}

export async function parseMentions(
text: string,
cwd: string,
Expand All @@ -81,9 +86,10 @@ export async function parseMentions(
includeDiagnosticMessages: boolean = true,
maxDiagnosticMessages: number = 50,
maxReadFileLine?: number,
): Promise<string> {
): Promise<ParseMentionsResult> {
const mentions: Set<string> = new Set()
const validCommands: Map<string, Command> = new Map()
let commandMode: string | undefined // Track mode from the first slash command that has one

// First pass: check which command mentions exist and cache the results
const commandMatches = Array.from(text.matchAll(commandRegexGlobal))
Expand All @@ -101,10 +107,14 @@ export async function parseMentions(
}),
)

// Store valid commands for later use
// Store valid commands for later use and capture the first mode found
for (const { commandName, command } of commandExistenceChecks) {
if (command) {
validCommands.set(commandName, command)
// Capture the mode from the first command that has one
if (!commandMode && command.mode) {
commandMode = command.mode
}
}
}

Expand Down Expand Up @@ -257,7 +267,7 @@ export async function parseMentions(
}
}

return parsedText
return { text: parsedText, mode: commandMode }
}

async function getFileOrFolderContent(
Expand Down Expand Up @@ -410,3 +420,4 @@ export async function getLatestTerminalOutput(): Promise<string> {

// Export processUserContentMentions from its own file
export { processUserContentMentions } from "./processUserContentMentions"
export type { ProcessUserContentMentionsResult } from "./processUserContentMentions"
Loading
Loading