Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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 .tmp/review/Roo-Code
Submodule Roo-Code added at 8dbd8c
1 change: 1 addition & 0 deletions .work/reviews/Roo-Code
Submodule Roo-Code added at ea8420
166 changes: 51 additions & 115 deletions packages/types/src/api.ts
Original file line number Diff line number Diff line change
@@ -1,144 +1,80 @@
import type { EventEmitter } from "events"
import type { Socket } from "net"
/**
* API for programmatic MCP server operations
*/

import type { EventEmitter } from "events"
import type { RooCodeEvents } from "./events.js"
import type { RooCodeSettings } from "./global-settings.js"
import type { ProviderSettingsEntry, ProviderSettings } from "./provider-settings.js"
import type { IpcMessage, IpcServerEvents } from "./ipc.js"
import type { RooCodeSettings, ProviderSettings, ProviderSettingsEntry } from "./index.js"

export type RooCodeAPIEvents = RooCodeEvents

export interface RooCodeAPI extends EventEmitter<RooCodeAPIEvents> {
/**
* Interface for MCP operations that can be accessed programmatically
*/
export interface McpApi {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

P0 (Critical): Cross-extension global instance will not work. VS Code loads each extension in an isolated host/bundle; other extensions importing @roo-code/types get a different module instance, so getMcpApi() remains undefined. Prefer a stable API via extension exports (vscode.extensions.getExtension('RooVeterinaryInc.roo-cline')?.exports) or a command-based API (vscode.commands.executeCommand('roo-cline.refreshMcpServers')). The new command already enables programmatic triggering; consider removing the global or documenting it as internal-only.

/**
* Starts a new task with an optional initial message and images.
* @param task Optional initial task message.
* @param images Optional array of image data URIs (e.g., "data:image/webp;base64,...").
* @returns The ID of the new task.
* Refresh all MCP server connections
* @returns Promise that resolves when refresh is complete
*/
startNewTask({
configuration,
text,
images,
newTab,
}: {
configuration?: RooCodeSettings
refreshMcpServers(): Promise<void>
}

/**
* Main API interface for Roo Code extension
*/
export interface RooCodeAPI extends EventEmitter<RooCodeEvents>, McpApi {
// Task Management
startNewTask(options: {
configuration: RooCodeSettings
text?: string
images?: string[]
newTab?: boolean
}): Promise<string>
/**
* Resumes a task with the given ID.
* @param taskId The ID of the task to resume.
* @throws Error if the task is not found in the task history.
*/
resumeTask(taskId: string): Promise<void>
/**
* Checks if a task with the given ID is in the task history.
* @param taskId The ID of the task to check.
* @returns True if the task is in the task history, false otherwise.
*/
isTaskInHistory(taskId: string): Promise<boolean>
/**
* Returns the current task stack.
* @returns An array of task IDs.
*/
getCurrentTaskStack(): string[]
/**
* Clears the current task.
*/
getCurrentTaskStack(): unknown
clearCurrentTask(lastMessage?: string): Promise<void>
/**
* Cancels the current task.
*/
cancelCurrentTask(): Promise<void>
/**
* Sends a message to the current task.
* @param message Optional message to send.
* @param images Optional array of image data URIs (e.g., "data:image/webp;base64,...").
*/
sendMessage(message?: string, images?: string[]): Promise<void>
/**
* Simulates pressing the primary button in the chat interface.
*/
cancelTask(taskId: string): Promise<void>

// Messaging
sendMessage(text?: string, images?: string[]): Promise<void>
pressPrimaryButton(): Promise<void>
/**
* Simulates pressing the secondary button in the chat interface.
*/
pressSecondaryButton(): Promise<void>
/**
* Returns true if the API is ready to use.
*/

// State
isReady(): boolean
/**
* Returns the current configuration.
* @returns The current configuration.
*/

// Configuration
getConfiguration(): RooCodeSettings
/**
* Sets the configuration for the current task.
* @param values An object containing key-value pairs to set.
*/
setConfiguration(values: RooCodeSettings): Promise<void>
/**
* Returns a list of all configured profile names
* @returns Array of profile names
*/

// Profile Management
getProfiles(): string[]
/**
* Returns the profile entry for a given name
* @param name The name of the profile
* @returns The profile entry, or undefined if the profile does not exist
*/
getProfileEntry(name: string): ProviderSettingsEntry | undefined
/**
* Creates a new API configuration profile
* @param name The name of the profile
* @param profile The profile to create; defaults to an empty object
* @param activate Whether to activate the profile after creation; defaults to true
* @returns The ID of the created profile
* @throws Error if the profile already exists
*/
createProfile(name: string, profile?: ProviderSettings, activate?: boolean): Promise<string>
/**
* Updates an existing API configuration profile
* @param name The name of the profile
* @param profile The profile to update
* @param activate Whether to activate the profile after update; defaults to true
* @returns The ID of the updated profile
* @throws Error if the profile does not exist
*/
updateProfile(name: string, profile: ProviderSettings, activate?: boolean): Promise<string | undefined>
/**
* Creates a new API configuration profile or updates an existing one
* @param name The name of the profile
* @param profile The profile to create or update; defaults to an empty object
* @param activate Whether to activate the profile after upsert; defaults to true
* @returns The ID of the upserted profile
*/
upsertProfile(name: string, profile: ProviderSettings, activate?: boolean): Promise<string | undefined>
/**
* Deletes a profile by name
* @param name The name of the profile to delete
* @throws Error if the profile does not exist
*/
deleteProfile(name: string): Promise<void>
/**
* Returns the name of the currently active profile
* @returns The profile name, or undefined if no profile is active
*/
getActiveProfile(): string | undefined
/**
* Changes the active API configuration profile
* @param name The name of the profile to activate
* @throws Error if the profile does not exist
*/
setActiveProfile(name: string): Promise<string | undefined>
}

export interface RooCodeIpcServer extends EventEmitter<IpcServerEvents> {
listen(): void
broadcast(message: IpcMessage): void
send(client: string | Socket, message: IpcMessage): void
get socketPath(): string
get isListening(): boolean
/**
* Global MCP API instance that will be set by the extension
*/
export let mcpApi: McpApi | undefined
Copy link
Contributor Author

Choose a reason for hiding this comment

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

P1 (High): Breaking change risk in @roo-code/types. This file replaces/removes RooCodeAPI and related exports that external consumers may rely on, which contradicts "No breaking changes" in the PR description. Re-introduce previous exports (deprecated) and add McpApi additively.


/**
* Set the global MCP API instance
* @param api The MCP API implementation
*/
export function setMcpApi(api: McpApi): void {
mcpApi = api
}

/**
* Get the global MCP API instance
* @returns The MCP API instance or undefined if not set
*/
export function getMcpApi(): McpApi | undefined {
return mcpApi
}
12 changes: 12 additions & 0 deletions packages/types/src/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,15 @@ export type IpcServerEvents = {
[IpcMessageType.TaskCommand]: [clientId: string, data: TaskCommand]
[IpcMessageType.TaskEvent]: [relayClientId: string | undefined, data: TaskEvent]
}

/**
* RooCodeIpcServer
*/

export interface RooCodeIpcServer {
socketPath: string
isListening: boolean
listen(): void
broadcast(message: IpcMessage): void
send(client: string | unknown, message: IpcMessage): void
Copy link
Contributor

Choose a reason for hiding this comment

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

The parameter type for client in the send() method is declared as string | unknown, which effectively resolves to unknown since unknown is a top type. Consider specifying a more concrete type (e.g. string) if that is the intended behavior.

Suggested change
send(client: string | unknown, message: IpcMessage): void
send(client: string, message: IpcMessage): void

}
1 change: 1 addition & 0 deletions packages/types/src/vscode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export const commandIds = [
"acceptInput",
"focusPanel",
"toggleAutoApprove",
"refreshMcpServers",
] as const

export type CommandId = (typeof commandIds)[number]
Expand Down
133 changes: 133 additions & 0 deletions src/activate/__tests__/registerCommands.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import { describe, it, expect, vi, beforeEach } from "vitest"
import * as vscode from "vscode"
import { registerCommands } from "../registerCommands"
import { ClineProvider } from "../../core/webview/ClineProvider"

// Mock vscode module
vi.mock("vscode", () => ({
window: {
showInformationMessage: vi.fn(),
showWarningMessage: vi.fn(),
},
commands: {
registerCommand: vi.fn(),
},
}))

describe("registerCommands", () => {
let mockContext: vscode.ExtensionContext
let mockOutputChannel: vscode.OutputChannel
let mockProvider: ClineProvider

beforeEach(() => {
// Reset all mocks
vi.clearAllMocks()

// Create mock objects
mockContext = {
subscriptions: {
push: vi.fn(),
},
} as any

mockOutputChannel = {
appendLine: vi.fn(),
} as any

mockProvider = {
getMcpHub: vi.fn(),
} as any
})

describe("refreshMcpServers command", () => {
it("should refresh MCP servers when hub is available", async () => {
// Arrange
const mockMcpHub = {
refreshAllConnections: vi.fn().mockResolvedValue(undefined),
}
mockProvider.getMcpHub = vi.fn().mockReturnValue(mockMcpHub)

// Mock getVisibleProviderOrLog to return our mock provider
const getVisibleProviderOrLog = vi.fn().mockReturnValue(mockProvider)

// Register commands
const commands = registerCommands({
context: mockContext,
outputChannel: mockOutputChannel,
provider: mockProvider,
})

// Find and execute the refreshMcpServers command
const registerCommandCalls = vi.mocked(vscode.commands.registerCommand).mock.calls
const refreshCommand = registerCommandCalls.find(([cmd]) => cmd === "roo-cline.refreshMcpServers")

expect(refreshCommand).toBeDefined()

// Execute the command callback
const commandCallback = refreshCommand![1] as () => Promise<void>
await commandCallback()

// Assert
expect(mockProvider.getMcpHub).toHaveBeenCalled()
expect(mockMcpHub.refreshAllConnections).toHaveBeenCalled()
expect(vscode.window.showInformationMessage).toHaveBeenCalledWith("MCP servers refreshed successfully")
})

it("should show warning when MCP hub is not available", async () => {
// Arrange
mockProvider.getMcpHub = vi.fn().mockReturnValue(undefined)

// Mock getVisibleProviderOrLog to return our mock provider
const getVisibleProviderOrLog = vi.fn().mockReturnValue(mockProvider)

// Register commands
const commands = registerCommands({
context: mockContext,
outputChannel: mockOutputChannel,
provider: mockProvider,
})

// Find and execute the refreshMcpServers command
const registerCommandCalls = vi.mocked(vscode.commands.registerCommand).mock.calls
const refreshCommand = registerCommandCalls.find(([cmd]) => cmd === "roo-cline.refreshMcpServers")

expect(refreshCommand).toBeDefined()

// Execute the command callback
const commandCallback = refreshCommand![1] as () => Promise<void>
await commandCallback()

// Assert
expect(mockProvider.getMcpHub).toHaveBeenCalled()
expect(vscode.window.showWarningMessage).toHaveBeenCalledWith("MCP hub is not available")
})

it("should not execute when no visible provider is found", async () => {
// Arrange
// Mock getVisibleProviderOrLog to return undefined
const getVisibleProviderOrLog = vi.fn().mockReturnValue(undefined)

// Register commands
const commands = registerCommands({
context: mockContext,
outputChannel: mockOutputChannel,
provider: mockProvider,
})

// Find and execute the refreshMcpServers command
const registerCommandCalls = vi.mocked(vscode.commands.registerCommand).mock.calls
const refreshCommand = registerCommandCalls.find(([cmd]) => cmd === "roo-cline.refreshMcpServers")

expect(refreshCommand).toBeDefined()

// Execute the command callback
const commandCallback = refreshCommand![1] as () => Promise<void>
await commandCallback()

// Assert - should return early without calling getMcpHub
expect(mockProvider.getMcpHub).not.toHaveBeenCalled()
expect(vscode.window.showInformationMessage).not.toHaveBeenCalled()
expect(vscode.window.showWarningMessage).not.toHaveBeenCalled()
})
})
})
15 changes: 15 additions & 0 deletions src/activate/registerCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,21 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt
action: "toggleAutoApprove",
})
},
refreshMcpServers: async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider internationalizing the hard-coded user messages here (e.g. 'MCP servers refreshed successfully' and 'MCP hub is not available') using the translation function (t('...')) to align with the extension’s i18n practices.

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

P2 (Medium): Improve testability/reliability. The handler ignores the injected provider and bails when no visible provider is active (common in tests/headless). Consider falling back to the injected provider, e.g., const p = getVisibleProviderOrLog(outputChannel) ?? provider; then use p.getMcpHub().

const visibleProvider = getVisibleProviderOrLog(outputChannel)

if (!visibleProvider) {
return
}

const mcpHub = visibleProvider.getMcpHub()
if (mcpHub) {
await mcpHub.refreshAllConnections()
vscode.window.showInformationMessage("MCP servers refreshed successfully")
} else {
vscode.window.showWarningMessage("MCP hub is not available")
}
},
})

export const openClineInNewTab = async ({ context, outputChannel }: Omit<RegisterCommandOptions, "provider">) => {
Expand Down
Loading
Loading