diff --git a/.changeset/eighty-nails-peel.md b/.changeset/eighty-nails-peel.md new file mode 100644 index 0000000..1810d53 --- /dev/null +++ b/.changeset/eighty-nails-peel.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +Add volume slider in settings and change sound effects to only trigger when user intervention is required, an error occurs, or a task is completed. diff --git a/.changeset/shaggy-moons-dance.md b/.changeset/shaggy-moons-dance.md new file mode 100644 index 0000000..e8de22b --- /dev/null +++ b/.changeset/shaggy-moons-dance.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +Fix lint errors and change npm run lint to also run on webview-ui diff --git a/CHANGELOG.md b/CHANGELOG.md index eeba972..f7db7df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Roo Cline Changelog +## [2.2.12] + +- Better support for pure deletion and insertion diffs + ## [2.2.11] - Added settings checkbox for verbose diff debugging diff --git a/package-lock.json b/package-lock.json index e79bda5..30da4c0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.2.11", + "version": "2.2.12", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.2.11", + "version": "2.2.12", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", @@ -34,10 +34,10 @@ "os-name": "^6.0.0", "p-wait-for": "^5.0.2", "pdf-parse": "^1.1.1", - "play-sound": "^1.1.6", "puppeteer-chromium-resolver": "^23.0.0", "puppeteer-core": "^23.4.0", "serialize-error": "^11.0.3", + "sound-play": "^1.1.0", "strip-ansi": "^7.1.0", "tree-sitter-wasms": "^0.1.11", "turndown": "^7.2.0", @@ -8851,14 +8851,6 @@ "node": ">=8" } }, - "node_modules/find-exec": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/find-exec/-/find-exec-1.0.3.tgz", - "integrity": "sha512-gnG38zW90mS8hm5smNcrBnakPEt+cGJoiMkJwCU0IYnEb0H2NQk0NIljhNW+48oniCriFek/PH6QXbwsJo/qug==", - "dependencies": { - "shell-quote": "^1.8.1" - } - }, "node_modules/find-up": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/find-up/-/find-up-5.0.0.tgz", @@ -13103,14 +13095,6 @@ "node": ">=8" } }, - "node_modules/play-sound": { - "version": "1.1.6", - "resolved": "https://registry.npmjs.org/play-sound/-/play-sound-1.1.6.tgz", - "integrity": "sha512-09eO4QiXNFXJffJaOW5P6x6F5RLihpLUkXttvUZeWml0fU6x6Zp7AjG9zaeMpgH2ZNvq4GR1ytB22ddYcqJIZA==", - "dependencies": { - "find-exec": "1.0.3" - } - }, "node_modules/possible-typed-array-names": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/possible-typed-array-names/-/possible-typed-array-names-1.0.0.tgz", @@ -13874,6 +13858,7 @@ "version": "1.8.2", "resolved": "https://registry.npmjs.org/shell-quote/-/shell-quote-1.8.2.tgz", "integrity": "sha512-AzqKpGKjrj7EM6rKVQEPpB288oCfnrEIuyoT9cyF4nmGa7V8Zk6f7RRqYisX8X9m+Q7bd632aZW4ky7EhbQztA==", + "dev": true, "engines": { "node": ">= 0.4" }, @@ -14002,6 +13987,11 @@ "node": ">= 14" } }, + "node_modules/sound-play": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/sound-play/-/sound-play-1.1.0.tgz", + "integrity": "sha512-Bd/L0AoCwITFeOnpNLMsfPXrV5GG5NhrC/T6odveahYbhPZkdTnrFXRia9FCC5WBWdUTw1d+yvLBvi4wnD1xOA==" + }, "node_modules/source-map": { "version": "0.6.1", "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.6.1.tgz", diff --git a/package.json b/package.json index 3d57994..83bd01c 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.11", + "version": "2.2.12", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", @@ -153,7 +153,7 @@ "compile": "npm run check-types && npm run lint && node esbuild.js", "compile-tests": "tsc -p . --outDir out", "install:all": "npm install && cd webview-ui && npm install", - "lint": "eslint src --ext ts", + "lint": "eslint src --ext ts && npm run lint --prefix webview-ui", "package": "npm run build:webview && npm run check-types && npm run lint && node esbuild.js --production", "pretest": "npm run compile-tests && npm run compile && npm run lint", "start:webview": "cd webview-ui && npm run start", @@ -217,10 +217,10 @@ "os-name": "^6.0.0", "p-wait-for": "^5.0.2", "pdf-parse": "^1.1.1", - "play-sound": "^1.1.6", "puppeteer-chromium-resolver": "^23.0.0", "puppeteer-core": "^23.4.0", "serialize-error": "^11.0.3", + "sound-play": "^1.1.0", "strip-ansi": "^7.1.0", "tree-sitter-wasms": "^0.1.11", "turndown": "^7.2.0", diff --git a/src/core/diff/strategies/__tests__/search-replace.test.ts b/src/core/diff/strategies/__tests__/search-replace.test.ts index 8e48680..f96aa17 100644 --- a/src/core/diff/strategies/__tests__/search-replace.test.ts +++ b/src/core/diff/strategies/__tests__/search-replace.test.ts @@ -591,6 +591,26 @@ this.init(); expect(result.content).toBe('function test() {\n return false;\n}\n') } }) + + it('should strip line numbers with leading spaces', () => { + const originalContent = 'function test() {\n return true;\n}\n' + const diffContent = `test.ts +<<<<<<< SEARCH + 1 | function test() { + 2 | return true; + 3 | } +======= + 1 | function test() { + 2 | return false; + 3 | } +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('function test() {\n return false;\n}\n') + } + }) it('should not strip when not all lines have numbers in either section', () => { const originalContent = 'function test() {\n return true;\n}\n' @@ -711,6 +731,212 @@ this.init(); }) }); + describe('insertion/deletion', () => { + let strategy: SearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new SearchReplaceDiffStrategy() + }) + + describe('deletion', () => { + it('should delete code when replace block is empty', () => { + const originalContent = `function test() { + console.log("hello"); + // Comment to remove + console.log("world"); +}` + const diffContent = `test.ts +<<<<<<< SEARCH + // Comment to remove +======= +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function test() { + console.log("hello"); + console.log("world"); +}`) + } + }) + + it('should delete multiple lines when replace block is empty', () => { + const originalContent = `class Example { + constructor() { + // Initialize + this.value = 0; + // Set defaults + this.name = ""; + // End init + } +}` + const diffContent = `test.ts +<<<<<<< SEARCH + // Initialize + this.value = 0; + // Set defaults + this.name = ""; + // End init +======= +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`class Example { + constructor() { + } +}`) + } + }) + + it('should preserve indentation when deleting nested code', () => { + const originalContent = `function outer() { + if (true) { + // Remove this + console.log("test"); + // And this + } + return true; +}` + const diffContent = `test.ts +<<<<<<< SEARCH + // Remove this + console.log("test"); + // And this +======= +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function outer() { + if (true) { + } + return true; +}`) + } + }) + }) + + describe('insertion', () => { + it('should insert code at specified line when search block is empty', () => { + const originalContent = `function test() { + const x = 1; + return x; +}` + const diffContent = `test.ts +<<<<<<< SEARCH +======= + console.log("Adding log"); +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent, 2, 2) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function test() { + console.log("Adding log"); + const x = 1; + return x; +}`) + } + }) + + it('should preserve indentation when inserting at nested location', () => { + const originalContent = `function test() { + if (true) { + const x = 1; + } +}` + const diffContent = `test.ts +<<<<<<< SEARCH +======= + console.log("Before"); + console.log("After"); +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent, 3, 3) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function test() { + if (true) { + console.log("Before"); + console.log("After"); + const x = 1; + } +}`) + } + }) + + it('should handle insertion at start of file', () => { + const originalContent = `function test() { + return true; +}` + const diffContent = `test.ts +<<<<<<< SEARCH +======= +// Copyright 2024 +// License: MIT + +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent, 1, 1) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`// Copyright 2024 +// License: MIT + +function test() { + return true; +}`) + } + }) + + it('should handle insertion at end of file', () => { + const originalContent = `function test() { + return true; +}` + const diffContent = `test.ts +<<<<<<< SEARCH +======= + +// End of file +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent, 4, 4) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function test() { + return true; +} + +// End of file`) + } + }) + + it('should insert at the start of the file if no start_line is provided for insertion', () => { + const originalContent = `function test() { + return true; +}` + const diffContent = `test.ts +<<<<<<< SEARCH +======= +console.log("test"); +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`console.log("test"); +function test() { + return true; +}`) + } + }) + }) + }) + describe('fuzzy matching', () => { let strategy: SearchReplaceDiffStrategy @@ -1241,8 +1467,8 @@ function two() { 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.') + expect(description).toContain('start_line: (required) The line number where the search block starts (inclusive).') + expect(description).toContain('end_line: (required) The line number where the search block ends (inclusive).') }) }) }) diff --git a/src/core/diff/strategies/search-replace.ts b/src/core/diff/strategies/search-replace.ts index 2fbfe5b..9153c4e 100644 --- a/src/core/diff/strategies/search-replace.ts +++ b/src/core/diff/strategies/search-replace.ts @@ -33,6 +33,10 @@ function levenshteinDistance(a: string, b: string): number { } function getSimilarity(original: string, search: string): number { + if (original === '' || search === '') { + return 1; + } + // Normalize strings by removing extra whitespace but preserve case const normalizeStr = (str: string) => str.replace(/\s+/g, ' ').trim(); @@ -71,8 +75,8 @@ 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 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. +- start_line: (required) The line number where the search block starts (inclusive). +- end_line: (required) The line number where the search block ends (inclusive). Diff format: \`\`\` @@ -94,35 +98,84 @@ Original file: 5 | return total \`\`\` -Search/Replace content: +1. Search/replace a specific chunk of code: \`\`\` + +File path here + <<<<<<< SEARCH -def calculate_total(items): total = 0 for item in items: total += item return total ======= -def calculate_total(items): """Calculate total with 10% markup""" return sum(item * 1.1 for item in items) >>>>>>> REPLACE + +2 +5 + \`\`\` -Usage: +Result: +\`\`\` +1 | def calculate_total(items): +2 | """Calculate total with 10% markup""" +3 | return sum(item * 1.1 for item in items) +\`\`\` + +2. Insert code at a specific line (start_line and end_line must be the same, and the content gets inserted before whatever is currently at that line): +\`\`\` File path here -Your search/replace content here +<<<<<<< SEARCH +======= + """TODO: Write a test for this""" +>>>>>>> REPLACE -1 +2 +2 + +\`\`\` + +Result: +\`\`\` +1 | def calculate_total(items): +2 | """TODO: Write a test for this""" +3 | """Calculate total with 10% markup""" +4 | return sum(item * 1.1 for item in items) +\`\`\` + +3. Delete code at a specific line range: +\`\`\` + +File path here + +<<<<<<< SEARCH + total = 0 + for item in items: + total += item + return total +======= +>>>>>>> REPLACE + +2 5 -` + +\`\`\` + +Result: +\`\`\` +1 | def calculate_total(items): +\`\`\` +` } applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): DiffResult { // Extract the search and replace blocks - const match = diffContent.match(/<<<<<<< SEARCH\n([\s\S]*?)\n=======\n([\s\S]*?)\n>>>>>>> REPLACE/); + const match = diffContent.match(/<<<<<<< SEARCH\n([\s\S]*?)\n?=======\n([\s\S]*?)\n?>>>>>>> REPLACE/); if (!match) { const debugInfo = this.debugEnabled ? `\n\nDebug Info:\n- Expected Format: <<<<<<< SEARCH\\n[search content]\\n=======\\n[replace content]\\n>>>>>>> REPLACE\n- Tip: Make sure to include both SEARCH and REPLACE sections with correct markers` : ''; @@ -133,19 +186,19 @@ Your search/replace content here } let [_, searchContent, replaceContent] = match; - + // Detect line ending from original content const lineEnding = originalContent.includes('\r\n') ? '\r\n' : '\n'; // Strip line numbers from search and replace content if every line starts with a line number const hasLineNumbers = (content: string) => { const lines = content.split(/\r?\n/); - return lines.length > 0 && lines.every(line => /^\d+\s+\|(?!\|)/.test(line)); + return lines.length > 0 && lines.every(line => /^\s*\d+\s+\|(?!\|)/.test(line)); }; if (hasLineNumbers(searchContent) && hasLineNumbers(replaceContent)) { const stripLineNumbers = (content: string) => { - return content.replace(/^\d+\s+\|(?!\|)/gm, '') + return content.replace(/^\s*\d+\s+\|(?!\|)/gm, ''); }; searchContent = stripLineNumbers(searchContent); @@ -153,8 +206,8 @@ Your search/replace content here } // Split content into lines, handling both \n and \r\n - const searchLines = searchContent.split(/\r?\n/); - const replaceLines = replaceContent.split(/\r?\n/); + const searchLines = searchContent === '' ? [] : searchContent.split(/\r?\n/); + const replaceLines = replaceContent === '' ? [] : replaceContent.split(/\r?\n/); const originalLines = originalContent.split(/\r?\n/); // First try exact line range if provided @@ -167,9 +220,15 @@ Your search/replace content here const exactStartIndex = startLine - 1; const exactEndIndex = endLine - 1; - if (exactStartIndex < 0 || exactEndIndex >= originalLines.length || exactStartIndex > exactEndIndex) { + if (exactStartIndex < 0 || exactEndIndex > originalLines.length || exactStartIndex > exactEndIndex) { const debugInfo = this.debugEnabled ? `\n\nDebug Info:\n- Requested Range: lines ${startLine}-${endLine}\n- File Bounds: lines 1-${originalLines.length}` : ''; + // Log detailed debug information + console.log('Invalid Line Range Debug:', { + requestedRange: { start: startLine, end: endLine }, + fileBounds: { start: 1, end: originalLines.length } + }); + return { success: false, error: `Line range ${startLine}-${endLine} is invalid (file has ${originalLines.length} lines)${debugInfo}`, @@ -263,7 +322,7 @@ Your search/replace content here // Apply the replacement while preserving exact indentation const indentedReplaceLines = replaceLines.map((line, i) => { // Get the matched line's exact indentation - const matchedIndent = originalIndents[0]; + const matchedIndent = originalIndents[0] || ''; // Get the current line's indentation relative to the search content const currentIndentMatch = line.match(/^[\t ]*/); diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 3f17b06..861046e 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -22,7 +22,7 @@ import { Cline } from "../Cline" import { openMention } from "../mentions" import { getNonce } from "./getNonce" import { getUri } from "./getUri" -import { playSound, setSoundEnabled } from "../../utils/sound" +import { playSound, setSoundEnabled, setSoundVolume } from "../../utils/sound" /* https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts @@ -66,6 +66,7 @@ type GlobalStateKey = | "openRouterUseMiddleOutTransform" | "allowedCommands" | "soundEnabled" + | "soundVolume" | "diffEnabled" | "debugDiffEnabled" | "alwaysAllowMcp" @@ -137,6 +138,11 @@ export class ClineProvider implements vscode.WebviewViewProvider { this.outputChannel.appendLine("Resolving webview view") this.view = webviewView + // Initialize sound enabled state + this.getState().then(({ soundEnabled }) => { + setSoundEnabled(soundEnabled ?? false) + }) + webviewView.webview.options = { // Allow scripts in the webview enableScripts: true, @@ -597,6 +603,12 @@ export class ClineProvider implements vscode.WebviewViewProvider { setSoundEnabled(soundEnabled) // Add this line to update the sound utility await this.postStateToWebview() break + case "soundVolume": + const soundVolume = message.value ?? 0.5 + await this.updateGlobalState("soundVolume", soundVolume) + setSoundVolume(soundVolume) + await this.postStateToWebview() + break case "diffEnabled": const diffEnabled = message.bool ?? true await this.updateGlobalState("diffEnabled", diffEnabled) @@ -935,6 +947,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { diffEnabled, debugDiffEnabled, taskHistory, + soundVolume, } = await this.getState() const allowedCommands = vscode.workspace @@ -960,6 +973,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { debugDiffEnabled: debugDiffEnabled ?? false, shouldShowAnnouncement: lastShownAnnouncementId !== this.latestAnnouncementId, allowedCommands, + soundVolume: soundVolume ?? 0.5, } } @@ -1053,6 +1067,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { soundEnabled, diffEnabled, debugDiffEnabled, + soundVolume, ] = await Promise.all([ this.getGlobalState("apiProvider") as Promise, this.getGlobalState("apiModelId") as Promise, @@ -1091,6 +1106,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { this.getGlobalState("soundEnabled") as Promise, this.getGlobalState("diffEnabled") as Promise, this.getGlobalState("debugDiffEnabled") as Promise, + this.getGlobalState("soundVolume") as Promise, ]) let apiProvider: ApiProvider @@ -1147,6 +1163,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { soundEnabled: soundEnabled ?? false, diffEnabled: diffEnabled ?? false, debugDiffEnabled: debugDiffEnabled ?? false, + soundVolume, } } diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index b9ba21f..e95bb80 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -51,6 +51,7 @@ export interface ExtensionState { uriScheme?: string allowedCommands?: string[] soundEnabled?: boolean + soundVolume?: number diffEnabled?: boolean debugDiffEnabled?: boolean } diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index d4377ca..aca93e1 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -32,6 +32,7 @@ export interface WebviewMessage { | "alwaysAllowMcp" | "playSound" | "soundEnabled" + | "soundVolume" | "diffEnabled" | "debugDiffEnabled" | "openMcpSettings" @@ -44,6 +45,7 @@ export interface WebviewMessage { apiConfiguration?: ApiConfiguration images?: string[] bool?: boolean + value?: number commands?: string[] audioType?: AudioType // For toggleToolAutoApprove diff --git a/src/utils/sound.ts b/src/utils/sound.ts index 9255db4..a7f0d73 100644 --- a/src/utils/sound.ts +++ b/src/utils/sound.ts @@ -21,6 +21,7 @@ export const isWAV = (filepath: string): boolean => { } let isSoundEnabled = false +let volume = .5 /** * Set sound configuration @@ -30,6 +31,14 @@ export const setSoundEnabled = (enabled: boolean): void => { isSoundEnabled = enabled } +/** + * Set sound volume + * @param volume number + */ +export const setSoundVolume = (newVolume: number): void => { + volume = newVolume +} + /** * Play a sound file * @param filepath string @@ -54,11 +63,9 @@ export const playSound = (filepath: string): void => { return // Skip playback within minimum interval to prevent continuous playback } - const player = require("play-sound")() - player.play(filepath, function (err: any) { - if (err) { - throw new Error("Failed to play sound effect") - } + const sound = require("sound-play") + sound.play(filepath, volume).catch(() => { + throw new Error("Failed to play sound effect") }) lastPlayedTime = currentTime diff --git a/webview-ui/package-lock.json b/webview-ui/package-lock.json index 189ee43..ade601f 100644 --- a/webview-ui/package-lock.json +++ b/webview-ui/package-lock.json @@ -34,7 +34,8 @@ }, "devDependencies": { "@babel/plugin-proposal-private-property-in-object": "^7.21.11", - "@types/vscode-webview": "^1.57.5" + "@types/vscode-webview": "^1.57.5", + "eslint": "^8.57.0" } }, "node_modules/@adobe/css-tools": { diff --git a/webview-ui/package.json b/webview-ui/package.json index 3d12cb9..2ede80f 100644 --- a/webview-ui/package.json +++ b/webview-ui/package.json @@ -31,7 +31,8 @@ "start": "react-scripts start", "build": "node ./scripts/build-react-no-split.js", "test": "react-scripts test --watchAll=false", - "eject": "react-scripts eject" + "eject": "react-scripts eject", + "lint": "eslint src --ext ts,tsx" }, "eslintConfig": { "extends": [ @@ -53,7 +54,8 @@ }, "devDependencies": { "@babel/plugin-proposal-private-property-in-object": "^7.21.11", - "@types/vscode-webview": "^1.57.5" + "@types/vscode-webview": "^1.57.5", + "eslint": "^8.57.0" }, "jest": { "transformIgnorePatterns": [ diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index e4e0880..ff765e2 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -64,7 +64,6 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie const [isAtBottom, setIsAtBottom] = useState(false) const [wasStreaming, setWasStreaming] = useState(false) - const [hasStarted, setHasStarted] = useState(false) // UI layout depends on the last 2 messages // (since it relies on the content of these messages, we are deep comparing. i.e. the button state after hitting button sets enableButtons to false, and this effect otherwise would have to true again even if messages didn't change @@ -75,12 +74,6 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie vscode.postMessage({ type: "playSound", audioType }) } - function playSoundOnMessage(audioType: AudioType) { - if (hasStarted && !isStreaming) { - playSound(audioType) - } - } - useDeepCompareEffect(() => { // if last message is an ask, show user ask UI // if user finished a task, then start a new task with a new conversation history since in this moment that the extension is waiting for user response, the user could close the extension and the conversation history would be lost. @@ -91,7 +84,7 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie const isPartial = lastMessage.partial === true switch (lastMessage.ask) { case "api_req_failed": - playSoundOnMessage("progress_loop") + playSound("progress_loop") setTextAreaDisabled(true) setClineAsk("api_req_failed") setEnableButtons(true) @@ -99,7 +92,7 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie setSecondaryButtonText("Start New Task") break case "mistake_limit_reached": - playSoundOnMessage("progress_loop") + playSound("progress_loop") setTextAreaDisabled(false) setClineAsk("mistake_limit_reached") setEnableButtons(true) @@ -107,7 +100,6 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie setSecondaryButtonText("Start New Task") break case "followup": - playSoundOnMessage("notification") setTextAreaDisabled(isPartial) setClineAsk("followup") setEnableButtons(isPartial) @@ -115,7 +107,9 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie // setSecondaryButtonText(undefined) break case "tool": - playSoundOnMessage("notification") + if (!isAutoApproved(lastMessage)) { + playSound("notification") + } setTextAreaDisabled(isPartial) setClineAsk("tool") setEnableButtons(!isPartial) @@ -134,7 +128,9 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie } break case "browser_action_launch": - playSoundOnMessage("notification") + if (!isAutoApproved(lastMessage)) { + playSound("notification") + } setTextAreaDisabled(isPartial) setClineAsk("browser_action_launch") setEnableButtons(!isPartial) @@ -142,7 +138,9 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie setSecondaryButtonText("Reject") break case "command": - playSoundOnMessage("notification") + if (!isAutoApproved(lastMessage)) { + playSound("notification") + } setTextAreaDisabled(isPartial) setClineAsk("command") setEnableButtons(!isPartial) @@ -150,7 +148,6 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie setSecondaryButtonText("Reject") break case "command_output": - playSoundOnMessage("notification") setTextAreaDisabled(false) setClineAsk("command_output") setEnableButtons(true) @@ -166,7 +163,7 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie break case "completion_result": // extension waiting for feedback. but we can just present a new task button - playSoundOnMessage("celebration") + playSound("celebration") setTextAreaDisabled(isPartial) setClineAsk("completion_result") setEnableButtons(!isPartial) @@ -174,7 +171,6 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie setSecondaryButtonText(undefined) break case "resume_task": - playSoundOnMessage("notification") setTextAreaDisabled(false) setClineAsk("resume_task") setEnableButtons(true) @@ -183,7 +179,7 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie setDidClickCancel(false) // special case where we reset the cancel button state break case "resume_completed_task": - playSoundOnMessage("celebration") + playSound("celebration") setTextAreaDisabled(false) setClineAsk("resume_completed_task") setEnableButtons(true) @@ -482,36 +478,122 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie return true }) }, [modifiedMessages]) - useEffect(() => { - if (isStreaming) { - // Set to true once any request has started - setHasStarted(true) + + const isReadOnlyToolAction = useCallback((message: ClineMessage | undefined) => { + if (message?.type === "ask") { + if (!message.text) { + return true + } + const tool = JSON.parse(message.text) + return ["readFile", "listFiles", "listFilesTopLevel", "listFilesRecursive", "listCodeDefinitionNames", "searchFiles"].includes(tool.tool) } + return false + }, []) + + const isWriteToolAction = useCallback((message: ClineMessage | undefined) => { + if (message?.type === "ask") { + if (!message.text) { + return true + } + const tool = JSON.parse(message.text) + return ["editedExistingFile", "appliedDiff", "newFileCreated"].includes(tool.tool) + } + return false + }, []) + + const isMcpToolAlwaysAllowed = useCallback((message: ClineMessage | undefined) => { + if (message?.type === "ask" && message.ask === "use_mcp_server") { + if (!message.text) { + return true + } + const mcpServerUse = JSON.parse(message.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 + }, [mcpServers]) + + const isAllowedCommand = useCallback((message: ClineMessage | undefined) => { + if (message?.type === "ask") { + const command = message.text + if (!command) { + return true + } + + // Split command by chaining operators + const commands = command.split(/&&|\|\||;|\||\$\(|`/).map(cmd => cmd.trim()) + + // Check if all individual commands are allowed + return commands.every((cmd) => { + const trimmedCommand = cmd.toLowerCase() + return allowedCommands?.some((prefix) => trimmedCommand.startsWith(prefix.toLowerCase())) + }) + } + return false + }, [allowedCommands]) + + const isAutoApproved = useCallback( + (message: ClineMessage | undefined) => { + if (!message || message.type !== "ask") return false + + return ( + (alwaysAllowBrowser && message.ask === "browser_action_launch") || + (alwaysAllowReadOnly && message.ask === "tool" && isReadOnlyToolAction(message)) || + (alwaysAllowWrite && message.ask === "tool" && isWriteToolAction(message)) || + (alwaysAllowExecute && message.ask === "command" && isAllowedCommand(message)) || + (alwaysAllowMcp && message.ask === "use_mcp_server" && isMcpToolAlwaysAllowed(message)) + ) + }, + [ + alwaysAllowBrowser, + alwaysAllowReadOnly, + alwaysAllowWrite, + alwaysAllowExecute, + alwaysAllowMcp, + isReadOnlyToolAction, + isWriteToolAction, + isAllowedCommand, + isMcpToolAlwaysAllowed + ] + ) + + useEffect(() => { // Only execute when isStreaming changes from true to false if (wasStreaming && !isStreaming && lastMessage) { // Play appropriate sound based on lastMessage content if (lastMessage.type === "ask") { - switch (lastMessage.ask) { - case "api_req_failed": - case "mistake_limit_reached": - playSound("progress_loop") - break - case "tool": - case "followup": - case "browser_action_launch": - case "resume_task": - playSound("notification") - break - case "completion_result": - case "resume_completed_task": - playSound("celebration") - break + // Don't play sounds for auto-approved actions + if (!isAutoApproved(lastMessage)) { + switch (lastMessage.ask) { + case "api_req_failed": + case "mistake_limit_reached": + playSound("progress_loop") + break + case "followup": + if (!lastMessage.partial) { + playSound("notification") + } + break + case "tool": + case "browser_action_launch": + case "resume_task": + case "use_mcp_server": + playSound("notification") + break + case "completion_result": + case "resume_completed_task": + playSound("celebration") + break + } } } } // Update previous value setWasStreaming(isStreaming) - }, [isStreaming, lastMessage, wasStreaming]) + }, [isStreaming, lastMessage, wasStreaming, isAutoApproved]) const isBrowserSessionMessage = (message: ClineMessage): boolean => { // which of visible messages are browser session messages, see above @@ -750,64 +832,10 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie // Only proceed if we have an ask and buttons are enabled if (!clineAsk || !enableButtons) return - const isReadOnlyToolAction = () => { - const lastMessage = messages.at(-1) - if (lastMessage?.type === "ask" && lastMessage.text) { - const tool = JSON.parse(lastMessage.text) - return ["readFile", "listFiles", "listFilesTopLevel", "listFilesRecursive", "listCodeDefinitionNames", "searchFiles"].includes(tool.tool) - } - return false - } - - const isWriteToolAction = () => { - const lastMessage = messages.at(-1) - if (lastMessage?.type === "ask" && lastMessage.text) { - const tool = JSON.parse(lastMessage.text) - return ["editedExistingFile", "appliedDiff", "newFileCreated"].includes(tool.tool) - } - 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) { - const command = lastMessage.text - - // Split command by chaining operators - const commands = command.split(/&&|\|\||;|\||\$\(|`/).map(cmd => cmd.trim()) - - // Check if all individual commands are allowed - return commands.every((cmd) => { - const trimmedCommand = cmd.toLowerCase() - return allowedCommands?.some((prefix) => trimmedCommand.startsWith(prefix.toLowerCase())) - }) - } - return false - } - - if ( - (alwaysAllowBrowser && clineAsk === "browser_action_launch") || - (alwaysAllowReadOnly && clineAsk === "tool" && isReadOnlyToolAction()) || - (alwaysAllowWrite && clineAsk === "tool" && isWriteToolAction()) || - (alwaysAllowExecute && clineAsk === "command" && isAllowedCommand()) || - (alwaysAllowMcp && clineAsk === "use_mcp_server" && isMcpToolAlwaysAllowed()) - ) { + if (isAutoApproved(lastMessage)) { handlePrimaryButtonClick() } - }, [clineAsk, enableButtons, handlePrimaryButtonClick, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, alwaysAllowMcp, messages, allowedCommands, mcpServers]) + }, [clineAsk, enableButtons, handlePrimaryButtonClick, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, alwaysAllowMcp, messages, allowedCommands, mcpServers, isAutoApproved, lastMessage]) return (
{ }) }) }) + +describe('ChatView - Sound Playing Tests', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it('does not play sound for auto-approved browser actions', async () => { + render( + + {}} + showHistoryView={() => {}} + /> + + ) + + // First hydrate state with initial task and streaming + mockPostMessage({ + alwaysAllowBrowser: true, + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + }, + { + type: 'say', + say: 'api_req_started', + ts: Date.now() - 1000, + text: JSON.stringify({}), + partial: true + } + ] + }) + + // Then send the browser action ask message (streaming finished) + mockPostMessage({ + alwaysAllowBrowser: true, + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + }, + { + type: 'ask', + ask: 'browser_action_launch', + ts: Date.now(), + text: JSON.stringify({ action: 'launch', url: 'http://example.com' }), + partial: false + } + ] + }) + + // Verify no sound was played + expect(vscode.postMessage).not.toHaveBeenCalledWith({ + type: 'playSound', + audioType: expect.any(String) + }) + }) + + it('plays notification sound for non-auto-approved browser actions', async () => { + render( + + {}} + showHistoryView={() => {}} + /> + + ) + + // First hydrate state with initial task and streaming + mockPostMessage({ + alwaysAllowBrowser: false, + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + }, + { + type: 'say', + say: 'api_req_started', + ts: Date.now() - 1000, + text: JSON.stringify({}), + partial: true + } + ] + }) + + // Then send the browser action ask message (streaming finished) + mockPostMessage({ + alwaysAllowBrowser: false, + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + }, + { + type: 'ask', + ask: 'browser_action_launch', + ts: Date.now(), + text: JSON.stringify({ action: 'launch', url: 'http://example.com' }), + partial: false + } + ] + }) + + // Verify notification sound was played + await waitFor(() => { + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: 'playSound', + audioType: 'notification' + }) + }) + }) + + it('plays celebration sound for completion results', async () => { + render( + + {}} + showHistoryView={() => {}} + /> + + ) + + // First hydrate state with initial task and streaming + mockPostMessage({ + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + }, + { + type: 'say', + say: 'api_req_started', + ts: Date.now() - 1000, + text: JSON.stringify({}), + partial: true + } + ] + }) + + // Then send the completion result message (streaming finished) + mockPostMessage({ + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + }, + { + type: 'ask', + ask: 'completion_result', + ts: Date.now(), + text: 'Task completed successfully', + partial: false + } + ] + }) + + // Verify celebration sound was played + await waitFor(() => { + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: 'playSound', + audioType: 'celebration' + }) + }) + }) + + it('plays progress_loop sound for api failures', async () => { + render( + + {}} + showHistoryView={() => {}} + /> + + ) + + // First hydrate state with initial task and streaming + mockPostMessage({ + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + }, + { + type: 'say', + say: 'api_req_started', + ts: Date.now() - 1000, + text: JSON.stringify({}), + partial: true + } + ] + }) + + // Then send the api failure message (streaming finished) + mockPostMessage({ + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + }, + { + type: 'ask', + ask: 'api_req_failed', + ts: Date.now(), + text: 'API request failed', + partial: false + } + ] + }) + + // Verify progress_loop sound was played + await waitFor(() => { + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: 'playSound', + audioType: 'progress_loop' + }) + }) + }) +}) diff --git a/webview-ui/src/components/mcp/__tests__/McpToolRow.test.tsx b/webview-ui/src/components/mcp/__tests__/McpToolRow.test.tsx index 9f3cd96..2f4d286 100644 --- a/webview-ui/src/components/mcp/__tests__/McpToolRow.test.tsx +++ b/webview-ui/src/components/mcp/__tests__/McpToolRow.test.tsx @@ -23,7 +23,6 @@ jest.mock('@vscode/webview-ui-toolkit/react', () => ({