-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add optional mode parameter for slash commands #6820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| # Implementation Plan: Add Optional Mode Parameter for Slash Commands | ||
|
|
||
| ## Overview | ||
|
|
||
| Add support for an optional `mode` parameter in slash command markdown files that will automatically trigger a mode switch when the slash command is executed. | ||
|
|
||
| ## Current Architecture Understanding | ||
|
|
||
| ### 1. Command System | ||
|
|
||
| - Commands are stored as markdown files in `.roo/commands/` directory | ||
| - Commands support frontmatter with `description` and `argument-hint` fields | ||
| - Commands are loaded by `src/services/command/commands.ts` | ||
| - Command interface is defined in `src/services/command/commands.ts` | ||
|
|
||
| ### 2. Slash Command Flow | ||
|
|
||
| - User types `/command` in the chat | ||
| - `ChatTextArea` component shows autocomplete menu with available commands | ||
| - When selected, the command text is inserted into the input | ||
| - Commands are processed when the message is sent | ||
|
|
||
| ### 3. Mode Switching | ||
|
|
||
| - Modes can be switched via the mode selector dropdown | ||
| - Mode switching sends a `mode` message to the backend via `vscode.postMessage` | ||
| - The `setMode` function updates the current mode state | ||
|
|
||
| ## Implementation Steps | ||
|
|
||
| ### Step 1: Update Command Interface | ||
|
|
||
| **File:** `src/services/command/commands.ts` | ||
|
|
||
| - Add optional `mode?: string` field to the `Command` interface | ||
| - Update the frontmatter parsing to extract the `mode` field | ||
|
|
||
| ### Step 2: Update Command Loading | ||
|
|
||
| **File:** `src/services/command/commands.ts` | ||
|
|
||
| - Modify `scanCommandDirectory` and `tryLoadCommand` functions | ||
| - Parse the `mode` field from frontmatter (similar to `description` and `argument-hint`) | ||
|
|
||
| ### Step 3: Update Frontend Command Handling | ||
|
|
||
| **File:** `webview-ui/src/components/chat/ChatTextArea.tsx` | ||
|
|
||
| - Modify the `handleMentionSelect` function for `ContextMenuOptionType.Command` | ||
| - Check if the selected command has a `mode` property | ||
| - If it does, trigger mode switch before inserting the command | ||
|
|
||
| ### Step 4: Pass Mode Information to Frontend | ||
|
|
||
| **File:** `src/core/webview/webviewMessageHandler.ts` | ||
|
|
||
| - Update the command list sent to frontend to include the `mode` field | ||
| - Ensure the `Command` type in `src/shared/ExtensionMessage.ts` includes the mode field | ||
|
|
||
| ### Step 5: Update Context Menu | ||
|
|
||
| **File:** `webview-ui/src/utils/context-mentions.ts` | ||
|
|
||
| - Ensure the command's mode is passed through when creating menu options | ||
| - Update the `ContextMenuQueryItem` type if needed | ||
|
|
||
| ## Example Usage | ||
|
|
||
| A command markdown file with mode specification: | ||
|
|
||
| ```markdown | ||
| --- | ||
| description: Deploy the application to production | ||
| argument-hint: <environment> | ||
| mode: architect | ||
| --- | ||
|
|
||
| # Deploy Command | ||
|
|
||
| This command helps you deploy the application... | ||
| ``` | ||
|
|
||
| When this command is selected: | ||
|
|
||
| 1. The mode automatically switches to "architect" | ||
| 2. The command `/deploy` is inserted into the input | ||
| 3. The user can continue typing arguments | ||
|
|
||
| ## Testing Requirements | ||
|
|
||
| 1. **Unit Tests:** | ||
|
|
||
| - Test command loading with mode parameter | ||
| - Test command loading without mode parameter (backward compatibility) | ||
| - Test mode switching when command is selected | ||
|
|
||
| 2. **Integration Tests:** | ||
| - Test full flow from command selection to mode switch | ||
| - Test that commands without mode don't trigger mode switch | ||
| - Test that invalid mode values are handled gracefully | ||
|
|
||
| ## Backward Compatibility | ||
|
|
||
| - Commands without the `mode` field should work as before | ||
| - Existing command files don't need to be updated | ||
| - The feature is entirely optional | ||
|
|
||
| ## Benefits | ||
|
|
||
| 1. **Improved UX:** Users don't need to manually switch modes for mode-specific commands | ||
| 2. **Workflow Optimization:** Commands can be pre-configured for the most appropriate mode | ||
| 3. **Discoverability:** Users learn which modes are best for which commands | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2422,6 +2422,7 @@ export const webviewMessageHandler = async ( | |
| filePath: command.filePath, | ||
| description: command.description, | ||
| argumentHint: command.argumentHint, | ||
| mode: command.mode, | ||
| })) | ||
|
|
||
| await provider.postMessageToWebview({ | ||
|
|
@@ -2581,6 +2582,8 @@ export const webviewMessageHandler = async ( | |
| source: command.source, | ||
| filePath: command.filePath, | ||
| description: command.description, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this intentional? The |
||
| argumentHint: command.argumentHint, | ||
| mode: command.mode, | ||
| })) | ||
| await provider.postMessageToWebview({ | ||
| type: "commands", | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,246 @@ | ||
| import { describe, it, expect, vi, beforeEach } from "vitest" | ||
| import fs from "fs/promises" | ||
| import * as path from "path" | ||
| import { getCommand, getCommands } from "../commands" | ||
|
|
||
| // Mock fs and path modules | ||
| vi.mock("fs/promises") | ||
| vi.mock("../roo-config", () => ({ | ||
| getGlobalRooDirectory: vi.fn(() => "/mock/global/.roo"), | ||
| getProjectRooDirectoryForCwd: vi.fn(() => "/mock/project/.roo"), | ||
| })) | ||
|
|
||
| const mockFs = vi.mocked(fs) | ||
|
|
||
| describe("Command mode parameter", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| describe("getCommand with mode parameter", () => { | ||
| it("should parse mode from frontmatter", async () => { | ||
| const commandContent = `--- | ||
| description: Deploy the application | ||
| argument-hint: <environment> | ||
| mode: architect | ||
| --- | ||
|
|
||
| # Deploy Command | ||
|
|
||
| This command helps you deploy the application.` | ||
|
|
||
| mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) | ||
| mockFs.readFile = vi.fn().mockResolvedValue(commandContent) | ||
|
|
||
| const command = await getCommand("/test/cwd", "deploy") | ||
|
|
||
| expect(command).toBeDefined() | ||
| expect(command?.name).toBe("deploy") | ||
| expect(command?.description).toBe("Deploy the application") | ||
| expect(command?.argumentHint).toBe("<environment>") | ||
| expect(command?.mode).toBe("architect") | ||
| }) | ||
|
|
||
| it("should handle commands without mode parameter", async () => { | ||
| const commandContent = `--- | ||
| description: Test command | ||
| argument-hint: <args> | ||
| --- | ||
|
|
||
| # Test Command | ||
|
|
||
| This is a test command without mode.` | ||
|
|
||
| mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) | ||
| mockFs.readFile = vi.fn().mockResolvedValue(commandContent) | ||
|
|
||
| const command = await getCommand("/test/cwd", "test") | ||
|
|
||
| expect(command).toBeDefined() | ||
| expect(command?.name).toBe("test") | ||
| expect(command?.description).toBe("Test command") | ||
| expect(command?.mode).toBeUndefined() | ||
| }) | ||
|
|
||
| it("should handle commands with empty mode parameter", async () => { | ||
| const commandContent = `--- | ||
| description: Test command | ||
| mode: "" | ||
| --- | ||
|
|
||
| # Test Command | ||
|
|
||
| This is a test command with empty mode.` | ||
|
|
||
| mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) | ||
| mockFs.readFile = vi.fn().mockResolvedValue(commandContent) | ||
|
|
||
| const command = await getCommand("/test/cwd", "test") | ||
|
|
||
| expect(command).toBeDefined() | ||
| expect(command?.mode).toBeUndefined() | ||
| }) | ||
|
|
||
| it("should trim whitespace from mode values", async () => { | ||
| const commandContent = `--- | ||
| description: Test command | ||
| mode: " code " | ||
| --- | ||
|
|
||
| # Test Command | ||
|
|
||
| This is a test command with whitespace in mode.` | ||
|
|
||
| mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) | ||
| mockFs.readFile = vi.fn().mockResolvedValue(commandContent) | ||
|
|
||
| const command = await getCommand("/test/cwd", "test") | ||
|
|
||
| expect(command?.mode).toBe("code") | ||
| }) | ||
|
|
||
| it("should handle non-string mode values", async () => { | ||
| const commandContent = `--- | ||
| description: Test command | ||
| mode: 123 | ||
| --- | ||
|
|
||
| # Test Command | ||
|
|
||
| This is a test command with non-string mode.` | ||
|
|
||
| mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) | ||
| mockFs.readFile = vi.fn().mockResolvedValue(commandContent) | ||
|
|
||
| const command = await getCommand("/test/cwd", "test") | ||
|
|
||
| expect(command?.mode).toBeUndefined() | ||
| }) | ||
| }) | ||
|
|
||
| describe("getCommands with mode parameter", () => { | ||
| it("should include mode parameter in command list", async () => { | ||
| const deployContent = `--- | ||
| description: Deploy the application | ||
| mode: architect | ||
| --- | ||
|
|
||
| # Deploy Command | ||
|
|
||
| Deploy instructions.` | ||
|
|
||
| const testContent = `--- | ||
| description: Test command | ||
| mode: debug | ||
| --- | ||
|
|
||
| # Test Command | ||
|
|
||
| Test instructions.` | ||
|
|
||
| const simpleContent = `--- | ||
| description: Simple command | ||
| --- | ||
|
|
||
| # Simple Command | ||
|
|
||
| Simple instructions without mode.` | ||
|
|
||
| mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) | ||
| mockFs.readdir = vi.fn().mockResolvedValue([ | ||
| { name: "deploy.md", isFile: () => true }, | ||
| { name: "test.md", isFile: () => true }, | ||
| { name: "simple.md", isFile: () => true }, | ||
| ]) | ||
| mockFs.readFile = vi | ||
| .fn() | ||
| .mockResolvedValueOnce(deployContent) | ||
| .mockResolvedValueOnce(testContent) | ||
| .mockResolvedValueOnce(simpleContent) | ||
|
|
||
| const commands = await getCommands("/test/cwd") | ||
|
|
||
| expect(commands).toHaveLength(3) | ||
|
|
||
| const deployCmd = commands.find((c) => c.name === "deploy") | ||
| expect(deployCmd?.mode).toBe("architect") | ||
|
|
||
| const testCmd = commands.find((c) => c.name === "test") | ||
| expect(testCmd?.mode).toBe("debug") | ||
|
|
||
| const simpleCmd = commands.find((c) => c.name === "simple") | ||
| expect(simpleCmd?.mode).toBeUndefined() | ||
| }) | ||
|
|
||
| it("should handle invalid mode values gracefully", async () => { | ||
| const commandContent = `--- | ||
| description: Test command | ||
| mode: [1, 2, 3] | ||
| --- | ||
|
|
||
| # Test Command | ||
|
|
||
| Test content.` | ||
|
|
||
| mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) | ||
| mockFs.readdir = vi.fn().mockResolvedValue([{ name: "test.md", isFile: () => true }]) | ||
| mockFs.readFile = vi.fn().mockResolvedValue(commandContent) | ||
|
|
||
| const commands = await getCommands("/test/cwd") | ||
|
|
||
| expect(commands).toHaveLength(1) | ||
| // Mode should be undefined since it's not a string | ||
| expect(commands[0].mode).toBeUndefined() | ||
| }) | ||
| }) | ||
|
|
||
| describe("Project commands override global commands with mode", () => { | ||
| it("should use project command mode over global command", async () => { | ||
| const projectDeployContent = `--- | ||
| description: Project deploy | ||
| mode: architect | ||
| --- | ||
|
|
||
| # Project Deploy | ||
|
|
||
| Project-specific deploy.` | ||
|
|
||
| const globalDeployContent = `--- | ||
| description: Global deploy | ||
| mode: code | ||
| --- | ||
|
|
||
| # Global Deploy | ||
|
|
||
| Global deploy.` | ||
|
|
||
| mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) | ||
|
|
||
| // Mock readdir for both global and project directories | ||
| mockFs.readdir = vi.fn().mockImplementation((dirPath) => { | ||
| if (dirPath.includes("global")) { | ||
| return Promise.resolve([{ name: "deploy.md", isFile: () => true }]) | ||
| } else { | ||
| return Promise.resolve([{ name: "deploy.md", isFile: () => true }]) | ||
| } | ||
| }) | ||
|
|
||
| // Mock readFile for both global and project files | ||
| mockFs.readFile = vi.fn().mockImplementation((filePath) => { | ||
| if (filePath.includes("global")) { | ||
| return Promise.resolve(globalDeployContent) | ||
| } else { | ||
| return Promise.resolve(projectDeployContent) | ||
| } | ||
| }) | ||
|
|
||
| const commands = await getCommands("/test/cwd") | ||
|
|
||
| // Should only have one deploy command (project overrides global) | ||
| const deployCommands = commands.filter((c) => c.name === "deploy") | ||
| expect(deployCommands).toHaveLength(1) | ||
| expect(deployCommands[0].mode).toBe("architect") | ||
| expect(deployCommands[0].source).toBe("project") | ||
| }) | ||
| }) | ||
| }) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that the implementation is complete, should this planning document be removed or moved to documentation? It provides good context but might not need to be in the main codebase.