Simplify auto-approving code and make it work better with browser actions (#21)

src/core/Cline.ts

@@ -56,16 +56,6 @@ type UserContent = Array<
 	Anthropic.TextBlockParam | Anthropic.ImageBlockParam | Anthropic.ToolUseBlockParam | Anthropic.ToolResultBlockParam
 >
 
-const ALLOWED_AUTO_EXECUTE_COMMANDS = [
-	'npm',
-	'npx',
-	'tsc',
-	'git log',
-	'git diff',
-	'git show',
-	'list'
-] as const
-
 export class Cline {
 	readonly taskId: string
 	api: ApiHandler
@@ -74,10 +64,6 @@ export class Cline {
 	private browserSession: BrowserSession
 	private didEditFile: boolean = false
 	customInstructions?: string
-	alwaysAllowReadOnly: boolean
-	alwaysAllowWrite: boolean
-	alwaysAllowExecute: boolean
-	alwaysAllowBrowser: boolean
 
 	apiConversationHistory: Anthropic.MessageParam[] = []
 	clineMessages: ClineMessage[] = []
@@ -107,10 +93,6 @@ export class Cline {
 		provider: ClineProvider,
 		apiConfiguration: ApiConfiguration,
 		customInstructions?: string,
-		alwaysAllowReadOnly?: boolean,
-		alwaysAllowWrite?: boolean,
-		alwaysAllowExecute?: boolean,
-		alwaysAllowBrowser?: boolean,
 		task?: string,
 		images?: string[],
 		historyItem?: HistoryItem,
@@ -122,10 +104,6 @@ export class Cline {
 		this.browserSession = new BrowserSession(provider.context)
 		this.diffViewProvider = new DiffViewProvider(cwd)
 		this.customInstructions = customInstructions
-		this.alwaysAllowReadOnly = alwaysAllowReadOnly ?? false
-		this.alwaysAllowWrite = alwaysAllowWrite ?? false
-		this.alwaysAllowExecute = alwaysAllowExecute ?? false		
-		this.alwaysAllowBrowser = alwaysAllowBrowser ?? false
 
 		if (historyItem) {
 			this.taskId = historyItem.id
@@ -138,25 +116,6 @@ export class Cline {
 		}
 	}
 
-	private isAllowedCommand(command?: string): boolean {
-		if (!command) {
-			return false;
-		}
-		// Check for command chaining characters
-		if (command.includes('&&') ||
-			command.includes(';') ||
-			command.includes('||') ||
-			command.includes('|') ||
-			command.includes('$(') ||
-			command.includes('`')) {
-			return false;
-		}
-		const trimmedCommand = command.trim().toLowerCase();
-		return ALLOWED_AUTO_EXECUTE_COMMANDS.some(prefix => 
-			trimmedCommand.startsWith(prefix.toLowerCase())
-		);
-	}
-
 	// Storing task to disk for history
 
 	private async ensureTaskDirectoryExists(): Promise<string> {
@@ -1101,11 +1060,7 @@ export class Cline {
 							if (block.partial) {
 								// update gui message
 								const partialMessage = JSON.stringify(sharedMessageProps)
-								if (this.alwaysAllowWrite) {
-									await this.say("tool", partialMessage, undefined, block.partial)
-								} else {
-									await this.ask("tool", partialMessage, block.partial).catch(() => {})
-								}
+								await this.ask("tool", partialMessage, block.partial).catch(() => {})
 								// update editor
 								if (!this.diffViewProvider.isEditing) {
 									// open the editor and prepare to stream content in
@@ -1135,11 +1090,7 @@ export class Cline {
 								if (!this.diffViewProvider.isEditing) {
 									// show gui message before showing edit animation
 									const partialMessage = JSON.stringify(sharedMessageProps)
-									if (this.alwaysAllowWrite) {
-										await this.say("tool", partialMessage, undefined, true)
-									} else {
-										await this.ask("tool", partialMessage, true).catch(() => {}) // sending true for partial even though it's not a partial, this shows the edit row before the content is streamed into the editor
-									}
+									await this.ask("tool", partialMessage, true).catch(() => {}) // sending true for partial even though it's not a partial, this shows the edit row before the content is streamed into the editor
 									await this.diffViewProvider.open(relPath)
 								}
 								await this.diffViewProvider.update(newContent, true)
@@ -1158,7 +1109,7 @@ export class Cline {
 											)
 										: undefined,
 								} satisfies ClineSayTool)
-								const didApprove = this.alwaysAllowWrite || (await askApproval("tool", completeMessage))
+								const didApprove = await askApproval("tool", completeMessage)
 								if (!didApprove) {
 									await this.diffViewProvider.revertChanges()
 									break
@@ -1211,11 +1162,7 @@ export class Cline {
 									...sharedMessageProps,
 									content: undefined,
 								} satisfies ClineSayTool)
-								if (this.alwaysAllowReadOnly) {
-									await this.say("tool", partialMessage, undefined, block.partial)
-								} else {
-									await this.ask("tool", partialMessage, block.partial).catch(() => {})
-								}
+								await this.ask("tool", partialMessage, block.partial).catch(() => {})
 								break
 							} else {
 								if (!relPath) {
@@ -1229,13 +1176,9 @@ export class Cline {
 									...sharedMessageProps,
 									content: absolutePath,
 								} satisfies ClineSayTool)
-								if (this.alwaysAllowReadOnly) {
-									await this.say("tool", completeMessage, undefined, false) // need to be sending partialValue bool, since undefined has its own purpose in that the message is treated neither as a partial or completion of a partial, but as a single complete message
-								} else {
-									const didApprove = await askApproval("tool", completeMessage)
-									if (!didApprove) {
-										break
-									}
+								const didApprove = await askApproval("tool", completeMessage)
+								if (!didApprove) {
+									break
 								}
 								// now execute the tool like normal
 								const content = await extractTextFromFile(absolutePath)
@@ -1261,11 +1204,7 @@ export class Cline {
 									...sharedMessageProps,
 									content: "",
 								} satisfies ClineSayTool)
-								if (this.alwaysAllowReadOnly) {
-									await this.say("tool", partialMessage, undefined, block.partial)
-								} else {
-									await this.ask("tool", partialMessage, block.partial).catch(() => {})
-								}
+								await this.ask("tool", partialMessage, block.partial).catch(() => {})
 								break
 							} else {
 								if (!relDirPath) {
@@ -1281,13 +1220,9 @@ export class Cline {
 									...sharedMessageProps,
 									content: result,
 								} satisfies ClineSayTool)
-								if (this.alwaysAllowReadOnly) {
-									await this.say("tool", completeMessage, undefined, false)
-								} else {
-									const didApprove = await askApproval("tool", completeMessage)
-									if (!didApprove) {
-										break
-									}
+								const didApprove = await askApproval("tool", completeMessage)
+								if (!didApprove) {
+									break
 								}
 								pushToolResult(result)
 								break
@@ -1309,11 +1244,7 @@ export class Cline {
 									...sharedMessageProps,
 									content: "",
 								} satisfies ClineSayTool)
-								if (this.alwaysAllowReadOnly) {
-									await this.say("tool", partialMessage, undefined, block.partial)
-								} else {
-									await this.ask("tool", partialMessage, block.partial).catch(() => {})
-								}
+								await this.ask("tool", partialMessage, block.partial).catch(() => {})
 								break
 							} else {
 								if (!relDirPath) {
@@ -1330,13 +1261,9 @@ export class Cline {
 									...sharedMessageProps,
 									content: result,
 								} satisfies ClineSayTool)
-								if (this.alwaysAllowReadOnly) {
-									await this.say("tool", completeMessage, undefined, false)
-								} else {
-									const didApprove = await askApproval("tool", completeMessage)
-									if (!didApprove) {
-										break
-									}
+								const didApprove = await askApproval("tool", completeMessage)
+								if (!didApprove) {
+									break
 								}
 								pushToolResult(result)
 								break
@@ -1362,11 +1289,7 @@ export class Cline {
 									...sharedMessageProps,
 									content: "",
 								} satisfies ClineSayTool)
-								if (this.alwaysAllowReadOnly) {
-									await this.say("tool", partialMessage, undefined, block.partial)
-								} else {
-									await this.ask("tool", partialMessage, block.partial).catch(() => {})
-								}
+								await this.ask("tool", partialMessage, block.partial).catch(() => {})
 								break
 							} else {
 								if (!relDirPath) {
@@ -1386,13 +1309,9 @@ export class Cline {
 									...sharedMessageProps,
 									content: results,
 								} satisfies ClineSayTool)
-								if (this.alwaysAllowReadOnly) {
-									await this.say("tool", completeMessage, undefined, false)
-								} else {
-									const didApprove = await askApproval("tool", completeMessage)
-									if (!didApprove) {
-										break
-									}
+								const didApprove = await askApproval("tool", completeMessage)
+								if (!didApprove) {
+									break
 								}
 								pushToolResult(results)
 								break
@@ -1421,24 +1340,11 @@ export class Cline {
 						try {
 							if (block.partial) {
 								if (action === "launch") {
-									if (this.alwaysAllowBrowser) {
-										await this.say(
-											"browser_action",
-											JSON.stringify({
-												action: action as BrowserAction,
-												coordinate: undefined,
-												text: undefined
-											} satisfies ClineSayBrowserAction),
-											undefined,
-											block.partial
-										)
-									} else {
-										await this.ask(
-											"browser_action_launch",
-											removeClosingTag("url", url),
-											block.partial
-										).catch(() => {})
-									}
+									await this.ask(
+										"browser_action_launch",
+										removeClosingTag("url", url),
+										block.partial
+									).catch(() => {})
 								} else {
 									await this.say(
 										"browser_action",
@@ -1464,7 +1370,7 @@ export class Cline {
 										break
 									}
 									this.consecutiveMistakeCount = 0
-									const didApprove = this.alwaysAllowBrowser || await askApproval("browser_action_launch", url)
+									const didApprove = await askApproval("browser_action_launch", url)
 									if (!didApprove) {
 										break
 									}
@@ -1565,13 +1471,9 @@ export class Cline {
 						const command: string | undefined = block.params.command
 						try {
 							if (block.partial) {
-								if (this.alwaysAllowExecute && this.isAllowedCommand(command)) {
-									await this.say("command", command, undefined, block.partial)
-								} else {
-									await this.ask("command", removeClosingTag("command", command), block.partial).catch(
-										() => {}
-									)
-								}								
+								await this.ask("command", removeClosingTag("command", command), block.partial).catch(
+									() => {}
+								)
 								break
 							} else {
 								if (!command) {
@@ -1583,8 +1485,7 @@ export class Cline {
 								}
 								this.consecutiveMistakeCount = 0
 
-								const didApprove = (this.alwaysAllowExecute && this.isAllowedCommand(command)) || 
-									(await askApproval("command", command))
+								const didApprove = await askApproval("command", command)
 								if (!didApprove) {
 									break
 								}

src/core/__tests__/Cline.test.ts

@@ -231,40 +231,14 @@ describe('Cline', () => {
     });
 
     describe('constructor', () => {
-        it('should initialize with default settings', () => {
-            const cline = new Cline(
-                mockProvider,
-                mockApiConfig,
-                undefined, // customInstructions
-                undefined, // alwaysAllowReadOnly
-                undefined, // alwaysAllowWrite
-                undefined, // alwaysAllowExecute
-                undefined, // alwaysAllowBrowser
-                'test task'
-            );
-
-            expect(cline.alwaysAllowReadOnly).toBe(false);
-            expect(cline.alwaysAllowWrite).toBe(false);
-            expect(cline.alwaysAllowExecute).toBe(false);
-            expect(cline.alwaysAllowBrowser).toBe(false);
-        });
-
         it('should respect provided settings', () => {
             const cline = new Cline(
                 mockProvider,
                 mockApiConfig,
                 'custom instructions',
-                true,  // alwaysAllowReadOnly
-                true,  // alwaysAllowWrite
-                true,  // alwaysAllowExecute
-                true,  // alwaysAllowBrowser
                 'test task'
             );
 
-            expect(cline.alwaysAllowReadOnly).toBe(true);
-            expect(cline.alwaysAllowWrite).toBe(true);
-            expect(cline.alwaysAllowExecute).toBe(true);
-            expect(cline.alwaysAllowBrowser).toBe(true);
             expect(cline.customInstructions).toBe('custom instructions');
         });
 
@@ -277,142 +251,4 @@ describe('Cline', () => {
             }).toThrow('Either historyItem or task/images must be provided');
         });
     });
-
-    describe('file operations', () => {
-        let cline: Cline;
-
-        beforeEach(() => {
-            cline = new Cline(
-                mockProvider,
-                mockApiConfig,
-                undefined,
-                false,
-                false,
-                false,
-                false,
-                'test task'
-            );
-        });
-
-        it('should bypass approval when alwaysAllowWrite is true', async () => {
-            const writeEnabledCline = new Cline(
-                mockProvider,
-                mockApiConfig,
-                undefined,
-                false,
-                true,  // alwaysAllowWrite
-                false,
-                false,
-                'test task'
-            );
-
-            expect(writeEnabledCline.alwaysAllowWrite).toBe(true);
-            // The write operation would bypass approval in actual implementation
-        });
-
-        it('should require approval when alwaysAllowWrite is false', async () => {
-            const writeDisabledCline = new Cline(
-                mockProvider,
-                mockApiConfig,
-                undefined,
-                false,
-                false,  // alwaysAllowWrite
-                false,
-                false,
-                'test task'
-            );
-
-            expect(writeDisabledCline.alwaysAllowWrite).toBe(false);
-            // The write operation would require approval in actual implementation
-        });
-    });
-
-    describe('isAllowedCommand', () => {
-        let cline: any
-
-        beforeEach(() => {
-            // Create a more complete mock provider
-            const mockProvider = {
-                context: {
-                    globalStorageUri: { fsPath: '/mock/path' }
-                },
-                postStateToWebview: jest.fn(),
-                postMessageToWebview: jest.fn(),
-                updateTaskHistory: jest.fn()
-            }
-
-            // Mock the required dependencies
-            const mockApiConfig = {
-                getModel: () => ({
-                    id: 'claude-3-sonnet',
-                    info: { supportsComputerUse: true }
-                })
-            }
-
-            // Create test instance with mocked constructor params
-            cline = new Cline(
-                mockProvider as any,
-                mockApiConfig as any,
-                undefined,  // customInstructions
-                false,      // alwaysAllowReadOnly
-                false,      // alwaysAllowWrite
-                false,      // alwaysAllowExecute
-                false,      // alwaysAllowBrowser
-                'test task' // task
-            )
-
-            // Mock internal methods that are called during initialization
-            cline.initiateTaskLoop = jest.fn()
-            cline.say = jest.fn()
-            cline.addToClineMessages = jest.fn()
-            cline.overwriteClineMessages = jest.fn()
-            cline.addToApiConversationHistory = jest.fn()
-            cline.overwriteApiConversationHistory = jest.fn()
-        })
-
-        test('returns true for allowed commands', () => {
-            expect(cline.isAllowedCommand('npm install')).toBe(true)
-            expect(cline.isAllowedCommand('npx create-react-app')).toBe(true)
-            expect(cline.isAllowedCommand('tsc --watch')).toBe(true)
-            expect(cline.isAllowedCommand('git log --oneline')).toBe(true)
-            expect(cline.isAllowedCommand('git diff main')).toBe(true)
-        })
-
-        test('returns true regardless of case or whitespace', () => {
-            expect(cline.isAllowedCommand('NPM install')).toBe(true)
-            expect(cline.isAllowedCommand('  npm install')).toBe(true)
-            expect(cline.isAllowedCommand('GIT DIFF')).toBe(true)
-        })
-
-        test('returns false for non-allowed commands', () => {
-            expect(cline.isAllowedCommand('rm -rf /')).toBe(false)
-            expect(cline.isAllowedCommand('git push')).toBe(false)
-            expect(cline.isAllowedCommand('git commit')).toBe(false)
-            expect(cline.isAllowedCommand('curl http://example.com')).toBe(false)
-        })
-
-        test('returns false for undefined or empty commands', () => {
-            expect(cline.isAllowedCommand()).toBe(false)
-            expect(cline.isAllowedCommand('')).toBe(false)
-            expect(cline.isAllowedCommand('  ')).toBe(false)
-        })
-
-        test('returns false for commands with chaining operators', () => {
-            const maliciousCommands = [
-                'npm install && rm -rf /',
-                'git status; dangerous-command',
-                'git log || evil-script',
-                'git status | malicious-pipe',
-                'git log $(evil-command)',
-                'git status `rm -rf /`',
-                'npm install && echo "malicious"',
-                'git status; curl http://evil.com',
-                'tsc --watch || wget malware',
-            ];
-
-            maliciousCommands.forEach(cmd => {
-                expect(cline.isAllowedCommand(cmd)).toBe(false);
-            });
-        });
-    })
 });

src/core/webview/ClineProvider.ts

@@ -197,20 +197,12 @@ export class ClineProvider implements vscode.WebviewViewProvider {
 		const { 
 			apiConfiguration, 
 			customInstructions, 
-			alwaysAllowReadOnly, 
-			alwaysAllowWrite, 
-			alwaysAllowExecute,
-			alwaysAllowBrowser 
 		} = await this.getState()
 		
 		this.cline = new Cline(
 			this, 
 			apiConfiguration, 
 			customInstructions, 
-			alwaysAllowReadOnly, 
-			alwaysAllowWrite, 
-			alwaysAllowExecute,
-			alwaysAllowBrowser,
 			task, 
 			images
 		)
@@ -221,20 +213,12 @@ export class ClineProvider implements vscode.WebviewViewProvider {
 		const { 
 			apiConfiguration, 
 			customInstructions, 
-			alwaysAllowReadOnly, 
-			alwaysAllowWrite, 
-			alwaysAllowExecute,
-			alwaysAllowBrowser 
 		} = await this.getState()
 		
 		this.cline = new Cline(
 			this,
 			apiConfiguration,
 			customInstructions,
-			alwaysAllowReadOnly,
-			alwaysAllowWrite,
-			alwaysAllowExecute,
-			alwaysAllowBrowser,
 			undefined,
 			undefined,
 			historyItem,
@@ -440,23 +424,18 @@ export class ClineProvider implements vscode.WebviewViewProvider {
 						break
 					case "alwaysAllowReadOnly":
 						await this.updateGlobalState("alwaysAllowReadOnly", message.bool ?? undefined)
-						if (this.cline) {
-							this.cline.alwaysAllowReadOnly = message.bool ?? false
-						}
 						await this.postStateToWebview()
 						break
 					case "alwaysAllowWrite":
 						await this.updateGlobalState("alwaysAllowWrite", message.bool ?? undefined)
-						if (this.cline) {
-							this.cline.alwaysAllowWrite = message.bool ?? false
-						}
 						await this.postStateToWebview()
 						break
 					case "alwaysAllowExecute":
 						await this.updateGlobalState("alwaysAllowExecute", message.bool ?? undefined)
-						if (this.cline) {
-							this.cline.alwaysAllowExecute = message.bool ?? false
-						}
+						await this.postStateToWebview()
+						break
+					case "alwaysAllowBrowser":
+						await this.updateGlobalState("alwaysAllowBrowser", message.bool ?? undefined)
 						await this.postStateToWebview()
 						break
 					case "askResponse":
@@ -530,13 +509,6 @@ export class ClineProvider implements vscode.WebviewViewProvider {
 							// await this.postStateToWebview() // new Cline instance will post state when it's ready. having this here sent an empty messages array to webview leading to virtuoso having to reload the entire list
 						}
 
-						break
-					case "alwaysAllowBrowser":
-						await this.updateGlobalState("alwaysAllowBrowser", message.bool ?? undefined)
-						if (this.cline) {
-							this.cline.alwaysAllowBrowser = message.bool ?? false
-						}
-						await this.postStateToWebview()
 						break
 					// Add more switch case statements here as more webview message commands
 					// are created within the webview context (i.e. inside media/main.js)
@@ -840,12 +812,12 @@ export class ClineProvider implements vscode.WebviewViewProvider {
 		const { 
 			apiConfiguration, 
 			lastShownAnnouncementId, 
-			customInstructions, 
-			alwaysAllowReadOnly, 
-			alwaysAllowWrite, 
+			customInstructions,
+			alwaysAllowReadOnly,
+			alwaysAllowWrite,
 			alwaysAllowExecute,
-			alwaysAllowBrowser, 
-			taskHistory 
+			alwaysAllowBrowser,
+			taskHistory,
 		} = await this.getState()
 		
 		return {
@@ -947,8 +919,8 @@ export class ClineProvider implements vscode.WebviewViewProvider {
 			alwaysAllowReadOnly,
 			alwaysAllowWrite,
 			alwaysAllowExecute,
-			taskHistory,
 			alwaysAllowBrowser,
+			taskHistory,
 		] = await Promise.all([
 			this.getGlobalState("apiProvider") as Promise<ApiProvider | undefined>,
 			this.getGlobalState("apiModelId") as Promise<string | undefined>,
@@ -979,8 +951,8 @@ export class ClineProvider implements vscode.WebviewViewProvider {
 			this.getGlobalState("alwaysAllowReadOnly") as Promise<boolean | undefined>,
 			this.getGlobalState("alwaysAllowWrite") as Promise<boolean | undefined>,
 			this.getGlobalState("alwaysAllowExecute") as Promise<boolean | undefined>,
-			this.getGlobalState("taskHistory") as Promise<HistoryItem[] | undefined>,
 			this.getGlobalState("alwaysAllowBrowser") as Promise<boolean | undefined>,
+			this.getGlobalState("taskHistory") as Promise<HistoryItem[] | undefined>,
 		])
 
 		let apiProvider: ApiProvider