From 1346f1280ca496c41460ee2a98714539b1f66267 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 14:23:31 -0500 Subject: [PATCH 01/57] MCP checkbox for always allow --- CHANGELOG.md | 4 + README.md | 3 +- jest.config.js | 24 ++- .../@modelcontextprotocol/sdk/client/index.js | 17 ++ .../@modelcontextprotocol/sdk/client/stdio.js | 22 ++ .../@modelcontextprotocol/sdk/index.js | 24 +++ .../@modelcontextprotocol/sdk/types.js | 51 +++++ src/__mocks__/McpHub.ts | 17 ++ src/__mocks__/default-shell.js | 12 ++ src/__mocks__/delay.js | 6 + src/__mocks__/globby.js | 10 + src/__mocks__/os-name.js | 6 + src/__mocks__/p-wait-for.js | 20 ++ src/__mocks__/serialize-error.js | 25 +++ src/__mocks__/strip-ansi.js | 7 + src/__mocks__/vscode.js | 57 ++++++ src/core/webview/ClineProvider.ts | 12 ++ src/services/mcp/McpHub.ts | 61 +++++- src/services/mcp/__tests__/McpHub.test.ts | 193 ++++++++++++++++++ src/shared/WebviewMessage.ts | 5 + src/shared/mcp.ts | 1 + webview-ui/src/components/chat/ChatRow.tsx | 21 +- webview-ui/src/components/chat/ChatView.tsx | 21 +- webview-ui/src/components/mcp/McpToolRow.tsx | 34 ++- webview-ui/src/components/mcp/McpView.tsx | 6 +- .../mcp/__tests__/McpToolRow.test.tsx | 107 ++++++++++ 26 files changed, 744 insertions(+), 22 deletions(-) create mode 100644 src/__mocks__/@modelcontextprotocol/sdk/client/index.js create mode 100644 src/__mocks__/@modelcontextprotocol/sdk/client/stdio.js create mode 100644 src/__mocks__/@modelcontextprotocol/sdk/index.js create mode 100644 src/__mocks__/@modelcontextprotocol/sdk/types.js create mode 100644 src/__mocks__/McpHub.ts create mode 100644 src/__mocks__/default-shell.js create mode 100644 src/__mocks__/delay.js create mode 100644 src/__mocks__/globby.js create mode 100644 src/__mocks__/os-name.js create mode 100644 src/__mocks__/p-wait-for.js create mode 100644 src/__mocks__/serialize-error.js create mode 100644 src/__mocks__/strip-ansi.js create mode 100644 src/__mocks__/vscode.js create mode 100644 src/services/mcp/__tests__/McpHub.test.ts create mode 100644 webview-ui/src/components/mcp/__tests__/McpToolRow.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 9899c18ef7..dff6e00707 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Roo Cline Changelog +## [2.2.2] + +- Add checkboxes to auto-approve MCP tools + ## [2.2.1] - Fix another diff editing indentation bug diff --git a/README.md b/README.md index 9daf1c613f..753b870000 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Roo-Cline -A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features. +A fork of Cline, an autonomous coding agent, optimized for speed and flexibility. - Auto-approval capabilities for commands, write, and browser operations - Support for .clinerules per-project custom instructions - Ability to run side-by-side with Cline @@ -10,6 +10,7 @@ A fork of Cline, an autonomous coding agent, with some added experimental config - Support for copying prompts from the history screen - Support for editing through diffs / handling truncated full-file edits - Support for newer Gemini models (gemini-exp-1206 and gemini-2.0-flash-exp) +- Support for auto-approving MCP tools ## Disclaimer diff --git a/jest.config.js b/jest.config.js index dbca14c8d5..b6012c0506 100644 --- a/jest.config.js +++ b/jest.config.js @@ -5,17 +5,35 @@ module.exports = { moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'], transform: { '^.+\\.tsx?$': ['ts-jest', { - tsconfig: 'tsconfig.json' + tsconfig: { + "module": "CommonJS", + "moduleResolution": "node", + "esModuleInterop": true, + "allowJs": true + } }] }, testMatch: ['**/__tests__/**/*.test.ts'], moduleNameMapper: { - '^vscode$': '/node_modules/@types/vscode/index.d.ts' + '^vscode$': '/src/__mocks__/vscode.js', + '@modelcontextprotocol/sdk$': '/src/__mocks__/@modelcontextprotocol/sdk/index.js', + '@modelcontextprotocol/sdk/(.*)': '/src/__mocks__/@modelcontextprotocol/sdk/$1', + '^delay$': '/src/__mocks__/delay.js', + '^p-wait-for$': '/src/__mocks__/p-wait-for.js', + '^globby$': '/src/__mocks__/globby.js', + '^serialize-error$': '/src/__mocks__/serialize-error.js', + '^strip-ansi$': '/src/__mocks__/strip-ansi.js', + '^default-shell$': '/src/__mocks__/default-shell.js', + '^os-name$': '/src/__mocks__/os-name.js' }, + transformIgnorePatterns: [ + 'node_modules/(?!(@modelcontextprotocol|delay|p-wait-for|globby|serialize-error|strip-ansi|default-shell|os-name)/)' + ], setupFiles: [], globals: { 'ts-jest': { - diagnostics: false + diagnostics: false, + isolatedModules: true } } }; diff --git a/src/__mocks__/@modelcontextprotocol/sdk/client/index.js b/src/__mocks__/@modelcontextprotocol/sdk/client/index.js new file mode 100644 index 0000000000..6ed5825645 --- /dev/null +++ b/src/__mocks__/@modelcontextprotocol/sdk/client/index.js @@ -0,0 +1,17 @@ +class Client { + constructor() { + this.request = jest.fn() + } + + connect() { + return Promise.resolve() + } + + close() { + return Promise.resolve() + } +} + +module.exports = { + Client +} \ No newline at end of file diff --git a/src/__mocks__/@modelcontextprotocol/sdk/client/stdio.js b/src/__mocks__/@modelcontextprotocol/sdk/client/stdio.js new file mode 100644 index 0000000000..afa42ad522 --- /dev/null +++ b/src/__mocks__/@modelcontextprotocol/sdk/client/stdio.js @@ -0,0 +1,22 @@ +class StdioClientTransport { + constructor() { + this.start = jest.fn().mockResolvedValue(undefined) + this.close = jest.fn().mockResolvedValue(undefined) + this.stderr = { + on: jest.fn() + } + } +} + +class StdioServerParameters { + constructor() { + this.command = '' + this.args = [] + this.env = {} + } +} + +module.exports = { + StdioClientTransport, + StdioServerParameters +} \ No newline at end of file diff --git a/src/__mocks__/@modelcontextprotocol/sdk/index.js b/src/__mocks__/@modelcontextprotocol/sdk/index.js new file mode 100644 index 0000000000..c6e43e6b68 --- /dev/null +++ b/src/__mocks__/@modelcontextprotocol/sdk/index.js @@ -0,0 +1,24 @@ +const { Client } = require('./client/index.js') +const { StdioClientTransport, StdioServerParameters } = require('./client/stdio.js') +const { + CallToolResultSchema, + ListToolsResultSchema, + ListResourcesResultSchema, + ListResourceTemplatesResultSchema, + ReadResourceResultSchema, + ErrorCode, + McpError +} = require('./types.js') + +module.exports = { + Client, + StdioClientTransport, + StdioServerParameters, + CallToolResultSchema, + ListToolsResultSchema, + ListResourcesResultSchema, + ListResourceTemplatesResultSchema, + ReadResourceResultSchema, + ErrorCode, + McpError +} \ No newline at end of file diff --git a/src/__mocks__/@modelcontextprotocol/sdk/types.js b/src/__mocks__/@modelcontextprotocol/sdk/types.js new file mode 100644 index 0000000000..a2b3ea1588 --- /dev/null +++ b/src/__mocks__/@modelcontextprotocol/sdk/types.js @@ -0,0 +1,51 @@ +const CallToolResultSchema = { + parse: jest.fn().mockReturnValue({}) +} + +const ListToolsResultSchema = { + parse: jest.fn().mockReturnValue({ + tools: [] + }) +} + +const ListResourcesResultSchema = { + parse: jest.fn().mockReturnValue({ + resources: [] + }) +} + +const ListResourceTemplatesResultSchema = { + parse: jest.fn().mockReturnValue({ + resourceTemplates: [] + }) +} + +const ReadResourceResultSchema = { + parse: jest.fn().mockReturnValue({ + contents: [] + }) +} + +const ErrorCode = { + InvalidRequest: 'InvalidRequest', + MethodNotFound: 'MethodNotFound', + InvalidParams: 'InvalidParams', + InternalError: 'InternalError' +} + +class McpError extends Error { + constructor(code, message) { + super(message) + this.code = code + } +} + +module.exports = { + CallToolResultSchema, + ListToolsResultSchema, + ListResourcesResultSchema, + ListResourceTemplatesResultSchema, + ReadResourceResultSchema, + ErrorCode, + McpError +} \ No newline at end of file diff --git a/src/__mocks__/McpHub.ts b/src/__mocks__/McpHub.ts new file mode 100644 index 0000000000..d39b2d7e6c --- /dev/null +++ b/src/__mocks__/McpHub.ts @@ -0,0 +1,17 @@ +export class McpHub { + connections = [] + isConnecting = false + + constructor() { + this.toggleToolAlwaysAllow = jest.fn() + this.callTool = jest.fn() + } + + async toggleToolAlwaysAllow(serverName: string, toolName: string, shouldAllow: boolean): Promise { + return Promise.resolve() + } + + async callTool(serverName: string, toolName: string, toolArguments?: Record): Promise { + return Promise.resolve({ result: 'success' }) + } +} \ No newline at end of file diff --git a/src/__mocks__/default-shell.js b/src/__mocks__/default-shell.js new file mode 100644 index 0000000000..f03e4fbe48 --- /dev/null +++ b/src/__mocks__/default-shell.js @@ -0,0 +1,12 @@ +// Mock default shell based on platform +const os = require('os'); + +let defaultShell; +if (os.platform() === 'win32') { + defaultShell = 'cmd.exe'; +} else { + defaultShell = '/bin/bash'; +} + +module.exports = defaultShell; +module.exports.default = defaultShell; \ No newline at end of file diff --git a/src/__mocks__/delay.js b/src/__mocks__/delay.js new file mode 100644 index 0000000000..9ecb36127d --- /dev/null +++ b/src/__mocks__/delay.js @@ -0,0 +1,6 @@ +function delay(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +module.exports = delay; +module.exports.default = delay; \ No newline at end of file diff --git a/src/__mocks__/globby.js b/src/__mocks__/globby.js new file mode 100644 index 0000000000..2584cd1bef --- /dev/null +++ b/src/__mocks__/globby.js @@ -0,0 +1,10 @@ +function globby(patterns, options) { + return Promise.resolve([]); +} + +globby.sync = function(patterns, options) { + return []; +}; + +module.exports = globby; +module.exports.default = globby; \ No newline at end of file diff --git a/src/__mocks__/os-name.js b/src/__mocks__/os-name.js new file mode 100644 index 0000000000..e760ff3893 --- /dev/null +++ b/src/__mocks__/os-name.js @@ -0,0 +1,6 @@ +function osName() { + return 'macOS'; +} + +module.exports = osName; +module.exports.default = osName; \ No newline at end of file diff --git a/src/__mocks__/p-wait-for.js b/src/__mocks__/p-wait-for.js new file mode 100644 index 0000000000..f1e6a6821d --- /dev/null +++ b/src/__mocks__/p-wait-for.js @@ -0,0 +1,20 @@ +function pWaitFor(condition, options = {}) { + return new Promise((resolve, reject) => { + const interval = setInterval(() => { + if (condition()) { + clearInterval(interval); + resolve(); + } + }, options.interval || 20); + + if (options.timeout) { + setTimeout(() => { + clearInterval(interval); + reject(new Error('Timed out')); + }, options.timeout); + } + }); +} + +module.exports = pWaitFor; +module.exports.default = pWaitFor; \ No newline at end of file diff --git a/src/__mocks__/serialize-error.js b/src/__mocks__/serialize-error.js new file mode 100644 index 0000000000..bf01dc1daa --- /dev/null +++ b/src/__mocks__/serialize-error.js @@ -0,0 +1,25 @@ +function serializeError(error) { + if (error instanceof Error) { + return { + name: error.name, + message: error.message, + stack: error.stack + }; + } + return error; +} + +function deserializeError(errorData) { + if (errorData && typeof errorData === 'object') { + const error = new Error(errorData.message); + error.name = errorData.name; + error.stack = errorData.stack; + return error; + } + return errorData; +} + +module.exports = { + serializeError, + deserializeError +}; \ No newline at end of file diff --git a/src/__mocks__/strip-ansi.js b/src/__mocks__/strip-ansi.js new file mode 100644 index 0000000000..bf7aff9e7a --- /dev/null +++ b/src/__mocks__/strip-ansi.js @@ -0,0 +1,7 @@ +function stripAnsi(string) { + // Simple mock that just returns the input string + return string; +} + +module.exports = stripAnsi; +module.exports.default = stripAnsi; \ No newline at end of file diff --git a/src/__mocks__/vscode.js b/src/__mocks__/vscode.js new file mode 100644 index 0000000000..23f3ae52a0 --- /dev/null +++ b/src/__mocks__/vscode.js @@ -0,0 +1,57 @@ +const vscode = { + window: { + showInformationMessage: jest.fn(), + showErrorMessage: jest.fn(), + createTextEditorDecorationType: jest.fn().mockReturnValue({ + dispose: jest.fn() + }) + }, + workspace: { + onDidSaveTextDocument: jest.fn() + }, + Disposable: class { + dispose() {} + }, + Uri: { + file: (path) => ({ + fsPath: path, + scheme: 'file', + authority: '', + path: path, + query: '', + fragment: '', + with: jest.fn(), + toJSON: jest.fn() + }) + }, + EventEmitter: class { + constructor() { + this.event = jest.fn(); + this.fire = jest.fn(); + } + }, + ConfigurationTarget: { + Global: 1, + Workspace: 2, + WorkspaceFolder: 3 + }, + Position: class { + constructor(line, character) { + this.line = line; + this.character = character; + } + }, + Range: class { + constructor(startLine, startCharacter, endLine, endCharacter) { + this.start = new vscode.Position(startLine, startCharacter); + this.end = new vscode.Position(endLine, endCharacter); + } + }, + ThemeColor: class { + constructor(id) { + this.id = id; + } + } +}; + +module.exports = vscode; \ No newline at end of file diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 48a12c079b..4218ab60f4 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -550,6 +550,18 @@ export class ClineProvider implements vscode.WebviewViewProvider { } break } + case "toggleToolAlwaysAllow": { + try { + await this.mcpHub?.toggleToolAlwaysAllow( + message.serverName!, + message.toolName!, + message.alwaysAllow! + ) + } catch (error) { + console.error(`Failed to toggle auto-approve for tool ${message.toolName}:`, error) + } + break + } // Add more switch case statements here as more webview message commands // are created within the webview context (i.e. inside media/main.js) case "playSound": diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 18a4685a9d..715410e816 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -33,14 +33,17 @@ export type McpConnection = { } // StdioServerParameters +const AlwaysAllowSchema = z.array(z.string()).default([]) + const StdioConfigSchema = z.object({ command: z.string(), args: z.array(z.string()).optional(), env: z.record(z.string()).optional(), + alwaysAllow: AlwaysAllowSchema.optional() }) const McpSettingsSchema = z.object({ - mcpServers: z.record(StdioConfigSchema), + mcpServers: z.record(StdioConfigSchema) }) export class McpHub { @@ -285,7 +288,21 @@ export class McpHub { const response = await this.connections .find((conn) => conn.server.name === serverName) ?.client.request({ method: "tools/list" }, ListToolsResultSchema) - return response?.tools || [] + + // Get always allow settings + const settingsPath = await this.getMcpSettingsFilePath() + const content = await fs.readFile(settingsPath, "utf-8") + const config = JSON.parse(content) + const alwaysAllowConfig = config.mcpServers[serverName]?.alwaysAllow || [] + + // Mark tools as always allowed based on settings + const tools = (response?.tools || []).map(tool => ({ + ...tool, + alwaysAllow: alwaysAllowConfig.includes(tool.name) + })) + + console.log(`[MCP] Fetched tools for ${serverName}:`, tools) + return tools } catch (error) { // console.error(`Failed to fetch tools for ${serverName}:`, error) return [] @@ -478,6 +495,7 @@ export class McpHub { `No connection found for server: ${serverName}. Please make sure to use MCP servers available under 'Connected MCP Servers'.`, ) } + return await connection.client.request( { method: "tools/call", @@ -490,6 +508,45 @@ export class McpHub { ) } + async toggleToolAlwaysAllow(serverName: string, toolName: string, shouldAllow: boolean): Promise { + try { + const settingsPath = await this.getMcpSettingsFilePath() + const content = await fs.readFile(settingsPath, "utf-8") + const config = JSON.parse(content) + + // Initialize alwaysAllow if it doesn't exist + if (!config.mcpServers[serverName].alwaysAllow) { + config.mcpServers[serverName].alwaysAllow = [] + } + + const alwaysAllow = config.mcpServers[serverName].alwaysAllow + const toolIndex = alwaysAllow.indexOf(toolName) + + if (shouldAllow && toolIndex === -1) { + // Add tool to always allow list + alwaysAllow.push(toolName) + } else if (!shouldAllow && toolIndex !== -1) { + // Remove tool from always allow list + alwaysAllow.splice(toolIndex, 1) + } + + // Write updated config back to file + await fs.writeFile(settingsPath, JSON.stringify(config, null, 2)) + + // Update the tools list to reflect the change + const connection = this.connections.find(conn => conn.server.name === serverName) + if (connection) { + connection.server.tools = await this.fetchToolsList(serverName) + await this.notifyWebviewOfServerChanges() + } + + } catch (error) { + console.error("Failed to update always allow settings:", error) + vscode.window.showErrorMessage("Failed to update always allow settings") + throw error // Re-throw to ensure the error is properly handled + } + } + async dispose(): Promise { this.removeAllFileWatchers() for (const connection of this.connections) { diff --git a/src/services/mcp/__tests__/McpHub.test.ts b/src/services/mcp/__tests__/McpHub.test.ts new file mode 100644 index 0000000000..cf4899b562 --- /dev/null +++ b/src/services/mcp/__tests__/McpHub.test.ts @@ -0,0 +1,193 @@ +import type { McpHub as McpHubType } from '../McpHub' +import type { ClineProvider } from '../../../core/webview/ClineProvider' +import type { ExtensionContext, Uri } from 'vscode' +import type { McpConnection } from '../McpHub' + +const vscode = require('vscode') +const fs = require('fs/promises') +const { McpHub } = require('../McpHub') + +jest.mock('vscode') +jest.mock('fs/promises') +jest.mock('../../../core/webview/ClineProvider') + +describe('McpHub', () => { + let mcpHub: McpHubType + let mockProvider: Partial + const mockSettingsPath = '/mock/settings/path/cline_mcp_settings.json' + + beforeEach(() => { + jest.clearAllMocks() + + const mockUri: Uri = { + scheme: 'file', + authority: '', + path: '/test/path', + query: '', + fragment: '', + fsPath: '/test/path', + with: jest.fn(), + toJSON: jest.fn() + } + + mockProvider = { + ensureSettingsDirectoryExists: jest.fn().mockResolvedValue('/mock/settings/path'), + ensureMcpServersDirectoryExists: jest.fn().mockResolvedValue('/mock/settings/path'), + postMessageToWebview: jest.fn(), + context: { + subscriptions: [], + workspaceState: {} as any, + globalState: {} as any, + secrets: {} as any, + extensionUri: mockUri, + extensionPath: '/test/path', + storagePath: '/test/storage', + globalStoragePath: '/test/global-storage', + environmentVariableCollection: {} as any, + extension: { + id: 'test-extension', + extensionUri: mockUri, + extensionPath: '/test/path', + extensionKind: 1, + isActive: true, + packageJSON: { + version: '1.0.0' + }, + activate: jest.fn(), + exports: undefined + } as any, + asAbsolutePath: (path: string) => path, + storageUri: mockUri, + globalStorageUri: mockUri, + logUri: mockUri, + extensionMode: 1, + logPath: '/test/path', + languageModelAccessInformation: {} as any + } as ExtensionContext + } + + // Mock fs.readFile for initial settings + ;(fs.readFile as jest.Mock).mockResolvedValue(JSON.stringify({ + mcpServers: { + 'test-server': { + command: 'node', + args: ['test.js'], + alwaysAllow: ['allowed-tool'] + } + } + })) + + mcpHub = new McpHub(mockProvider as ClineProvider) + }) + + describe('toggleToolAlwaysAllow', () => { + it('should add tool to always allow list when enabling', async () => { + const mockConfig = { + mcpServers: { + 'test-server': { + command: 'node', + args: ['test.js'], + alwaysAllow: [] + } + } + } + + // Mock reading initial config + ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) + + await mcpHub.toggleToolAlwaysAllow('test-server', 'new-tool', true) + + // Verify the config was updated correctly + const writeCall = (fs.writeFile as jest.Mock).mock.calls[0] + const writtenConfig = JSON.parse(writeCall[1]) + expect(writtenConfig.mcpServers['test-server'].alwaysAllow).toContain('new-tool') + }) + + it('should remove tool from always allow list when disabling', async () => { + const mockConfig = { + mcpServers: { + 'test-server': { + command: 'node', + args: ['test.js'], + alwaysAllow: ['existing-tool'] + } + } + } + + // Mock reading initial config + ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) + + await mcpHub.toggleToolAlwaysAllow('test-server', 'existing-tool', false) + + // Verify the config was updated correctly + const writeCall = (fs.writeFile as jest.Mock).mock.calls[0] + const writtenConfig = JSON.parse(writeCall[1]) + expect(writtenConfig.mcpServers['test-server'].alwaysAllow).not.toContain('existing-tool') + }) + + it('should initialize alwaysAllow if it does not exist', async () => { + const mockConfig = { + mcpServers: { + 'test-server': { + command: 'node', + args: ['test.js'] + } + } + } + + // Mock reading initial config + ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) + + await mcpHub.toggleToolAlwaysAllow('test-server', 'new-tool', true) + + // Verify the config was updated with initialized alwaysAllow + const writeCall = (fs.writeFile as jest.Mock).mock.calls[0] + const writtenConfig = JSON.parse(writeCall[1]) + expect(writtenConfig.mcpServers['test-server'].alwaysAllow).toBeDefined() + expect(writtenConfig.mcpServers['test-server'].alwaysAllow).toContain('new-tool') + }) + }) + + describe('callTool', () => { + it('should execute tool successfully', async () => { + // Mock the connection with a minimal client implementation + const mockConnection: McpConnection = { + server: { + name: 'test-server', + config: JSON.stringify({}), + status: 'connected' as const + }, + client: { + request: jest.fn().mockResolvedValue({ result: 'success' }) + } as any, + transport: { + start: jest.fn(), + close: jest.fn(), + stderr: { on: jest.fn() } + } as any + } + + mcpHub.connections = [mockConnection] + + await mcpHub.callTool('test-server', 'some-tool', {}) + + // Verify the request was made with correct parameters + expect(mockConnection.client.request).toHaveBeenCalledWith( + { + method: 'tools/call', + params: { + name: 'some-tool', + arguments: {} + } + }, + expect.any(Object) + ) + }) + + it('should throw error if server not found', async () => { + await expect(mcpHub.callTool('non-existent-server', 'some-tool', {})) + .rejects + .toThrow('No connection found for server: non-existent-server') + }) + }) +}) \ No newline at end of file diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 519756dfc6..fd5b63efc9 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -34,6 +34,7 @@ export interface WebviewMessage { | "diffEnabled" | "openMcpSettings" | "restartMcpServer" + | "toggleToolAlwaysAllow" text?: string askResponse?: ClineAskResponse apiConfiguration?: ApiConfiguration @@ -41,6 +42,10 @@ export interface WebviewMessage { bool?: boolean commands?: string[] audioType?: AudioType + // For toggleToolAutoApprove + serverName?: string + toolName?: string + alwaysAllow?: boolean } export type ClineAskResponse = "yesButtonClicked" | "noButtonClicked" | "messageResponse" diff --git a/src/shared/mcp.ts b/src/shared/mcp.ts index 82efae2f72..a00b34328b 100644 --- a/src/shared/mcp.ts +++ b/src/shared/mcp.ts @@ -12,6 +12,7 @@ export type McpTool = { name: string description?: string inputSchema?: object + alwaysAllow?: boolean } export type McpResource = { diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index d068220d11..6d9042f49f 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -813,14 +813,19 @@ export const ChatRowContent = ({ {useMcpServer.type === "use_mcp_tool" && ( <> - tool.name === useMcpServer.toolName) - ?.description || "", - }} - /> +
e.stopPropagation()}> + tool.name === useMcpServer.toolName) + ?.description || "", + alwaysAllow: server?.tools?.find((tool) => tool.name === useMcpServer.toolName) + ?.alwaysAllow || false, + }} + serverName={useMcpServer.serverName} + /> +
{useMcpServer.arguments && useMcpServer.arguments !== "{}" && (
{ - const { version, clineMessages: messages, taskHistory, apiConfiguration, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, allowedCommands } = useExtensionState() + const { version, clineMessages: messages, taskHistory, apiConfiguration, mcpServers, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, allowedCommands } = useExtensionState() //const task = messages.length > 0 ? (messages[0].say === "task" ? messages[0] : undefined) : undefined) : undefined const task = useMemo(() => messages.at(0), [messages]) // leaving this less safe version here since if the first message is not a task, then the extension is in a bad state and needs to be debugged (see Cline.abort) @@ -767,6 +768,19 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie return false } + const isMcpToolAlwaysAllowed = () => { + const lastMessage = messages.at(-1) + if (lastMessage?.type === "ask" && lastMessage.ask === "use_mcp_server" && lastMessage.text) { + const mcpServerUse = JSON.parse(lastMessage.text) as { type: string; serverName: string; toolName: string } + if (mcpServerUse.type === "use_mcp_tool") { + const server = mcpServers?.find((s: McpServer) => s.name === mcpServerUse.serverName) + const tool = server?.tools?.find((t: McpTool) => t.name === mcpServerUse.toolName) + return tool?.alwaysAllow || false + } + } + return false + } + const isAllowedCommand = () => { const lastMessage = messages.at(-1) if (lastMessage?.type === "ask" && lastMessage.text) { @@ -788,11 +802,12 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie (alwaysAllowBrowser && clineAsk === "browser_action_launch") || (alwaysAllowReadOnly && clineAsk === "tool" && isReadOnlyToolAction()) || (alwaysAllowWrite && clineAsk === "tool" && isWriteToolAction()) || - (alwaysAllowExecute && clineAsk === "command" && isAllowedCommand()) + (alwaysAllowExecute && clineAsk === "command" && isAllowedCommand()) || + (clineAsk === "use_mcp_server" && isMcpToolAlwaysAllowed()) ) { handlePrimaryButtonClick() } - }, [clineAsk, enableButtons, handlePrimaryButtonClick, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, messages, allowedCommands]) + }, [clineAsk, enableButtons, handlePrimaryButtonClick, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, messages, allowedCommands, mcpServers]) return (
{ +const McpToolRow = ({ tool, serverName }: McpToolRowProps) => { + const handleAlwaysAllowChange = () => { + if (!serverName) return; + + vscode.postMessage({ + type: "toggleToolAlwaysAllow", + serverName, + toolName: tool.name, + alwaysAllow: !tool.alwaysAllow + }); + } + return (
-
- - {tool.name} +
e.stopPropagation()}> +
+ + {tool.name} +
+ {serverName && ( + + Always allow + + )}
{tool.description && (
{
{server.tools.map((tool) => ( - + ))}
) : ( diff --git a/webview-ui/src/components/mcp/__tests__/McpToolRow.test.tsx b/webview-ui/src/components/mcp/__tests__/McpToolRow.test.tsx new file mode 100644 index 0000000000..ff708dbf56 --- /dev/null +++ b/webview-ui/src/components/mcp/__tests__/McpToolRow.test.tsx @@ -0,0 +1,107 @@ +import React from 'react' +import { render, fireEvent, screen } from '@testing-library/react' +import McpToolRow from '../McpToolRow' +import { vscode } from '../../../utils/vscode' + +jest.mock('../../../utils/vscode', () => ({ + vscode: { + postMessage: jest.fn() + } +})) + +describe('McpToolRow', () => { + const mockTool = { + name: 'test-tool', + description: 'A test tool', + alwaysAllow: false + } + + beforeEach(() => { + jest.clearAllMocks() + }) + + it('renders tool name and description', () => { + render() + + expect(screen.getByText('test-tool')).toBeInTheDocument() + expect(screen.getByText('A test tool')).toBeInTheDocument() + }) + + it('does not show always allow checkbox when serverName is not provided', () => { + render() + + expect(screen.queryByText('Always allow')).not.toBeInTheDocument() + }) + + it('shows always allow checkbox when serverName is provided', () => { + render() + + expect(screen.getByText('Always allow')).toBeInTheDocument() + }) + + it('sends message to toggle always allow when checkbox is clicked', () => { + render() + + const checkbox = screen.getByRole('checkbox') + fireEvent.click(checkbox) + + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: 'toggleToolAlwaysAllow', + serverName: 'test-server', + toolName: 'test-tool', + alwaysAllow: true + }) + }) + + it('reflects always allow state in checkbox', () => { + const alwaysAllowedTool = { + ...mockTool, + alwaysAllow: true + } + + render() + + const checkbox = screen.getByRole('checkbox') + expect(checkbox).toBeChecked() + }) + + it('prevents event propagation when clicking the checkbox', () => { + const mockStopPropagation = jest.fn() + render() + + const container = screen.getByTestId('tool-row-container') + fireEvent.click(container, { + stopPropagation: mockStopPropagation + }) + + expect(mockStopPropagation).toHaveBeenCalled() + }) + + it('displays input schema parameters when provided', () => { + const toolWithSchema = { + ...mockTool, + inputSchema: { + type: 'object', + properties: { + param1: { + type: 'string', + description: 'First parameter' + }, + param2: { + type: 'number', + description: 'Second parameter' + } + }, + required: ['param1'] + } + } + + render() + + expect(screen.getByText('Parameters')).toBeInTheDocument() + expect(screen.getByText('param1')).toBeInTheDocument() + expect(screen.getByText('param2')).toBeInTheDocument() + expect(screen.getByText('First parameter')).toBeInTheDocument() + expect(screen.getByText('Second parameter')).toBeInTheDocument() + }) +}) \ No newline at end of file From b4cf86f03f8df38ade446d473fad8323c249ff23 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 16:27:21 -0500 Subject: [PATCH 02/57] Add test ID --- webview-ui/src/components/mcp/McpToolRow.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/webview-ui/src/components/mcp/McpToolRow.tsx b/webview-ui/src/components/mcp/McpToolRow.tsx index 44a0d86450..0ad8b16caa 100644 --- a/webview-ui/src/components/mcp/McpToolRow.tsx +++ b/webview-ui/src/components/mcp/McpToolRow.tsx @@ -26,6 +26,7 @@ const McpToolRow = ({ tool, serverName }: McpToolRowProps) => { padding: "3px 0", }}>
e.stopPropagation()}>
From 96af31c98e7dca1f9af310aad6af576c6dcd0204 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 16:58:42 -0500 Subject: [PATCH 03/57] Readme updates --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 753b870000..be5f192ebc 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ A fork of Cline, an autonomous coding agent, optimized for speed and flexibility - Support for copying prompts from the history screen - Support for editing through diffs / handling truncated full-file edits - Support for newer Gemini models (gemini-exp-1206 and gemini-2.0-flash-exp) +- Support for dragging and dropping images into chats - Support for auto-approving MCP tools ## Disclaimer From 8565d5614694b251b7293a899b26b96b57653a9c Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 17:02:56 -0500 Subject: [PATCH 04/57] Package bump --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index f2907547a0..7205cce2a0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.2.1", + "version": "2.2.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.2.1", + "version": "2.2.2", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", diff --git a/package.json b/package.json index 15d5e18e0b..57f94ae417 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "displayName": "Roo Cline", "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.", "publisher": "RooVeterinaryInc", - "version": "2.2.1", + "version": "2.2.2", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", From 23efdeaf3584f842a1d774e7992edd8d74fe8bd5 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 17:16:27 -0500 Subject: [PATCH 05/57] More safety around always allowing MCP --- src/core/prompts/system.ts | 2 ++ src/core/webview/ClineProvider.ts | 10 ++++++++ src/shared/ExtensionMessage.ts | 1 + src/shared/WebviewMessage.ts | 1 + webview-ui/src/components/chat/ChatView.tsx | 6 ++--- webview-ui/src/components/mcp/McpToolRow.tsx | 5 ++-- webview-ui/src/components/mcp/McpView.tsx | 7 +++--- .../src/components/settings/SettingsView.tsx | 25 +++++++++++++++++++ .../src/context/ExtensionStateContext.tsx | 2 ++ 9 files changed, 51 insertions(+), 8 deletions(-) diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index 88cf9f6b5c..25e693a369 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -633,6 +633,8 @@ npm run build 5. Install the MCP Server by adding the MCP server configuration to the settings file located at '${await mcpHub.getMcpSettingsFilePath()}'. The settings file may have other MCP servers already configured, so you would read it first and then add your new server to the existing \`mcpServers\` object. +IMPORTANT: Regardless of what else you see in the settings file, you must not set any defaults for the \`alwaysAllow\` array in the newly added MCP server. + \`\`\`json { "mcpServers": { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 4218ab60f4..5877f4fd1d 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -67,6 +67,7 @@ type GlobalStateKey = | "allowedCommands" | "soundEnabled" | "diffEnabled" + | "alwaysAllowMcp" export const GlobalFileNames = { apiConversationHistory: "api_conversation_history.json", @@ -456,6 +457,10 @@ export class ClineProvider implements vscode.WebviewViewProvider { await this.updateGlobalState("alwaysAllowBrowser", message.bool ?? undefined) await this.postStateToWebview() break + case "alwaysAllowMcp": + await this.updateGlobalState("alwaysAllowMcp", message.bool) + await this.postStateToWebview() + break case "askResponse": this.cline?.handleWebviewAskResponse(message.askResponse!, message.text, message.images) break @@ -904,6 +909,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { alwaysAllowWrite, alwaysAllowExecute, alwaysAllowBrowser, + alwaysAllowMcp, soundEnabled, diffEnabled, taskHistory, @@ -921,6 +927,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { alwaysAllowWrite: alwaysAllowWrite ?? false, alwaysAllowExecute: alwaysAllowExecute ?? false, alwaysAllowBrowser: alwaysAllowBrowser ?? false, + alwaysAllowMcp: alwaysAllowMcp ?? false, uriScheme: vscode.env.uriScheme, clineMessages: this.cline?.clineMessages || [], taskHistory: (taskHistory || []) @@ -1017,6 +1024,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { alwaysAllowWrite, alwaysAllowExecute, alwaysAllowBrowser, + alwaysAllowMcp, taskHistory, allowedCommands, soundEnabled, @@ -1053,6 +1061,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { this.getGlobalState("alwaysAllowWrite") as Promise, this.getGlobalState("alwaysAllowExecute") as Promise, this.getGlobalState("alwaysAllowBrowser") as Promise, + this.getGlobalState("alwaysAllowMcp") as Promise, this.getGlobalState("taskHistory") as Promise, this.getGlobalState("allowedCommands") as Promise, this.getGlobalState("soundEnabled") as Promise, @@ -1107,6 +1116,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { alwaysAllowWrite: alwaysAllowWrite ?? false, alwaysAllowExecute: alwaysAllowExecute ?? false, alwaysAllowBrowser: alwaysAllowBrowser ?? false, + alwaysAllowMcp: alwaysAllowMcp ?? false, taskHistory, allowedCommands, soundEnabled, diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index c38cb63e80..608b5e5bb8 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -47,6 +47,7 @@ export interface ExtensionState { alwaysAllowWrite?: boolean alwaysAllowExecute?: boolean alwaysAllowBrowser?: boolean + alwaysAllowMcp?: boolean uriScheme?: string allowedCommands?: string[] soundEnabled?: boolean diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index fd5b63efc9..e7cfe43184 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -29,6 +29,7 @@ export interface WebviewMessage { | "cancelTask" | "refreshOpenRouterModels" | "alwaysAllowBrowser" + | "alwaysAllowMcp" | "playSound" | "soundEnabled" | "diffEnabled" diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 08780ddab9..e4e0880ba7 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -37,7 +37,7 @@ interface ChatViewProps { export const MAX_IMAGES_PER_MESSAGE = 20 // Anthropic limits to 20 images const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryView }: ChatViewProps) => { - const { version, clineMessages: messages, taskHistory, apiConfiguration, mcpServers, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, allowedCommands } = useExtensionState() + const { version, clineMessages: messages, taskHistory, apiConfiguration, mcpServers, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, alwaysAllowMcp, allowedCommands } = useExtensionState() //const task = messages.length > 0 ? (messages[0].say === "task" ? messages[0] : undefined) : undefined) : undefined const task = useMemo(() => messages.at(0), [messages]) // leaving this less safe version here since if the first message is not a task, then the extension is in a bad state and needs to be debugged (see Cline.abort) @@ -803,11 +803,11 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie (alwaysAllowReadOnly && clineAsk === "tool" && isReadOnlyToolAction()) || (alwaysAllowWrite && clineAsk === "tool" && isWriteToolAction()) || (alwaysAllowExecute && clineAsk === "command" && isAllowedCommand()) || - (clineAsk === "use_mcp_server" && isMcpToolAlwaysAllowed()) + (alwaysAllowMcp && clineAsk === "use_mcp_server" && isMcpToolAlwaysAllowed()) ) { handlePrimaryButtonClick() } - }, [clineAsk, enableButtons, handlePrimaryButtonClick, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, messages, allowedCommands, mcpServers]) + }, [clineAsk, enableButtons, handlePrimaryButtonClick, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, alwaysAllowMcp, messages, allowedCommands, mcpServers]) return (
{ +const McpToolRow = ({ tool, serverName, alwaysAllowMcp }: McpToolRowProps) => { const handleAlwaysAllowChange = () => { if (!serverName) return; @@ -33,7 +34,7 @@ const McpToolRow = ({ tool, serverName }: McpToolRowProps) => { {tool.name}
- {serverName && ( + {serverName && alwaysAllowMcp && ( { - const { mcpServers: servers } = useExtensionState() + const { mcpServers: servers, alwaysAllowMcp } = useExtensionState() // const [servers, setServers] = useState([ // // Add some mock servers for testing // { @@ -126,7 +126,7 @@ const McpView = ({ onDone }: McpViewProps) => { {servers.length > 0 && (
{servers.map((server) => ( - + ))}
)} @@ -152,7 +152,7 @@ const McpView = ({ onDone }: McpViewProps) => { } // Server Row Component -const ServerRow = ({ server }: { server: McpServer }) => { +const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer, alwaysAllowMcp?: boolean }) => { const [isExpanded, setIsExpanded] = useState(false) const getStatusColor = () => { @@ -260,6 +260,7 @@ const ServerRow = ({ server }: { server: McpServer }) => { key={tool.name} tool={tool} serverName={server.name} + alwaysAllowMcp={alwaysAllowMcp} /> ))}
diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index acff00cec4..0290974b4a 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -25,6 +25,8 @@ const SettingsView = ({ onDone }: SettingsViewProps) => { setAlwaysAllowExecute, alwaysAllowBrowser, setAlwaysAllowBrowser, + alwaysAllowMcp, + setAlwaysAllowMcp, soundEnabled, setSoundEnabled, diffEnabled, @@ -50,6 +52,7 @@ const SettingsView = ({ onDone }: SettingsViewProps) => { vscode.postMessage({ type: "alwaysAllowWrite", bool: alwaysAllowWrite }) vscode.postMessage({ type: "alwaysAllowExecute", bool: alwaysAllowExecute }) vscode.postMessage({ type: "alwaysAllowBrowser", bool: alwaysAllowBrowser }) + vscode.postMessage({ type: "alwaysAllowMcp", bool: alwaysAllowMcp }) vscode.postMessage({ type: "allowedCommands", commands: allowedCommands ?? [] }) vscode.postMessage({ type: "soundEnabled", bool: soundEnabled }) vscode.postMessage({ type: "diffEnabled", bool: diffEnabled }) @@ -195,7 +198,29 @@ const SettingsView = ({ onDone }: SettingsViewProps) => { color: "var(--vscode-errorForeground)", }}> ⚠️ WARNING: When enabled, Cline will automatically perform browser actions without requiring approval. This is potentially very dangerous and could lead to unwanted system modifications or security risks. Enable only if you fully trust the AI and understand the risks.

NOTE: The checkbox only applies when the model supports computer use. +

+
+
+ { + setAlwaysAllowMcp(e.target.checked) + vscode.postMessage({ type: "alwaysAllowMcp", bool: e.target.checked }) + }}> + Always approve MCP tools + +

+ ⚠️ WARNING: When enabled, you can set individual MCP tools to auto-approve in the MCP Servers view. A tool will only be auto-approved if both this setting and the tool's individual "Always allow" checkbox are enabled. This is potentially very dangerous and could lead to unwanted system modifications or security risks. Enable only if you fully trust the AI and understand the risks.

diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 18345734bd..f9690b60c2 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -25,6 +25,7 @@ export interface ExtensionStateContextType extends ExtensionState { setAlwaysAllowWrite: (value: boolean) => void setAlwaysAllowExecute: (value: boolean) => void setAlwaysAllowBrowser: (value: boolean) => void + setAlwaysAllowMcp: (value: boolean) => void setShowAnnouncement: (value: boolean) => void setAllowedCommands: (value: string[]) => void setSoundEnabled: (value: boolean) => void @@ -134,6 +135,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode setAlwaysAllowWrite: (value) => setState((prevState) => ({ ...prevState, alwaysAllowWrite: value })), setAlwaysAllowExecute: (value) => setState((prevState) => ({ ...prevState, alwaysAllowExecute: value })), setAlwaysAllowBrowser: (value) => setState((prevState) => ({ ...prevState, alwaysAllowBrowser: value })), + setAlwaysAllowMcp: (value) => setState((prevState) => ({ ...prevState, alwaysAllowMcp: value })), setShowAnnouncement: (value) => setState((prevState) => ({ ...prevState, shouldShowAnnouncement: value })), setAllowedCommands: (value) => setState((prevState) => ({ ...prevState, allowedCommands: value })), setSoundEnabled: (value) => setState((prevState) => ({ ...prevState, soundEnabled: value })), From 5c564d8e25ac4a3b09cd1f190cad55a00c1f6278 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 18:58:25 -0500 Subject: [PATCH 06/57] Clean up the settings page --- .../src/components/settings/SettingsView.tsx | 298 ++++++++---------- 1 file changed, 135 insertions(+), 163 deletions(-) diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 0290974b4a..deab3d6c4a 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -145,6 +145,20 @@ const SettingsView = ({ onDone }: SettingsViewProps) => {

+
+ setDiffEnabled(e.target.checked)}> + Enable editing through diffs + +

+ When enabled, Cline will be able to edit files more quickly and will automatically reject truncated full-file writes. +

+
+
{

-
- setAlwaysAllowWrite(e.target.checked)}> - Always approve write operations - -

- ⚠️ WARNING: When enabled, Cline will automatically create and edit files without requiring approval. This is potentially very dangerous and could lead to unwanted system modifications or security risks. Enable only if you fully trust the AI and understand the risks. +

+

⚠️ High-Risk Auto-Approve Settings

+

+ The following settings allow Cline to automatically perform potentially dangerous operations without requiring approval. + Enable these settings only if you fully trust the AI and understand the associated security risks.

-
-
- setAlwaysAllowBrowser(e.target.checked)}> - Always approve browser actions - -

- ⚠️ WARNING: When enabled, Cline will automatically perform browser actions without requiring approval. This is potentially very dangerous and could lead to unwanted system modifications or security risks. Enable only if you fully trust the AI and understand the risks.

NOTE: The checkbox only applies when the model supports computer use. -

-
- -
- { - setAlwaysAllowMcp(e.target.checked) - vscode.postMessage({ type: "alwaysAllowMcp", bool: e.target.checked }) - }}> - Always approve MCP tools - -

- ⚠️ WARNING: When enabled, you can set individual MCP tools to auto-approve in the MCP Servers view. A tool will only be auto-approved if both this setting and the tool's individual "Always allow" checkbox are enabled. This is potentially very dangerous and could lead to unwanted system modifications or security risks. Enable only if you fully trust the AI and understand the risks. -

-
- -
- setAlwaysAllowExecute(e.target.checked)}> - Always approve allowed execute operations - -

- ⚠️ WARNING: When enabled, Cline will automatically execute allowed terminal commands without requiring approval. This is potentially very dangerous and could lead to unwanted system modifications or security risks. Enable only if you fully trust the AI and understand the risks. -

-
- - {alwaysAllowExecute && (
-
- Allowed Auto-Execute Commands -

- Command prefixes that can be auto-executed when "Always approve execute operations" is enabled. -

- -
- setCommandInput(e.target.value)} - placeholder="Enter command prefix (e.g., 'git ')" - style={{ flexGrow: 1 }} - /> - - Add - -
- -
- {(allowedCommands ?? []).map((cmd, index) => ( -
- {cmd} - { - const newCommands = (allowedCommands ?? []).filter((_, i) => i !== index) - setAllowedCommands(newCommands) - vscode.postMessage({ - type: "allowedCommands", - commands: newCommands - }) - }} - > - - -
- ))} -
-
+ setAlwaysAllowWrite(e.target.checked)}> + Always approve write operations + +

+ Automatically create and edit files without requiring approval +

- )} + +
+ setAlwaysAllowBrowser(e.target.checked)}> + Always approve browser actions + +

+ Automatically perform browser actions without requiring approval
+ Note: Only applies when the model supports computer use +

+
+ +
+ { + setAlwaysAllowMcp(e.target.checked) + vscode.postMessage({ type: "alwaysAllowMcp", bool: e.target.checked }) + }}> + Always approve MCP tools + +

+ Enable auto-approval of individual MCP tools in the MCP Servers view (requires both this setting and the tool's individual "Always allow" checkbox) +

+
+ +
+ setAlwaysAllowExecute(e.target.checked)}> + Always approve allowed execute operations + +

+ Automatically execute allowed terminal commands without requiring approval +

+ + {alwaysAllowExecute && ( +
+ Allowed Auto-Execute Commands +

+ Command prefixes that can be auto-executed when "Always approve execute operations" is enabled. +

+ +
+ setCommandInput(e.target.value)} + placeholder="Enter command prefix (e.g., 'git ')" + style={{ flexGrow: 1 }} + /> + + Add + +
+ +
+ {(allowedCommands ?? []).map((cmd, index) => ( +
+ {cmd} + { + const newCommands = (allowedCommands ?? []).filter((_, i) => i !== index) + setAllowedCommands(newCommands) + vscode.postMessage({ + type: "allowedCommands", + commands: newCommands + }) + }} + > + + +
+ ))} +
+
+ )} +
+

Experimental Features

-
- setDiffEnabled(e.target.checked)}> - Enable editing through diffs - -

- When enabled, Cline will be able to apply diffs to make changes to files and will automatically reject truncated full-file edits. -

-
-
setSoundEnabled(e.target.checked)}> Enable sound effects From 663759b20759dcc923e76dc7e9ac14e64c79b047 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 19:03:16 -0500 Subject: [PATCH 07/57] Changeset --- .changeset/cyan-dragons-behave.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/cyan-dragons-behave.md diff --git a/.changeset/cyan-dragons-behave.md b/.changeset/cyan-dragons-behave.md new file mode 100644 index 0000000000..722a31f88d --- /dev/null +++ b/.changeset/cyan-dragons-behave.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +Clean up the settings screen From b3ced0aac034447a360d685a1fb1deec4c17a79a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sat, 14 Dec 2024 00:28:49 +0000 Subject: [PATCH 08/57] changeset version bump --- .changeset/cyan-dragons-behave.md | 5 ----- CHANGELOG.md | 12 +++++++++--- package-lock.json | 4 ++-- package.json | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-) delete mode 100644 .changeset/cyan-dragons-behave.md diff --git a/.changeset/cyan-dragons-behave.md b/.changeset/cyan-dragons-behave.md deleted file mode 100644 index 722a31f88d..0000000000 --- a/.changeset/cyan-dragons-behave.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"roo-cline": patch ---- - -Clean up the settings screen diff --git a/CHANGELOG.md b/CHANGELOG.md index dff6e00707..f680bbddfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Roo Cline Changelog +## 2.2.3 + +### Patch Changes + +- 663759b: Clean up the settings screen + ## [2.2.2] - Add checkboxes to auto-approve MCP tools @@ -76,9 +82,9 @@ ## [2.2.0] -- Add support for Model Context Protocol (MCP), enabling Cline to use custom tools like web-search tool or GitHub tool -- Add MCP server management tab accessible via the server icon in the menu bar -- Add ability for Cline to dynamically create new MCP servers based on user requests (e.g., "add a tool that gets the latest npm docs") +- Add support for Model Context Protocol (MCP), enabling Cline to use custom tools like web-search tool or GitHub tool +- Add MCP server management tab accessible via the server icon in the menu bar +- Add ability for Cline to dynamically create new MCP servers based on user requests (e.g., "add a tool that gets the latest npm docs") ## [2.1.6] diff --git a/package-lock.json b/package-lock.json index 7205cce2a0..77cc150cfd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.2.2", + "version": "2.2.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.2.2", + "version": "2.2.3", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", diff --git a/package.json b/package.json index 57f94ae417..8103fee0c1 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "displayName": "Roo Cline", "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.", "publisher": "RooVeterinaryInc", - "version": "2.2.2", + "version": "2.2.3", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", From b82c21f38fa508864ffde6b120b225fc33847b29 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 19:31:28 -0500 Subject: [PATCH 09/57] Update CHANGELOG.md --- CHANGELOG.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f680bbddfc..738a5ca379 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,8 @@ # Roo Cline Changelog -## 2.2.3 +## [2.2.3] -### Patch Changes - -- 663759b: Clean up the settings screen +- Clean up the settings screen ## [2.2.2] From 5cbf4ca5c8ab4410de4f821ce0b8772dc1aea051 Mon Sep 17 00:00:00 2001 From: ColemanRoo Date: Fri, 13 Dec 2024 21:56:03 -0600 Subject: [PATCH 10/57] Adding temp publish workflow Disable automated changeset flow --- .github/workflows/changeset-ai-releases.yml | 5 +-- .../workflows/temp-marketplace-publish.yml | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/temp-marketplace-publish.yml diff --git a/.github/workflows/changeset-ai-releases.yml b/.github/workflows/changeset-ai-releases.yml index cf22faa20e..d7e7310467 100644 --- a/.github/workflows/changeset-ai-releases.yml +++ b/.github/workflows/changeset-ai-releases.yml @@ -8,8 +8,9 @@ run-name: Changeset AI Release ${{ github.actor != 'R00-B0T' && '- Create PR' || # 4. Creating a GitHub release with the AI-generated notes on: - pull_request: - types: [closed, opened, synchronize, labeled] + # pull_request: + # types: [closed, opened, synchronize, labeled] + workflow_dispatch: env: REPO_PATH: ${{ github.repository }} diff --git a/.github/workflows/temp-marketplace-publish.yml b/.github/workflows/temp-marketplace-publish.yml new file mode 100644 index 0000000000..a98aca38e6 --- /dev/null +++ b/.github/workflows/temp-marketplace-publish.yml @@ -0,0 +1,31 @@ +name: Publish Extension Temporary +on: + push: + branches: ["main"] + workflow_dispatch: + +jobs: + publish-extension: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: 18 + - run: | + git config user.name github-actions + git config user.email github-actions@github.com + - name: Install Dependencies + run: | + npm install -g vsce ovsx + npm install + cd webview-ui + npm install + cd .. + - name: Package and Publish Extension + env: + VSCE_PAT: ${{ secrets.VSCE_PAT }} + run: | + current_package_version=$(node -p "require('./package.json').version") + npm run publish:marketplace + echo "Successfully published version $current_package_version to VS Code Marketplace" From 5224be8026a317df9acf0a4b2a87a6998ee0d048 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 23:21:01 -0500 Subject: [PATCH 11/57] Add issue template --- .github/ISSUE_TEMPLATE/config.yml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/config.yml diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000000..58663cec6e --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,8 @@ +blank_issues_enabled: false +contact_links: + - name: Feature Request + url: https://github.com/RooVetGit/Roo-Cline/discussions/categories/feature-requests + about: Share and vote on feature requests for Roo Cline + - name: Leave a Review + url: https://marketplace.visualstudio.com/items?itemName=RooVeterinaryInc.roo-cline + about: Enjoying Roo Cline? Leave a review here! From b2c8805e5b97b6fce2d1fc6617d67483eb3dd52f Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 23:08:11 -0500 Subject: [PATCH 12/57] Update the prompt to encourage diff edits when the box is checked --- src/core/diff/strategies/search-replace.ts | 2 +- src/core/prompts/system.ts | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/core/diff/strategies/search-replace.ts b/src/core/diff/strategies/search-replace.ts index 53be5413d0..d83beb1f56 100644 --- a/src/core/diff/strategies/search-replace.ts +++ b/src/core/diff/strategies/search-replace.ts @@ -5,10 +5,10 @@ export class SearchReplaceDiffStrategy implements DiffStrategy { return `## apply_diff Description: Request to replace existing code using search and replace blocks. This tool allows for precise, surgical replaces to files by specifying exactly what content to search for and what to replace it with. -Only use this tool when you need to replace/fix existing code. The tool will maintain proper indentation and formatting while making changes. Only a single operation is allowed per tool use. The SEARCH section must exactly match existing content including whitespace and indentation. +If you're not confident in the exact content to search for, use the read_file tool first to get the exact content. Parameters: - path: (required) The path of the file to modify (relative to the current working directory ${cwd}) diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index 25e693a369..de3c19e052 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -663,7 +663,7 @@ The user may ask to add tools or resources that may make sense to add to an exis .getServers() .map((server) => server.name) .join(", ") || "(None running currently)" -}, e.g. if it would use the same API. This would be possible if you can locate the MCP server repository on the user's system by looking at the server arguments for a filepath. You might then use list_files and read_file to explore the files in the repository, and use write_to_file to make changes to the files. +}, e.g. if it would use the same API. This would be possible if you can locate the MCP server repository on the user's system by looking at the server arguments for a filepath. You might then use list_files and read_file to explore the files in the repository, and use write_to_file ${diffStrategy ? "or apply_diff " : ""}to make changes to the files. However some MCP servers may be running from installed packages rather than a local repository, in which case it may make more sense to create a new MCP server. @@ -701,10 +701,9 @@ RULES - Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context provided to understand the user's environment and tailor your commands to ensure they are compatible with their system. You must also consider if the command you need to run should be executed in a specific directory outside of the current working directory '${cwd.toPosix()}', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '${cwd.toPosix()}'). For example, if you needed to run \`npm install\` in a project outside of '${cwd.toPosix()}', you would need to prepend with a \`cd\` i.e. pseudocode for this would be \`cd (path to project) && (command, in this case npm install)\`. - When using the search_files tool, craft your regex patterns carefully to balance specificity and flexibility. Based on the user's task you may use it to find code patterns, TODO comments, function definitions, or any text-based information across the project. The results include context, so analyze the surrounding code to better understand the matches. Leverage the search_files tool in combination with other tools for more comprehensive analysis. For example, use it to find specific code patterns, then use read_file to examine the full context of interesting matches before using write_to_file to make informed changes. - When creating a new project (such as an app, website, or any software project), organize all new files within a dedicated project directory unless the user specifies otherwise. Use appropriate file paths when writing files, as the write_to_file tool will automatically create any necessary directories. Structure the project logically, adhering to best practices for the specific type of project being created. Unless otherwise specified, new projects should be easily run without additional setup, for example most projects can be built in HTML, CSS, and JavaScript - which you can open in a browser. -${diffStrategy ? "- Prefer to use apply_diff over write_to_file when making changes to existing files, particularly when editing files more than 200 lines of code, as it allows you to apply specific modifications based on a set of changes provided in a diff. This is particularly useful when you need to make targeted edits or updates to a file without overwriting the entire content." : ""} +${diffStrategy ? "- You should use apply_diff instead of write_to_file when making changes to existing files since it is much faster and easier to apply a diff than to write the entire file again. Only use write_to_file to edit files when apply_diff has failed repeatedly to apply the diff." : "- When you want to modify a file, use the write_to_file tool directly with the desired content. You do not need to display the content before using the tool."} - Be sure to consider the type of project (e.g. Python, JavaScript, web application) when determining the appropriate structure and files to include. Also consider what files may be most relevant to accomplishing the task, for example looking at a project's manifest file would help you understand the project's dependencies, which you could incorporate into any code you write. - When making changes to code, always consider the context in which the code is being used. Ensure that your changes are compatible with the existing codebase and that they follow the project's coding standards and best practices. -- When you want to modify a file, use the write_to_file tool directly with the desired content. You do not need to display the content before using the tool. - Do not ask for more information than necessary. Use the tools provided to accomplish the user's request efficiently and effectively. When you've completed your task, you must use the attempt_completion tool to present the result to the user. The user may provide feedback, which you can use to make improvements and try again. - You are only allowed to ask the user questions using the ask_followup_question tool. Use this tool only when you need additional details to complete a task, and be sure to use a clear and concise question that will help you move forward with the task. However if you can use the available tools to avoid having to ask the user questions, you should do so. For example, if the user mentions a file that may be in an outside directory like the Desktop, you should use the list_files tool to list the files in the Desktop and check if the file they are talking about is there, rather than asking the user to provide the file path themselves. - When executing commands, if you don't see the expected output, assume the terminal executed the command successfully and proceed with the task. The user's terminal may be unable to stream the output back properly. If you absolutely need to see the actual terminal output, use the ask_followup_question tool to request the user to copy and paste it back to you. From 408bf917fb6392301b0aa6ffc3114344f0fde55b Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 23:38:14 -0500 Subject: [PATCH 13/57] Better review link --- .github/ISSUE_TEMPLATE/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 58663cec6e..e9033a41c8 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -4,5 +4,5 @@ contact_links: url: https://github.com/RooVetGit/Roo-Cline/discussions/categories/feature-requests about: Share and vote on feature requests for Roo Cline - name: Leave a Review - url: https://marketplace.visualstudio.com/items?itemName=RooVeterinaryInc.roo-cline + url: https://marketplace.visualstudio.com/items?itemName=RooVeterinaryInc.roo-cline&ssr=false#review-details about: Enjoying Roo Cline? Leave a review here! From ac93738937b9c08e663470d4d63009652e35adbc Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 23:56:51 -0500 Subject: [PATCH 14/57] Release --- CHANGELOG.md | 4 ++++ package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 738a5ca379..db1e6a9759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Roo Cline Changelog +## [2.2.4] + +- Tweak the prompt to encourage diff edits when they're enabled + ## [2.2.3] - Clean up the settings screen diff --git a/package-lock.json b/package-lock.json index 77cc150cfd..40baa7776a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.2.3", + "version": "2.2.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.2.3", + "version": "2.2.4", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", diff --git a/package.json b/package.json index 8103fee0c1..d5798bea31 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "displayName": "Roo Cline", "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.", "publisher": "RooVeterinaryInc", - "version": "2.2.3", + "version": "2.2.4", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", From 3d7ff3240612c81295c7d6a00e12e895855381f2 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sat, 14 Dec 2024 01:17:01 -0500 Subject: [PATCH 15/57] Allow enabling/disabling of MCP servers --- CHANGELOG.md | 4 + README.md | 1 + package-lock.json | 4 +- package.json | 2 +- src/core/prompts/system.ts | 2 +- src/core/webview/ClineProvider.ts | 11 +++ src/services/mcp/McpHub.ts | 93 ++++++++++++++++++++-- src/services/mcp/__tests__/McpHub.test.ts | 97 +++++++++++++++++++++++ src/shared/WebviewMessage.ts | 2 + src/shared/mcp.ts | 1 + webview-ui/src/components/mcp/McpView.tsx | 50 ++++++++++++ 11 files changed, 258 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db1e6a9759..f3cf1dd13a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Roo Cline Changelog +## [2.2.5] + +- Allow MCP servers to be enabled/disabled + ## [2.2.4] - Tweak the prompt to encourage diff edits when they're enabled diff --git a/README.md b/README.md index be5f192ebc..e7e0b78b31 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ A fork of Cline, an autonomous coding agent, optimized for speed and flexibility - Support for newer Gemini models (gemini-exp-1206 and gemini-2.0-flash-exp) - Support for dragging and dropping images into chats - Support for auto-approving MCP tools +- Support for enabling/disabling MCP servers ## Disclaimer diff --git a/package-lock.json b/package-lock.json index 40baa7776a..4731e27657 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.2.4", + "version": "2.2.5", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.2.4", + "version": "2.2.5", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", diff --git a/package.json b/package.json index d5798bea31..19de801b9e 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "displayName": "Roo Cline", "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.", "publisher": "RooVeterinaryInc", - "version": "2.2.4", + "version": "2.2.5", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index de3c19e052..b497368163 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -633,7 +633,7 @@ npm run build 5. Install the MCP Server by adding the MCP server configuration to the settings file located at '${await mcpHub.getMcpSettingsFilePath()}'. The settings file may have other MCP servers already configured, so you would read it first and then add your new server to the existing \`mcpServers\` object. -IMPORTANT: Regardless of what else you see in the settings file, you must not set any defaults for the \`alwaysAllow\` array in the newly added MCP server. +IMPORTANT: Regardless of what else you see in the MCP settings file, you must default any new MCP servers you create to disabled=false and alwaysAllow=[]. \`\`\`json { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 5877f4fd1d..e998332780 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -567,6 +567,17 @@ export class ClineProvider implements vscode.WebviewViewProvider { } break } + case "toggleMcpServer": { + try { + await this.mcpHub?.toggleServerDisabled( + message.serverName!, + message.disabled! + ) + } catch (error) { + console.error(`Failed to toggle MCP server ${message.serverName}:`, error) + } + break + } // Add more switch case statements here as more webview message commands // are created within the webview context (i.e. inside media/main.js) case "playSound": diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 715410e816..9004a78d6a 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -39,7 +39,8 @@ const StdioConfigSchema = z.object({ command: z.string(), args: z.array(z.string()).optional(), env: z.record(z.string()).optional(), - alwaysAllow: AlwaysAllowSchema.optional() + alwaysAllow: AlwaysAllowSchema.optional(), + disabled: z.boolean().optional() }) const McpSettingsSchema = z.object({ @@ -61,7 +62,10 @@ export class McpHub { } getServers(): McpServer[] { - return this.connections.map((conn) => conn.server) + // Only return enabled servers + return this.connections + .filter((conn) => !conn.server.disabled) + .map((conn) => conn.server) } async getMcpServersPath(): Promise { @@ -117,9 +121,7 @@ export class McpHub { return } try { - vscode.window.showInformationMessage("Updating MCP servers...") await this.updateServerConnections(result.data.mcpServers || {}) - vscode.window.showInformationMessage("MCP servers updated") } catch (error) { console.error("Failed to process MCP settings change:", error) } @@ -202,11 +204,13 @@ export class McpHub { } // valid schema + const parsedConfig = StdioConfigSchema.parse(config) const connection: McpConnection = { server: { name, config: JSON.stringify(config), status: "connecting", + disabled: parsedConfig.disabled, }, client, transport, @@ -466,13 +470,89 @@ export class McpHub { }) } - // Using server + // Public methods for server management + + public async toggleServerDisabled(serverName: string, disabled: boolean): Promise { + let settingsPath: string + try { + settingsPath = await this.getMcpSettingsFilePath() + + // Ensure the settings file exists and is accessible + try { + await fs.access(settingsPath) + } catch (error) { + console.error('Settings file not accessible:', error) + throw new Error('Settings file not accessible') + } + const content = await fs.readFile(settingsPath, "utf-8") + const config = JSON.parse(content) + + // Validate the config structure + if (!config || typeof config !== 'object') { + throw new Error('Invalid config structure') + } + + if (!config.mcpServers || typeof config.mcpServers !== 'object') { + config.mcpServers = {} + } + + if (config.mcpServers[serverName]) { + // Create a new server config object to ensure clean structure + const serverConfig = { + ...config.mcpServers[serverName], + disabled + } + + // Ensure required fields exist + if (!serverConfig.alwaysAllow) { + serverConfig.alwaysAllow = [] + } + + config.mcpServers[serverName] = serverConfig + + // Write the entire config back + const updatedConfig = { + mcpServers: config.mcpServers + } + + await fs.writeFile(settingsPath, JSON.stringify(updatedConfig, null, 2)) + + const connection = this.connections.find(conn => conn.server.name === serverName) + if (connection) { + try { + connection.server.disabled = disabled + + // Only refresh capabilities if connected + if (connection.server.status === "connected") { + connection.server.tools = await this.fetchToolsList(serverName) + connection.server.resources = await this.fetchResourcesList(serverName) + connection.server.resourceTemplates = await this.fetchResourceTemplatesList(serverName) + } + } catch (error) { + console.error(`Failed to refresh capabilities for ${serverName}:`, error) + } + } + + await this.notifyWebviewOfServerChanges() + } + } catch (error) { + console.error("Failed to update server disabled state:", error) + if (error instanceof Error) { + console.error("Error details:", error.message, error.stack) + } + vscode.window.showErrorMessage(`Failed to update server state: ${error instanceof Error ? error.message : String(error)}`) + throw error + } + } async readResource(serverName: string, uri: string): Promise { const connection = this.connections.find((conn) => conn.server.name === serverName) if (!connection) { throw new Error(`No connection found for server: ${serverName}`) } + if (connection.server.disabled) { + throw new Error(`Server "${serverName}" is disabled`) + } return await connection.client.request( { method: "resources/read", @@ -495,6 +575,9 @@ export class McpHub { `No connection found for server: ${serverName}. Please make sure to use MCP servers available under 'Connected MCP Servers'.`, ) } + if (connection.server.disabled) { + throw new Error(`Server "${serverName}" is disabled and cannot be used`) + } return await connection.client.request( { diff --git a/src/services/mcp/__tests__/McpHub.test.ts b/src/services/mcp/__tests__/McpHub.test.ts index cf4899b562..cd63e29ee6 100644 --- a/src/services/mcp/__tests__/McpHub.test.ts +++ b/src/services/mcp/__tests__/McpHub.test.ts @@ -148,6 +148,103 @@ describe('McpHub', () => { }) }) + describe('server disabled state', () => { + it('should toggle server disabled state', async () => { + const mockConfig = { + mcpServers: { + 'test-server': { + command: 'node', + args: ['test.js'], + disabled: false + } + } + } + + // Mock reading initial config + ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) + + await mcpHub.toggleServerDisabled('test-server', true) + + // Verify the config was updated correctly + const writeCall = (fs.writeFile as jest.Mock).mock.calls[0] + const writtenConfig = JSON.parse(writeCall[1]) + expect(writtenConfig.mcpServers['test-server'].disabled).toBe(true) + }) + + it('should filter out disabled servers from getServers', () => { + const mockConnections: McpConnection[] = [ + { + server: { + name: 'enabled-server', + config: '{}', + status: 'connected', + disabled: false + }, + client: {} as any, + transport: {} as any + }, + { + server: { + name: 'disabled-server', + config: '{}', + status: 'connected', + disabled: true + }, + client: {} as any, + transport: {} as any + } + ] + + mcpHub.connections = mockConnections + const servers = mcpHub.getServers() + + expect(servers.length).toBe(1) + expect(servers[0].name).toBe('enabled-server') + }) + + it('should prevent calling tools on disabled servers', async () => { + const mockConnection: McpConnection = { + server: { + name: 'disabled-server', + config: '{}', + status: 'connected', + disabled: true + }, + client: { + request: jest.fn().mockResolvedValue({ result: 'success' }) + } as any, + transport: {} as any + } + + mcpHub.connections = [mockConnection] + + await expect(mcpHub.callTool('disabled-server', 'some-tool', {})) + .rejects + .toThrow('Server "disabled-server" is disabled and cannot be used') + }) + + it('should prevent reading resources from disabled servers', async () => { + const mockConnection: McpConnection = { + server: { + name: 'disabled-server', + config: '{}', + status: 'connected', + disabled: true + }, + client: { + request: jest.fn() + } as any, + transport: {} as any + } + + mcpHub.connections = [mockConnection] + + await expect(mcpHub.readResource('disabled-server', 'some/uri')) + .rejects + .toThrow('Server "disabled-server" is disabled') + }) + }) + describe('callTool', () => { it('should execute tool successfully', async () => { // Mock the connection with a minimal client implementation diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index e7cfe43184..31802b9680 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -36,7 +36,9 @@ export interface WebviewMessage { | "openMcpSettings" | "restartMcpServer" | "toggleToolAlwaysAllow" + | "toggleMcpServer" text?: string + disabled?: boolean askResponse?: ClineAskResponse apiConfiguration?: ApiConfiguration images?: string[] diff --git a/src/shared/mcp.ts b/src/shared/mcp.ts index a00b34328b..7df1415cf4 100644 --- a/src/shared/mcp.ts +++ b/src/shared/mcp.ts @@ -6,6 +6,7 @@ export type McpServer = { tools?: McpTool[] resources?: McpResource[] resourceTemplates?: McpResourceTemplate[] + disabled?: boolean } export type McpTool = { diff --git a/webview-ui/src/components/mcp/McpView.tsx b/webview-ui/src/components/mcp/McpView.tsx index e15c2a1ded..318cbab3bd 100644 --- a/webview-ui/src/components/mcp/McpView.tsx +++ b/webview-ui/src/components/mcp/McpView.tsx @@ -189,6 +189,7 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer, alwaysAllowM background: "var(--vscode-textCodeBlock-background)", cursor: server.error ? "default" : "pointer", borderRadius: isExpanded || server.error ? "4px 4px 0 0" : "4px", + opacity: server.disabled ? 0.6 : 1, }} onClick={handleRowClick}> {!server.error && ( @@ -198,6 +199,55 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer, alwaysAllowM /> )} {server.name} +
e.stopPropagation()}> +
{ + vscode.postMessage({ + type: "toggleMcpServer", + serverName: server.name, + disabled: !server.disabled + }); + }} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + vscode.postMessage({ + type: "toggleMcpServer", + serverName: server.name, + disabled: !server.disabled + }); + } + }} + > +
+
+
Date: Sat, 14 Dec 2024 13:46:26 -0500 Subject: [PATCH 16/57] Support fuzzy matching for apply_diff --- CHANGELOG.md | 4 + package-lock.json | 4 +- package.json | 2 +- src/core/diff/DiffStrategy.ts | 4 +- .../__tests__/search-replace.test.ts | 346 ++++-------------- src/core/diff/strategies/search-replace.ts | 83 ++++- 6 files changed, 146 insertions(+), 297 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3cf1dd13a..23111f993f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Roo Cline Changelog +## [2.2.6] + +- Add a fuzzy match tolerance when applying diffs + ## [2.2.5] - Allow MCP servers to be enabled/disabled diff --git a/package-lock.json b/package-lock.json index 4731e27657..62aa553334 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.2.5", + "version": "2.2.6", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.2.5", + "version": "2.2.6", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", diff --git a/package.json b/package.json index 19de801b9e..fde1305d39 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "displayName": "Roo Cline", "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.", "publisher": "RooVeterinaryInc", - "version": "2.2.5", + "version": "2.2.6", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", diff --git a/src/core/diff/DiffStrategy.ts b/src/core/diff/DiffStrategy.ts index 6562520b7d..355424e48d 100644 --- a/src/core/diff/DiffStrategy.ts +++ b/src/core/diff/DiffStrategy.ts @@ -7,9 +7,9 @@ import { SearchReplaceDiffStrategy } from './strategies/search-replace' * @returns The appropriate diff strategy for the model */ export function getDiffStrategy(model: string): DiffStrategy { - // For now, return SearchReplaceDiffStrategy for all models + // For now, return SearchReplaceDiffStrategy for all models (with a fuzzy threshold of 0.9) // This architecture allows for future optimizations based on model capabilities - return new SearchReplaceDiffStrategy() + return new SearchReplaceDiffStrategy(0.9) } export type { DiffStrategy } diff --git a/src/core/diff/strategies/__tests__/search-replace.test.ts b/src/core/diff/strategies/__tests__/search-replace.test.ts index 1ad32fe3aa..d90ccd0a91 100644 --- a/src/core/diff/strategies/__tests__/search-replace.test.ts +++ b/src/core/diff/strategies/__tests__/search-replace.test.ts @@ -1,18 +1,15 @@ import { SearchReplaceDiffStrategy } from '../search-replace' describe('SearchReplaceDiffStrategy', () => { - let strategy: SearchReplaceDiffStrategy + describe('exact matching', () => { + let strategy: SearchReplaceDiffStrategy - beforeEach(() => { - strategy = new SearchReplaceDiffStrategy() - }) + beforeEach(() => { + strategy = new SearchReplaceDiffStrategy() // Default 1.0 threshold for exact matching + }) - describe('applyDiff', () => { it('should replace matching content', () => { - const originalContent = `function hello() { - console.log("hello") -} -` + const originalContent = 'function hello() {\n console.log("hello")\n}\n' const diffContent = `test.ts <<<<<<< SEARCH function hello() { @@ -25,19 +22,11 @@ function hello() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(`function hello() { - console.log("hello world") -} -`) + expect(result).toBe('function hello() {\n console.log("hello world")\n}\n') }) it('should match content with different surrounding whitespace', () => { - const originalContent = ` -function example() { - return 42; -} - -` + const originalContent = '\nfunction example() {\n return 42;\n}\n\n' const diffContent = `test.ts <<<<<<< SEARCH function example() { @@ -50,19 +39,11 @@ function example() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(` -function example() { - return 43; -} - -`) + expect(result).toBe('\nfunction example() {\n return 43;\n}\n\n') }) it('should match content with different indentation in search block', () => { - const originalContent = ` function test() { - return true; - } -` + const originalContent = ' function test() {\n return true;\n }\n' const diffContent = `test.ts <<<<<<< SEARCH function test() { @@ -75,10 +56,7 @@ function test() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(` function test() { - return false; - } -`) + expect(result).toBe(' function test() {\n return false;\n }\n') }) it('should handle tab-based indentation', () => { @@ -174,10 +152,7 @@ function test() { }) it('should return false if search content does not match', () => { - const originalContent = `function hello() { - console.log("hello") -} -` + const originalContent = 'function hello() {\n console.log("hello")\n}\n' const diffContent = `test.ts <<<<<<< SEARCH function hello() { @@ -194,28 +169,15 @@ function hello() { }) it('should return false if diff format is invalid', () => { - const originalContent = `function hello() { - console.log("hello") -} -` - const diffContent = `test.ts -Invalid diff format` + const originalContent = 'function hello() {\n console.log("hello")\n}\n' + const diffContent = `test.ts\nInvalid diff format` const result = strategy.applyDiff(originalContent, diffContent) expect(result).toBe(false) }) it('should handle multiple lines with proper indentation', () => { - const originalContent = `class Example { - constructor() { - this.value = 0 - } - - getValue() { - return this.value - } -} -` + const originalContent = 'class Example {\n constructor() {\n this.value = 0\n }\n\n getValue() {\n return this.value\n }\n}\n' const diffContent = `test.ts <<<<<<< SEARCH getValue() { @@ -230,18 +192,7 @@ Invalid diff format` >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(`class Example { - constructor() { - this.value = 0 - } - - getValue() { - // Add logging - console.log("Getting value") - return this.value - } -} -`) + expect(result).toBe('class Example {\n constructor() {\n this.value = 0\n }\n\n getValue() {\n // Add logging\n console.log("Getting value")\n return this.value\n }\n}\n') }) it('should preserve whitespace exactly in the output', () => { @@ -262,7 +213,7 @@ Invalid diff format` }) it('should preserve indentation when adding new lines after existing content', () => { - const originalContent = ` onScroll={() => updateHighlights()}` + const originalContent = ' onScroll={() => updateHighlights()}' const diffContent = `test.ts <<<<<<< SEARCH onScroll={() => updateHighlights()} @@ -275,230 +226,78 @@ Invalid diff format` >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(` onScroll={() => updateHighlights()} - onDragOver={(e) => { - e.preventDefault() - e.stopPropagation() - }}`) + expect(result).toBe(' onScroll={() => updateHighlights()}\n onDragOver={(e) => {\n e.preventDefault()\n e.stopPropagation()\n }}') + }) + }) + + describe('fuzzy matching', () => { + let strategy: SearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new SearchReplaceDiffStrategy(0.9) // 90% similarity threshold }) - it('should handle complex refactoring with multiple functions', () => { - const originalContent = `export async function extractTextFromFile(filePath: string): Promise { - try { - await fs.access(filePath) - } catch (error) { - throw new Error(\`File not found: \${filePath}\`) - } - const fileExtension = path.extname(filePath).toLowerCase() - switch (fileExtension) { - case ".pdf": - return extractTextFromPDF(filePath) - case ".docx": - return extractTextFromDOCX(filePath) - case ".ipynb": - return extractTextFromIPYNB(filePath) - default: - const isBinary = await isBinaryFile(filePath).catch(() => false) - if (!isBinary) { - return addLineNumbers(await fs.readFile(filePath, "utf8")) - } else { - throw new Error(\`Cannot read text for file type: \${fileExtension}\`) - } - } -} - -export function addLineNumbers(content: string): string { - const lines = content.split('\\n') - const maxLineNumberWidth = String(lines.length).length - return lines - .map((line, index) => { - const lineNumber = String(index + 1).padStart(maxLineNumberWidth, ' ') - return \`\${lineNumber} | \${line}\` - }).join('\\n') -}` - + it('should match content with small differences (>90% similar)', () => { + const originalContent = 'function getData() {\n const results = fetchData();\n return results.filter(Boolean);\n}\n' const diffContent = `test.ts <<<<<<< SEARCH -export async function extractTextFromFile(filePath: string): Promise { - try { - await fs.access(filePath) - } catch (error) { - throw new Error(\`File not found: \${filePath}\`) - } - const fileExtension = path.extname(filePath).toLowerCase() - switch (fileExtension) { - case ".pdf": - return extractTextFromPDF(filePath) - case ".docx": - return extractTextFromDOCX(filePath) - case ".ipynb": - return extractTextFromIPYNB(filePath) - default: - const isBinary = await isBinaryFile(filePath).catch(() => false) - if (!isBinary) { - return addLineNumbers(await fs.readFile(filePath, "utf8")) - } else { - throw new Error(\`Cannot read text for file type: \${fileExtension}\`) - } - } -} - -export function addLineNumbers(content: string): string { - const lines = content.split('\\n') - const maxLineNumberWidth = String(lines.length).length - return lines - .map((line, index) => { - const lineNumber = String(index + 1).padStart(maxLineNumberWidth, ' ') - return \`\${lineNumber} | \${line}\` - }).join('\\n') +function getData() { + const result = fetchData(); + return results.filter(Boolean); } ======= -function extractLineRange(content: string, startLine?: number, endLine?: number): string { - const lines = content.split('\\n') - const start = startLine ? Math.max(1, startLine) : 1 - const end = endLine ? Math.min(lines.length, endLine) : lines.length - - if (start > end || start > lines.length) { - throw new Error(\`Invalid line range: start=\${start}, end=\${end}, total lines=\${lines.length}\`) - } - - return lines.slice(start - 1, end).join('\\n') -} - -export async function extractTextFromFile(filePath: string, startLine?: number, endLine?: number): Promise { - try { - await fs.access(filePath) - } catch (error) { - throw new Error(\`File not found: \${filePath}\`) - } - const fileExtension = path.extname(filePath).toLowerCase() - let content: string - - switch (fileExtension) { - case ".pdf": { - const dataBuffer = await fs.readFile(filePath) - const data = await pdf(dataBuffer) - content = extractLineRange(data.text, startLine, endLine) - break - } - case ".docx": { - const result = await mammoth.extractRawText({ path: filePath }) - content = extractLineRange(result.value, startLine, endLine) - break - } - case ".ipynb": { - const data = await fs.readFile(filePath, "utf8") - const notebook = JSON.parse(data) - let extractedText = "" - - for (const cell of notebook.cells) { - if ((cell.cell_type === "markdown" || cell.cell_type === "code") && cell.source) { - extractedText += cell.source.join("\\n") + "\\n" - } - } - content = extractLineRange(extractedText, startLine, endLine) - break - } - default: { - const isBinary = await isBinaryFile(filePath).catch(() => false) - if (!isBinary) { - const fileContent = await fs.readFile(filePath, "utf8") - content = extractLineRange(fileContent, startLine, endLine) - } else { - throw new Error(\`Cannot read text for file type: \${fileExtension}\`) - } - } - } - - return addLineNumbers(content, startLine) -} - -export function addLineNumbers(content: string, startLine: number = 1): string { - const lines = content.split('\\n') - const maxLineNumberWidth = String(startLine + lines.length - 1).length - return lines - .map((line, index) => { - const lineNumber = String(startLine + index).padStart(maxLineNumberWidth, ' ') - return \`\${lineNumber} | \${line}\` - }).join('\\n') +function getData() { + const data = fetchData(); + return data.filter(Boolean); } >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - const expected = `function extractLineRange(content: string, startLine?: number, endLine?: number): string { - const lines = content.split('\\n') - const start = startLine ? Math.max(1, startLine) : 1 - const end = endLine ? Math.min(lines.length, endLine) : lines.length - - if (start > end || start > lines.length) { - throw new Error(\`Invalid line range: start=\${start}, end=\${end}, total lines=\${lines.length}\`) - } - - return lines.slice(start - 1, end).join('\\n') -} + expect(result).toBe('function getData() {\n const data = fetchData();\n return data.filter(Boolean);\n}\n') + }) -export async function extractTextFromFile(filePath: string, startLine?: number, endLine?: number): Promise { - try { - await fs.access(filePath) - } catch (error) { - throw new Error(\`File not found: \${filePath}\`) - } - const fileExtension = path.extname(filePath).toLowerCase() - let content: string - - switch (fileExtension) { - case ".pdf": { - const dataBuffer = await fs.readFile(filePath) - const data = await pdf(dataBuffer) - content = extractLineRange(data.text, startLine, endLine) - break - } - case ".docx": { - const result = await mammoth.extractRawText({ path: filePath }) - content = extractLineRange(result.value, startLine, endLine) - break - } - case ".ipynb": { - const data = await fs.readFile(filePath, "utf8") - const notebook = JSON.parse(data) - let extractedText = "" - - for (const cell of notebook.cells) { - if ((cell.cell_type === "markdown" || cell.cell_type === "code") && cell.source) { - extractedText += cell.source.join("\\n") + "\\n" - } - } - content = extractLineRange(extractedText, startLine, endLine) - break - } - default: { - const isBinary = await isBinaryFile(filePath).catch(() => false) - if (!isBinary) { - const fileContent = await fs.readFile(filePath, "utf8") - content = extractLineRange(fileContent, startLine, endLine) - } else { - throw new Error(\`Cannot read text for file type: \${fileExtension}\`) - } - } - } - - return addLineNumbers(content, startLine) + it('should not match when content is too different (<90% similar)', () => { + const originalContent = 'function processUsers(data) {\n return data.map(user => user.name);\n}\n' + const diffContent = `test.ts +<<<<<<< SEARCH +function handleItems(items) { + return items.map(item => item.username); } +======= +function processData(data) { + return data.map(d => d.value); +} +>>>>>>> REPLACE` -export function addLineNumbers(content: string, startLine: number = 1): string { - const lines = content.split('\\n') - const maxLineNumberWidth = String(startLine + lines.length - 1).length - return lines - .map((line, index) => { - const lineNumber = String(startLine + index).padStart(maxLineNumberWidth, ' ') - return \`\${lineNumber} | \${line}\` - }).join('\\n') -}` - expect(result).toBe(expected) + const result = strategy.applyDiff(originalContent, diffContent) + expect(result).toBe(false) + }) + + it('should match content with extra whitespace', () => { + const originalContent = 'function sum(a, b) {\n return a + b;\n}' + const diffContent = `test.ts +<<<<<<< SEARCH +function sum(a, b) { + return a + b; +} +======= +function sum(a, b) { + return a + b + 1; +} +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result).toBe('function sum(a, b) {\n return a + b + 1;\n}') }) }) describe('getToolDescription', () => { + let strategy: SearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new SearchReplaceDiffStrategy() + }) + it('should include the current working directory', () => { const cwd = '/test/dir' const description = strategy.getToolDescription(cwd) @@ -515,4 +314,3 @@ export function addLineNumbers(content: string, startLine: number = 1): string { }) }) }) - diff --git a/src/core/diff/strategies/search-replace.ts b/src/core/diff/strategies/search-replace.ts index d83beb1f56..1e51c3429a 100644 --- a/src/core/diff/strategies/search-replace.ts +++ b/src/core/diff/strategies/search-replace.ts @@ -1,6 +1,59 @@ import { DiffStrategy } from "../types" +function levenshteinDistance(a: string, b: string): number { + const matrix: number[][] = []; + + // Initialize matrix + for (let i = 0; i <= a.length; i++) { + matrix[i] = [i]; + } + for (let j = 0; j <= b.length; j++) { + matrix[0][j] = j; + } + + // Fill matrix + for (let i = 1; i <= a.length; i++) { + for (let j = 1; j <= b.length; j++) { + if (a[i-1] === b[j-1]) { + matrix[i][j] = matrix[i-1][j-1]; + } else { + matrix[i][j] = Math.min( + matrix[i-1][j-1] + 1, // substitution + matrix[i][j-1] + 1, // insertion + matrix[i-1][j] + 1 // deletion + ); + } + } + } + + return matrix[a.length][b.length]; +} + +function getSimilarity(original: string, search: string): number { + // Normalize strings by removing extra whitespace but preserve case + const normalizeStr = (str: string) => str.replace(/\s+/g, ' ').trim(); + + const normalizedOriginal = normalizeStr(original); + const normalizedSearch = normalizeStr(search); + + if (normalizedOriginal === normalizedSearch) { return 1; } + + // Calculate Levenshtein distance + const distance = levenshteinDistance(normalizedOriginal, normalizedSearch); + + // Calculate similarity ratio (0 to 1, where 1 is exact match) + const maxLength = Math.max(normalizedOriginal.length, normalizedSearch.length); + return 1 - (distance / maxLength); +} + export class SearchReplaceDiffStrategy implements DiffStrategy { + private fuzzyThreshold: number; + + constructor(fuzzyThreshold?: number) { + // Default to exact matching (1.0) unless fuzzy threshold specified + this.fuzzyThreshold = fuzzyThreshold ?? 1.0; + } + getToolDescription(cwd: string): string { return `## apply_diff Description: Request to replace existing code using search and replace blocks. @@ -78,30 +131,24 @@ Your search/replace content here const replaceLines = replaceContent.split(/\r?\n/); const originalLines = originalContent.split(/\r?\n/); - // Find the search content in the original + // Find the search content in the original using fuzzy matching let matchIndex = -1; + let bestMatchScore = 0; for (let i = 0; i <= originalLines.length - searchLines.length; i++) { - let found = true; + // Join the lines and calculate overall similarity + const originalChunk = originalLines.slice(i, i + searchLines.length).join('\n'); + const searchChunk = searchLines.join('\n'); - for (let j = 0; j < searchLines.length; j++) { - const originalLine = originalLines[i + j]; - const searchLine = searchLines[j]; - - // Compare lines after removing leading/trailing whitespace - if (originalLine.trim() !== searchLine.trim()) { - found = false; - break; - } - } - - if (found) { + const similarity = getSimilarity(originalChunk, searchChunk); + if (similarity > bestMatchScore) { + bestMatchScore = similarity; matchIndex = i; - break; } } - if (matchIndex === -1) { + // Require similarity to meet threshold + if (matchIndex === -1 || bestMatchScore < this.fuzzyThreshold) { return false; } @@ -121,7 +168,7 @@ Your search/replace content here }); // Apply the replacement while preserving exact indentation - const indentedReplace = replaceLines.map((line, i) => { + const indentedReplaceLines = replaceLines.map((line, i) => { // Get the corresponding original and search indentations const originalIndent = originalIndents[Math.min(i, originalIndents.length - 1)]; const searchIndent = searchIndents[Math.min(i, searchIndents.length - 1)]; @@ -162,6 +209,6 @@ Your search/replace content here const beforeMatch = originalLines.slice(0, matchIndex); const afterMatch = originalLines.slice(matchIndex + searchLines.length); - return [...beforeMatch, ...indentedReplace, ...afterMatch].join(lineEnding); + return [...beforeMatch, ...indentedReplaceLines, ...afterMatch].join(lineEnding); } } From 2292372e427f2044a558eaf0d9bf7cf68679235a Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sat, 14 Dec 2024 20:29:52 -0500 Subject: [PATCH 17/57] Improvements to search/replace diff --- CHANGELOG.md | 4 + package-lock.json | 4 +- package.json | 2 +- .../__tests__/search-replace.test.ts | 329 ++++++++++++++++++ src/core/diff/strategies/search-replace.ts | 94 +++-- src/core/diff/types.ts | 4 +- 6 files changed, 383 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23111f993f..76fdaf74f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Roo Cline Changelog +## [2.2.7] + +- More fixes to search/replace diffs + ## [2.2.6] - Add a fuzzy match tolerance when applying diffs diff --git a/package-lock.json b/package-lock.json index 62aa553334..945e8e7103 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.2.6", + "version": "2.2.7", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.2.6", + "version": "2.2.7", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", diff --git a/package.json b/package.json index fde1305d39..9fda9aa89a 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "displayName": "Roo Cline", "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.", "publisher": "RooVeterinaryInc", - "version": "2.2.6", + "version": "2.2.7", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", diff --git a/src/core/diff/strategies/__tests__/search-replace.test.ts b/src/core/diff/strategies/__tests__/search-replace.test.ts index d90ccd0a91..ee2174a7c1 100644 --- a/src/core/diff/strategies/__tests__/search-replace.test.ts +++ b/src/core/diff/strategies/__tests__/search-replace.test.ts @@ -291,6 +291,329 @@ function sum(a, b) { }) }) + describe('line-constrained search', () => { + let strategy: SearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new SearchReplaceDiffStrategy() + }) + + it('should find and replace within specified line range', () => { + const originalContent = ` +function one() { + return 1; +} + +function two() { + return 2; +} + +function three() { + return 3; +} +`.trim() + const diffContent = `test.ts +<<<<<<< SEARCH +function two() { + return 2; +} +======= +function two() { + return "two"; +} +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent, 5, 7) + expect(result).toBe(`function one() { + return 1; +} + +function two() { + return "two"; +} + +function three() { + return 3; +}`) + }) + + it('should find and replace within buffer zone (5 lines before/after)', () => { + const originalContent = ` +function one() { + return 1; +} + +function two() { + return 2; +} + +function three() { + return 3; +} +`.trim() + const diffContent = `test.ts +<<<<<<< SEARCH +function three() { + return 3; +} +======= +function three() { + return "three"; +} +>>>>>>> REPLACE` + + // Even though we specify lines 5-7, it should still find the match at lines 9-11 + // because it's within the 5-line buffer zone + const result = strategy.applyDiff(originalContent, diffContent, 5, 7) + expect(result).toBe(`function one() { + return 1; +} + +function two() { + return 2; +} + +function three() { + return "three"; +}`) + }) + + it('should not find matches outside search range and buffer zone', () => { + const originalContent = ` +function one() { + return 1; +} + +function two() { + return 2; +} + +function three() { + return 3; +} + +function four() { + return 4; +} + +function five() { + return 5; +} +`.trim() + const diffContent = `test.ts +<<<<<<< SEARCH +function five() { + return 5; +} +======= +function five() { + return "five"; +} +>>>>>>> REPLACE` + + // Searching around function two() (lines 5-7) + // function five() is more than 5 lines away, so it shouldn't match + const result = strategy.applyDiff(originalContent, diffContent, 5, 7) + expect(result).toBe(false) + }) + + it('should handle search range at start of file', () => { + const originalContent = ` +function one() { + return 1; +} + +function two() { + return 2; +} +`.trim() + const diffContent = `test.ts +<<<<<<< SEARCH +function one() { + return 1; +} +======= +function one() { + return "one"; +} +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent, 1, 3) + expect(result).toBe(`function one() { + return "one"; +} + +function two() { + return 2; +}`) + }) + + it('should handle search range at end of file', () => { + const originalContent = ` +function one() { + return 1; +} + +function two() { + return 2; +} +`.trim() + const diffContent = `test.ts +<<<<<<< SEARCH +function two() { + return 2; +} +======= +function two() { + return "two"; +} +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent, 5, 7) + expect(result).toBe(`function one() { + return 1; +} + +function two() { + return "two"; +}`) + }) + + it('should match specific instance of duplicate code using line numbers', () => { + const originalContent = ` +function processData(data) { + return data.map(x => x * 2); +} + +function unrelatedStuff() { + console.log("hello"); +} + +// Another data processor +function processData(data) { + return data.map(x => x * 2); +} + +function moreStuff() { + console.log("world"); +} +`.trim() + const diffContent = `test.ts +<<<<<<< SEARCH +function processData(data) { + return data.map(x => x * 2); +} +======= +function processData(data) { + // Add logging + console.log("Processing data..."); + return data.map(x => x * 2); +} +>>>>>>> REPLACE` + + // Target the second instance of processData + const result = strategy.applyDiff(originalContent, diffContent, 10, 12) + expect(result).toBe(`function processData(data) { + return data.map(x => x * 2); +} + +function unrelatedStuff() { + console.log("hello"); +} + +// Another data processor +function processData(data) { + // Add logging + console.log("Processing data..."); + return data.map(x => x * 2); +} + +function moreStuff() { + console.log("world"); +}`) + }) + + it('should search from start line to end of file when only start_line is provided', () => { + const originalContent = ` +function one() { + return 1; +} + +function two() { + return 2; +} + +function three() { + return 3; +} +`.trim() + const diffContent = `test.ts +<<<<<<< SEARCH +function three() { + return 3; +} +======= +function three() { + return "three"; +} +>>>>>>> REPLACE` + + // Only provide start_line, should search from there to end of file + const result = strategy.applyDiff(originalContent, diffContent, 8) + expect(result).toBe(`function one() { + return 1; +} + +function two() { + return 2; +} + +function three() { + return "three"; +}`) + }) + + it('should search from start of file to end line when only end_line is provided', () => { + const originalContent = ` +function one() { + return 1; +} + +function two() { + return 2; +} + +function three() { + return 3; +} +`.trim() + const diffContent = `test.ts +<<<<<<< SEARCH +function one() { + return 1; +} +======= +function one() { + return "one"; +} +>>>>>>> REPLACE` + + // Only provide end_line, should search from start of file to there + const result = strategy.applyDiff(originalContent, diffContent, undefined, 4) + expect(result).toBe(`function one() { + return "one"; +} + +function two() { + return 2; +} + +function three() { + return 3; +}`) + }) + }) + describe('getToolDescription', () => { let strategy: SearchReplaceDiffStrategy @@ -312,5 +635,11 @@ function sum(a, b) { expect(description).toContain('') expect(description).toContain('') }) + + it('should document start_line and end_line parameters', () => { + const description = strategy.getToolDescription('/test') + expect(description).toContain('start_line: (required) The line number where the search block starts.') + expect(description).toContain('end_line: (required) The line number where the search block ends.') + }) }) }) diff --git a/src/core/diff/strategies/search-replace.ts b/src/core/diff/strategies/search-replace.ts index 1e51c3429a..ee0272eed7 100644 --- a/src/core/diff/strategies/search-replace.ts +++ b/src/core/diff/strategies/search-replace.ts @@ -56,7 +56,7 @@ export class SearchReplaceDiffStrategy implements DiffStrategy { getToolDescription(cwd: string): string { return `## apply_diff -Description: Request to replace existing code using search and replace blocks. +Description: Request to replace existing code using a search and replace block. This tool allows for precise, surgical replaces to files by specifying exactly what content to search for and what to replace it with. The tool will maintain proper indentation and formatting while making changes. Only a single operation is allowed per tool use. @@ -65,33 +65,32 @@ If you're not confident in the exact content to search for, use the read_file to Parameters: - path: (required) The path of the file to modify (relative to the current working directory ${cwd}) -- diff: (required) The search/replace blocks defining the changes. +- diff: (required) The search/replace block defining the changes. +- start_line: (required) The line number where the search block starts. +- end_line: (required) The line number where the search block ends. -Format: -1. First line must be the file path -2. Followed by search/replace blocks: - \`\`\` - <<<<<<< SEARCH - [exact content to find including whitespace] - ======= - [new content to replace with] - >>>>>>> REPLACE - \`\`\` +Diff format: +\`\`\` +<<<<<<< SEARCH +[exact content to find including whitespace] +======= +[new content to replace with] +>>>>>>> REPLACE +\`\`\` Example: Original file: \`\`\` -def calculate_total(items): - total = 0 - for item in items: - total += item - return total +1 | def calculate_total(items): +2 | total = 0 +3 | for item in items: +4 | total += item +5 | return total \`\`\` Search/Replace content: \`\`\` -main.py <<<<<<< SEARCH def calculate_total(items): total = 0 @@ -111,10 +110,12 @@ Usage: Your search/replace content here +1 +5 ` } - applyDiff(originalContent: string, diffContent: string): string | false { + applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): string | false { // Extract the search and replace blocks const match = diffContent.match(/<<<<<<< SEARCH\n([\s\S]*?)\n=======\n([\s\S]*?)\n>>>>>>> REPLACE/); if (!match) { @@ -131,11 +132,25 @@ Your search/replace content here const replaceLines = replaceContent.split(/\r?\n/); const originalLines = originalContent.split(/\r?\n/); + // Determine search range based on provided line numbers + let searchStartIndex = 0; + let searchEndIndex = originalLines.length; + + if (startLine !== undefined || endLine !== undefined) { + // Convert to 0-based index and add buffer + if (startLine !== undefined) { + searchStartIndex = Math.max(0, startLine - 6); + } + if (endLine !== undefined) { + searchEndIndex = Math.min(originalLines.length, endLine + 5); + } + } + // Find the search content in the original using fuzzy matching let matchIndex = -1; let bestMatchScore = 0; - for (let i = 0; i <= originalLines.length - searchLines.length; i++) { + for (let i = searchStartIndex; i <= searchEndIndex - searchLines.length; i++) { // Join the lines and calculate overall similarity const originalChunk = originalLines.slice(i, i + searchLines.length).join('\n'); const searchChunk = searchLines.join('\n'); @@ -169,40 +184,19 @@ Your search/replace content here // Apply the replacement while preserving exact indentation const indentedReplaceLines = replaceLines.map((line, i) => { - // Get the corresponding original and search indentations - const originalIndent = originalIndents[Math.min(i, originalIndents.length - 1)]; - const searchIndent = searchIndents[Math.min(i, searchIndents.length - 1)]; + // Get the matched line's exact indentation + const matchedIndent = originalIndents[0]; - // Get the current line's indentation + // Get the current line's indentation relative to the search content const currentIndentMatch = line.match(/^[\t ]*/); const currentIndent = currentIndentMatch ? currentIndentMatch[0] : ''; + const searchBaseIndent = searchIndents[0] || ''; - // Get the corresponding search line's indentation - const searchLineIndex = Math.min(i, searchLines.length - 1); - const searchLineIndent = searchIndents[searchLineIndex]; - - // Get the corresponding original line's indentation - const originalLineIndex = Math.min(i, originalIndents.length - 1); - const originalLineIndent = originalIndents[originalLineIndex]; - - // If this line has the same indentation as its corresponding search line, - // use the original indentation - if (currentIndent === searchLineIndent) { - return originalLineIndent + line.trim(); - } - - // Otherwise, preserve the original indentation structure - const indentChar = originalLineIndent.charAt(0) || '\t'; - const indentLevel = Math.floor(originalLineIndent.length / indentChar.length); - - // Calculate the relative indentation from the search line - const searchLevel = Math.floor(searchLineIndent.length / indentChar.length); - const currentLevel = Math.floor(currentIndent.length / indentChar.length); - const relativeLevel = currentLevel - searchLevel; - - // Apply the relative indentation to the original level - const targetLevel = Math.max(0, indentLevel + relativeLevel); - return indentChar.repeat(targetLevel) + line.trim(); + // Calculate the relative indentation from the search content + const relativeIndent = currentIndent.slice(searchBaseIndent.length); + + // Apply the matched indentation plus any relative indentation + return matchedIndent + relativeIndent + line.trim(); }); // Construct the final content diff --git a/src/core/diff/types.ts b/src/core/diff/types.ts index f4cdf176aa..e5d4478fc8 100644 --- a/src/core/diff/types.ts +++ b/src/core/diff/types.ts @@ -13,7 +13,9 @@ export interface DiffStrategy { * Apply a diff to the original content * @param originalContent The original file content * @param diffContent The diff content in the strategy's format + * @param startLine Optional line number where the search block starts. If not provided, searches the entire file. + * @param endLine Optional line number where the search block ends. If not provided, searches the entire file. * @returns The new content after applying the diff, or false if the diff could not be applied */ - applyDiff(originalContent: string, diffContent: string): string | false + applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): string | false } From e4c23fb61dd413fe13e8e6b1dbbae3c371b11c04 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 15 Dec 2024 15:19:58 -0500 Subject: [PATCH 18/57] Prioritize exact line matches in search/replace --- .../__tests__/search-replace.test.ts | 89 +++++++++++++++++++ src/core/diff/strategies/search-replace.ts | 69 ++++++++------ 2 files changed, 133 insertions(+), 25 deletions(-) diff --git a/src/core/diff/strategies/__tests__/search-replace.test.ts b/src/core/diff/strategies/__tests__/search-replace.test.ts index ee2174a7c1..8aa50ee0ac 100644 --- a/src/core/diff/strategies/__tests__/search-replace.test.ts +++ b/src/core/diff/strategies/__tests__/search-replace.test.ts @@ -610,6 +610,95 @@ function two() { function three() { return 3; +}`) + }) + + it('should prioritize exact line match over expanded search', () => { + const originalContent = ` +function one() { + return 1; +} + +function process() { + return "old"; +} + +function process() { + return "old"; +} + +function two() { + return 2; +}` + const diffContent = `test.ts +<<<<<<< SEARCH +function process() { + return "old"; +} +======= +function process() { + return "new"; +} +>>>>>>> REPLACE` + + // Should match the second instance exactly at lines 10-12 + // even though the first instance at 6-8 is within the expanded search range + const result = strategy.applyDiff(originalContent, diffContent, 10, 12) + expect(result).toBe(` +function one() { + return 1; +} + +function process() { + return "old"; +} + +function process() { + return "new"; +} + +function two() { + return 2; +}`) + }) + + it('should fall back to expanded search only if exact match fails', () => { + const originalContent = ` +function one() { + return 1; +} + +function process() { + return "target"; +} + +function two() { + return 2; +}`.trim() + const diffContent = `test.ts +<<<<<<< SEARCH +function process() { + return "target"; +} +======= +function process() { + return "updated"; +} +>>>>>>> REPLACE` + + // Specify wrong line numbers (3-5), but content exists at 6-8 + // Should still find and replace it since it's within the expanded range + const result = strategy.applyDiff(originalContent, diffContent, 3, 5) + expect(result).toBe(`function one() { + return 1; +} + +function process() { + return "updated"; +} + +function two() { + return 2; }`) }) }) diff --git a/src/core/diff/strategies/search-replace.ts b/src/core/diff/strategies/search-replace.ts index ee0272eed7..3d524b1ddc 100644 --- a/src/core/diff/strategies/search-replace.ts +++ b/src/core/diff/strategies/search-replace.ts @@ -132,41 +132,60 @@ Your search/replace content here const replaceLines = replaceContent.split(/\r?\n/); const originalLines = originalContent.split(/\r?\n/); - // Determine search range based on provided line numbers - let searchStartIndex = 0; - let searchEndIndex = originalLines.length; - - if (startLine !== undefined || endLine !== undefined) { - // Convert to 0-based index and add buffer - if (startLine !== undefined) { - searchStartIndex = Math.max(0, startLine - 6); - } - if (endLine !== undefined) { - searchEndIndex = Math.min(originalLines.length, endLine + 5); - } - } - - // Find the search content in the original using fuzzy matching + // First try exact line range if provided let matchIndex = -1; let bestMatchScore = 0; - for (let i = searchStartIndex; i <= searchEndIndex - searchLines.length; i++) { - // Join the lines and calculate overall similarity - const originalChunk = originalLines.slice(i, i + searchLines.length).join('\n'); + if (startLine !== undefined && endLine !== undefined) { + // Convert to 0-based index + const exactStartIndex = startLine - 1; + const exactEndIndex = endLine - 1; + + // Check exact range first + const originalChunk = originalLines.slice(exactStartIndex, exactEndIndex + 1).join('\n'); const searchChunk = searchLines.join('\n'); const similarity = getSimilarity(originalChunk, searchChunk); - if (similarity > bestMatchScore) { + if (similarity >= this.fuzzyThreshold) { + matchIndex = exactStartIndex; bestMatchScore = similarity; - matchIndex = i; } } - + + // If no match found in exact range, try expanded range + if (matchIndex === -1) { + let searchStartIndex = 0; + let searchEndIndex = originalLines.length; + + if (startLine !== undefined || endLine !== undefined) { + // Convert to 0-based index and add buffer + if (startLine !== undefined) { + searchStartIndex = Math.max(0, startLine - 6); + } + if (endLine !== undefined) { + searchEndIndex = Math.min(originalLines.length, endLine + 5); + } + } + + // Find the search content in the expanded range using fuzzy matching + for (let i = searchStartIndex; i <= searchEndIndex - searchLines.length; i++) { + // Join the lines and calculate overall similarity + const originalChunk = originalLines.slice(i, i + searchLines.length).join('\n'); + const searchChunk = searchLines.join('\n'); + + const similarity = getSimilarity(originalChunk, searchChunk); + if (similarity > bestMatchScore) { + bestMatchScore = similarity; + matchIndex = i; + } + } + } + // Require similarity to meet threshold if (matchIndex === -1 || bestMatchScore < this.fuzzyThreshold) { return false; } - + // Get the matched lines from the original content const matchedLines = originalLines.slice(matchIndex, matchIndex + searchLines.length); @@ -175,13 +194,13 @@ Your search/replace content here const match = line.match(/^[\t ]*/); return match ? match[0] : ''; }); - + // Get the exact indentation of each line in the search block const searchIndents = searchLines.map(line => { const match = line.match(/^[\t ]*/); return match ? match[0] : ''; }); - + // Apply the replacement while preserving exact indentation const indentedReplaceLines = replaceLines.map((line, i) => { // Get the matched line's exact indentation @@ -198,7 +217,7 @@ Your search/replace content here // Apply the matched indentation plus any relative indentation return matchedIndent + relativeIndent + line.trim(); }); - + // Construct the final content const beforeMatch = originalLines.slice(0, matchIndex); const afterMatch = originalLines.slice(matchIndex + searchLines.length); From 468d317f2f2d625f4a4427bfba35c3e0c05eccde Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 15 Dec 2024 15:28:24 -0500 Subject: [PATCH 19/57] More indentation tests --- .../__tests__/search-replace.test.ts | 190 ++++++++++++++++++ 1 file changed, 190 insertions(+) diff --git a/src/core/diff/strategies/__tests__/search-replace.test.ts b/src/core/diff/strategies/__tests__/search-replace.test.ts index 8aa50ee0ac..cdfa137986 100644 --- a/src/core/diff/strategies/__tests__/search-replace.test.ts +++ b/src/core/diff/strategies/__tests__/search-replace.test.ts @@ -228,6 +228,196 @@ function hello() { const result = strategy.applyDiff(originalContent, diffContent) expect(result).toBe(' onScroll={() => updateHighlights()}\n onDragOver={(e) => {\n e.preventDefault()\n e.stopPropagation()\n }}') }) + + it('should handle varying indentation levels correctly', () => { + const originalContent = ` +class Example { + constructor() { + this.value = 0; + if (true) { + this.init(); + } + } +}`.trim(); + + const diffContent = `test.ts +<<<<<<< SEARCH + class Example { + constructor() { + this.value = 0; + if (true) { + this.init(); + } + } + } +======= + class Example { + constructor() { + this.value = 1; + if (true) { + this.init(); + this.setup(); + this.validate(); + } + } + } +>>>>>>> REPLACE`.trim(); + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result).toBe(` +class Example { + constructor() { + this.value = 1; + if (true) { + this.init(); + this.setup(); + this.validate(); + } + } +}`.trim()); + }); + + it('should handle mixed indentation styles in the same file', () => { + const originalContent = `class Example { + constructor() { + this.value = 0; + if (true) { + this.init(); + } + } +}`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + constructor() { + this.value = 0; + if (true) { + this.init(); + } + } +======= + constructor() { + this.value = 1; + if (true) { + this.init(); + this.validate(); + } + } +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result).toBe(`class Example { + constructor() { + this.value = 1; + if (true) { + this.init(); + this.validate(); + } + } +}`); + }); + + it('should handle Python-style significant whitespace', () => { + const originalContent = `def example(): + if condition: + do_something() + for item in items: + process(item) + return True`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + if condition: + do_something() + for item in items: + process(item) +======= + if condition: + do_something() + while items: + item = items.pop() + process(item) +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result).toBe(`def example(): + if condition: + do_something() + while items: + item = items.pop() + process(item) + return True`); + }); + + it('should preserve empty lines with indentation', () => { + const originalContent = `function test() { + const x = 1; + + if (x) { + return true; + } +}`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + const x = 1; + + if (x) { +======= + const x = 1; + + // Check x + if (x) { +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result).toBe(`function test() { + const x = 1; + + // Check x + if (x) { + return true; + } +}`); + }); + + it('should handle indentation when replacing entire blocks', () => { + const originalContent = `class Test { + method() { + if (true) { + console.log("test"); + } + } +}`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + method() { + if (true) { + console.log("test"); + } + } +======= + method() { + try { + if (true) { + console.log("test"); + } + } catch (e) { + console.error(e); + } + } +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result).toBe(`class Test { + method() { + try { + if (true) { + console.log("test"); + } + } catch (e) { + console.error(e); + } + } +}`); + }); }) describe('fuzzy matching', () => { From 8159c51b03235928b76a2a96550eae3f974a49a6 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 15 Dec 2024 15:41:10 -0500 Subject: [PATCH 20/57] Update omission format and keywords --- .../editor/__tests__/detect-omission.test.ts | 66 +++++++++++++++++++ src/integrations/editor/detect-omission.ts | 3 +- 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 src/integrations/editor/__tests__/detect-omission.test.ts diff --git a/src/integrations/editor/__tests__/detect-omission.test.ts b/src/integrations/editor/__tests__/detect-omission.test.ts new file mode 100644 index 0000000000..4740b1f34f --- /dev/null +++ b/src/integrations/editor/__tests__/detect-omission.test.ts @@ -0,0 +1,66 @@ +import { detectCodeOmission } from '../detect-omission' + +describe('detectCodeOmission', () => { + const originalContent = `function example() { + // Some code + const x = 1; + const y = 2; + return x + y; +}` + + it('should detect square bracket line range omission', () => { + const newContent = `[Previous content from line 1-305 remains exactly the same] +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should detect single-line comment omission', () => { + const newContent = `// Lines 1-50 remain unchanged +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should detect multi-line comment omission', () => { + const newContent = `/* Previous content remains the same */ +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should detect HTML-style comment omission', () => { + const newContent = ` +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should detect JSX-style comment omission', () => { + const newContent = `{/* Rest of the code remains the same */} +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should detect Python-style comment omission', () => { + const newContent = `# Previous content remains unchanged +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should not detect regular comments without omission keywords', () => { + const newContent = `// Adding new functionality +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(false) + }) + + it('should not detect when comment is part of original content', () => { + const originalWithComment = `// Content remains unchanged +${originalContent}` + const newContent = `// Content remains unchanged +const z = 3;` + expect(detectCodeOmission(originalWithComment, newContent)).toBe(false) + }) + + it('should not detect code that happens to contain omission keywords', () => { + const newContent = `const remains = 'some value'; +const unchanged = true;` + expect(detectCodeOmission(originalContent, newContent)).toBe(false) + }) +}) \ No newline at end of file diff --git a/src/integrations/editor/detect-omission.ts b/src/integrations/editor/detect-omission.ts index 565ebd3ace..5cb0f8e419 100644 --- a/src/integrations/editor/detect-omission.ts +++ b/src/integrations/editor/detect-omission.ts @@ -7,7 +7,7 @@ export function detectCodeOmission(originalFileContent: string, newFileContent: string): boolean { const originalLines = originalFileContent.split("\n") const newLines = newFileContent.split("\n") - const omissionKeywords = ["remain", "remains", "unchanged", "rest", "previous", "existing", "..."] + const omissionKeywords = ["remain", "remains", "unchanged", "rest", "previous", "existing", "content", "same", "..."] const commentPatterns = [ /^\s*\/\//, // Single-line comment for most languages @@ -15,6 +15,7 @@ export function detectCodeOmission(originalFileContent: string, newFileContent: /^\s*\/\*/, // Multi-line comment opening /^\s*{\s*\/\*/, // JSX comment opening /^\s*