From 47f32f2e85a9da9784914c917e16119489942872 Mon Sep 17 00:00:00 2001 From: Simon Siefke Date: Mon, 24 Nov 2025 14:58:20 +0100 Subject: [PATCH 01/76] fix: memory leak in terminal process --- src/vs/platform/terminal/node/terminalProcess.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/platform/terminal/node/terminalProcess.ts b/src/vs/platform/terminal/node/terminalProcess.ts index 3360541b0a4..b1374107cd8 100644 --- a/src/vs/platform/terminal/node/terminalProcess.ts +++ b/src/vs/platform/terminal/node/terminalProcess.ts @@ -179,7 +179,7 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess // Delay resizes to avoid conpty not respecting very early resize calls if (isWindows) { if (useConpty && cols === 0 && rows === 0 && this.shellLaunchConfig.executable?.endsWith('Git\\bin\\bash.exe')) { - this._delayedResizer = new DelayedResizer(); + this._delayedResizer = this._register(new DelayedResizer()); this._register(this._delayedResizer.onTrigger(dimensions => { this._delayedResizer?.dispose(); this._delayedResizer = undefined; @@ -189,11 +189,11 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess })); } // WindowsShellHelper is used to fetch the process title and shell type - this.onProcessReady(e => { + this._register(this.onProcessReady(e => { this._windowsShellHelper = this._register(new WindowsShellHelper(e.pid)); this._register(this._windowsShellHelper.onShellTypeChanged(e => this._onDidChangeProperty.fire({ type: ProcessPropertyType.ShellType, value: e }))); this._register(this._windowsShellHelper.onShellNameChanged(e => this._onDidChangeProperty.fire({ type: ProcessPropertyType.Title, value: e }))); - }); + })); } this._register(toDisposable(() => { if (this._titleInterval) { From e65d19da82f4e810e9102650f56a12211feb49c9 Mon Sep 17 00:00:00 2001 From: Beace Date: Wed, 26 Nov 2025 11:40:46 +0000 Subject: [PATCH 02/76] fix: fix terminal webgl context memory leak --- src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts b/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts index a6869fe4e98..3ee8cf1c37d 100644 --- a/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts @@ -966,6 +966,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach override dispose(): void { this._anyTerminalFocusContextKey.reset(); this._anyFocusedTerminalHasSelection.reset(); + this._disposeOfWebglRenderer(); this._onDidDispose.fire(); super.dispose(); } From 4bbc4fc576941c87e0f9657d195151d513adc4b4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Dec 2025 12:54:02 +0000 Subject: [PATCH 03/76] Initial plan From a074c2aeb18bf86ea966c7befc063cf26f731bb6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Dec 2025 13:06:55 +0000 Subject: [PATCH 04/76] Add support for auto-approve suggestions when flags appear between command and subcommand Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com> --- .../browser/runInTerminalHelpers.ts | 30 ++- .../test/browser/runInTerminalHelpers.test.ts | 175 +++++++++++++++++- 2 files changed, 198 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts index d36d891f74c..fac649fdddd 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts @@ -105,13 +105,25 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str // instead of `foo`) const commandsWithSubSubCommands = new Set(['npm run', 'yarn run']); + // Helper function to find the first non-flag argument after a given index + const findNextNonFlagArg = (parts: string[], startIndex: number): string | undefined => { + for (let i = startIndex; i < parts.length; i++) { + if (!parts[i].startsWith('-')) { + return parts[i]; + } + } + return undefined; + }; + // For each unapproved sub-command (within the overall command line), decide whether to // suggest new rules for the command, a sub-command, a sub-command of a sub-command or to // not suggest at all. const subCommandsToSuggest = Array.from(new Set(coalesce(unapprovedSubCommands.map(command => { const parts = command.trim().split(/\s+/); const baseCommand = parts[0].toLowerCase(); - const baseSubCommand = parts.length > 1 ? `${parts[0]} ${parts[1]}`.toLowerCase() : ''; + // For sub-sub-commands, we need to find the first two non-flag arguments + const firstSubCommand = findNextNonFlagArg(parts, 1); + const baseSubCommand = firstSubCommand ? `${parts[0]} ${firstSubCommand}`.toLowerCase() : ''; // Security check: Never suggest auto-approval for dangerous interpreter commands if (neverAutoApproveCommands.has(baseCommand)) { @@ -119,13 +131,21 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str } if (commandsWithSubSubCommands.has(baseSubCommand)) { - if (parts.length >= 3 && !parts[2].startsWith('-')) { - return `${parts[0]} ${parts[1]} ${parts[2]}`; + // Look for the second non-flag argument after the first subcommand + // Find the index of the first subcommand in parts + const firstSubCommandIndex = parts.findIndex((part, idx) => idx > 0 && part === firstSubCommand); + if (firstSubCommandIndex !== -1) { + const subSubCommand = findNextNonFlagArg(parts, firstSubCommandIndex + 1); + if (subSubCommand) { + return `${parts[0]} ${firstSubCommand} ${subSubCommand}`; + } } return undefined; } else if (commandsWithSubcommands.has(baseCommand)) { - if (parts.length >= 2 && !parts[1].startsWith('-')) { - return `${parts[0]} ${parts[1]}`; + // Look for the first non-flag argument after the command + const subCommand = findNextNonFlagArg(parts, 1); + if (subCommand) { + return `${parts[0]} ${subCommand}`; } return undefined; } else { diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts index fc802aafa1b..3cb093a2315 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts @@ -3,8 +3,8 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { ok, strictEqual } from 'assert'; -import { TRUNCATION_MESSAGE, dedupeRules, isPowerShell, sanitizeTerminalOutput, truncateOutputKeepingTail } from '../../browser/runInTerminalHelpers.js'; +import { deepStrictEqual, ok, strictEqual } from 'assert'; +import { generateAutoApproveActions, TRUNCATION_MESSAGE, dedupeRules, isPowerShell, sanitizeTerminalOutput, truncateOutputKeepingTail } from '../../browser/runInTerminalHelpers.js'; import { OperatingSystem } from '../../../../../../base/common/platform.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { ConfigurationTarget } from '../../../../../../platform/configuration/common/configuration.js'; @@ -287,3 +287,174 @@ suite('sanitizeTerminalOutput', () => { ok(result.endsWith('line')); }); }); + +suite('generateAutoApproveActions', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + function createMockRule(sourceText: string): IAutoApproveRule { + return { + regex: new RegExp(sourceText), + regexCaseInsensitive: new RegExp(sourceText, 'i'), + sourceText, + sourceTarget: ConfigurationTarget.USER, + isDefaultRule: false + }; + } + + function createMockResult(result: 'approved' | 'denied' | 'noMatch', reason: string, rule?: IAutoApproveRule): ICommandApprovalResultWithReason { + return { + result, + reason, + rule + }; + } + + test('should suggest mvn test when command is mvn test', () => { + const commandLine = 'mvn test'; + const subCommands = ['mvn test']; + const autoApproveResult = { + subCommandResults: [createMockResult('noMatch', 'not approved')], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + const subCommandAction = actions.find(action => action.label.includes('mvn test')); + ok(subCommandAction, 'Should suggest mvn test approval'); + }); + + test('should suggest mvn test when flags appear before subcommand', () => { + const commandLine = 'mvn -DskipIT test'; + const subCommands = ['mvn -DskipIT test']; + const autoApproveResult = { + subCommandResults: [createMockResult('noMatch', 'not approved')], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + const subCommandAction = actions.find(action => action.label.includes('mvn test')); + ok(subCommandAction, 'Should suggest mvn test approval even with flags before subcommand'); + }); + + test('should suggest mvn test when multiple flags appear before subcommand', () => { + const commandLine = 'mvn -X -DskipIT test'; + const subCommands = ['mvn -X -DskipIT test']; + const autoApproveResult = { + subCommandResults: [createMockResult('noMatch', 'not approved')], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + const subCommandAction = actions.find(action => action.label.includes('mvn test')); + ok(subCommandAction, 'Should suggest mvn test approval with multiple flags'); + }); + + test('should suggest gradle build when flags appear before subcommand', () => { + const commandLine = 'gradle --info build'; + const subCommands = ['gradle --info build']; + const autoApproveResult = { + subCommandResults: [createMockResult('noMatch', 'not approved')], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + const subCommandAction = actions.find(action => action.label.includes('gradle build')); + ok(subCommandAction, 'Should suggest gradle build approval'); + }); + + test('should suggest npm run test when flags appear before subcommand', () => { + const commandLine = 'npm --silent run test'; + const subCommands = ['npm --silent run test']; + const autoApproveResult = { + subCommandResults: [createMockResult('noMatch', 'not approved')], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + // npm run is a sub-sub-command case, should suggest npm run test + const subCommandAction = actions.find(action => action.label.includes('npm run test')); + ok(subCommandAction, 'Should suggest npm run test approval'); + }); + + test('should suggest npm run test when flags appear between run and test', () => { + const commandLine = 'npm --silent run --verbose test'; + const subCommands = ['npm --silent run --verbose test']; + const autoApproveResult = { + subCommandResults: [createMockResult('noMatch', 'not approved')], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + const subCommandAction = actions.find(action => action.label.includes('npm run test')); + ok(subCommandAction, 'Should suggest npm run test approval even with flags between run and test'); + }); + + test('should not suggest approval when only flags and no subcommand', () => { + const commandLine = 'mvn -X -DskipIT'; + const subCommands = ['mvn -X -DskipIT']; + const autoApproveResult = { + subCommandResults: [createMockResult('noMatch', 'not approved')], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + const subCommandAction = actions.find(action => action.label.includes('Always Allow Command:') && action.label.includes('mvn')); + strictEqual(subCommandAction, undefined, 'Should not suggest mvn approval when no subcommand found'); + }); + + test('should suggest exact command line when subcommand cannot be extracted', () => { + const commandLine = 'mvn -X -DskipIT'; + const subCommands = ['mvn -X -DskipIT']; + const autoApproveResult = { + subCommandResults: [createMockResult('noMatch', 'not approved')], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + const exactCommandAction = actions.find(action => action.label.includes('Always Allow Exact Command Line')); + ok(exactCommandAction, 'Should suggest exact command line approval'); + }); + + test('should handle multiple subcommands with flags', () => { + const commandLine = 'mvn -DskipIT test && gradle --info build'; + const subCommands = ['mvn -DskipIT test', 'gradle --info build']; + const autoApproveResult = { + subCommandResults: [ + createMockResult('noMatch', 'not approved'), + createMockResult('noMatch', 'not approved') + ], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + const subCommandAction = actions.find(action => + action.label.includes('mvn test') && action.label.includes('gradle build') + ); + ok(subCommandAction, 'Should suggest both mvn test and gradle build'); + }); + + test('should not suggest when commands are denied', () => { + const commandLine = 'mvn -DskipIT test'; + const subCommands = ['mvn -DskipIT test']; + const autoApproveResult = { + subCommandResults: [createMockResult('denied', 'denied by rule', createMockRule('mvn test'))], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + const subCommandAction = actions.find(action => action.label.includes('Always Allow Command:')); + strictEqual(subCommandAction, undefined, 'Should not suggest approval for denied commands'); + }); + + test('should not suggest when commands are already approved', () => { + const commandLine = 'mvn -DskipIT test'; + const subCommands = ['mvn -DskipIT test']; + const autoApproveResult = { + subCommandResults: [createMockResult('approved', 'approved by rule', createMockRule('mvn test'))], + commandLineResult: createMockResult('noMatch', 'not approved') + }; + + const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); + const subCommandAction = actions.find(action => action.label.includes('mvn test') && action.label.includes('Always Allow Command:')); + strictEqual(subCommandAction, undefined, 'Should not suggest approval for already approved commands'); + }); +}); From 0a7c09faebf1ac0662d798aefc5414bd245f77b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Dec 2025 13:10:14 +0000 Subject: [PATCH 05/76] Address code review feedback: add clarifying comment and fix regex escaping in tests Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com> --- .../chatAgentTools/browser/runInTerminalHelpers.ts | 3 +++ .../test/browser/runInTerminalHelpers.test.ts | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts index fac649fdddd..fcf576e37e9 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts @@ -106,6 +106,9 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str const commandsWithSubSubCommands = new Set(['npm run', 'yarn run']); // Helper function to find the first non-flag argument after a given index + // Note: This considers any argument starting with '-' as a flag, which is the + // standard convention for CLI tools. Subcommands in tools like mvn, gradle, npm + // never start with '-'. const findNextNonFlagArg = (parts: string[], startIndex: number): string | undefined => { for (let i = startIndex; i < parts.length; i++) { if (!parts[i].startsWith('-')) { diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts index 3cb093a2315..3ea9cc7e7bf 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts @@ -292,9 +292,11 @@ suite('generateAutoApproveActions', () => { ensureNoDisposablesAreLeakedInTestSuite(); function createMockRule(sourceText: string): IAutoApproveRule { + // Escape special regex characters for test purposes + const escapedText = sourceText.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); return { - regex: new RegExp(sourceText), - regexCaseInsensitive: new RegExp(sourceText, 'i'), + regex: new RegExp(escapedText), + regexCaseInsensitive: new RegExp(escapedText, 'i'), sourceText, sourceTarget: ConfigurationTarget.USER, isDefaultRule: false From 137cd8dad63fc9d36cdbd6cc587ec7f526c695bc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Dec 2025 15:01:04 +0000 Subject: [PATCH 06/76] Fix: Include flags in auto-approve suggestions as specified by @Tyriar Changed implementation to suggest the exact command with flags included (e.g., `mvn -DskipIT test` instead of just `mvn test`). This matches the requirement that for `mvn -abc foo bar`, the suggestion should be `mvn -abc foo` (including the flags between command and subcommand). Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com> --- .../browser/runInTerminalHelpers.ts | 42 +++++++++---------- .../test/browser/runInTerminalHelpers.test.ts | 40 +++++++++--------- 2 files changed, 38 insertions(+), 44 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts index fcf576e37e9..dc093f7250b 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts @@ -106,13 +106,10 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str const commandsWithSubSubCommands = new Set(['npm run', 'yarn run']); // Helper function to find the first non-flag argument after a given index - // Note: This considers any argument starting with '-' as a flag, which is the - // standard convention for CLI tools. Subcommands in tools like mvn, gradle, npm - // never start with '-'. - const findNextNonFlagArg = (parts: string[], startIndex: number): string | undefined => { + const findNextNonFlagArg = (parts: string[], startIndex: number): number | undefined => { for (let i = startIndex; i < parts.length; i++) { if (!parts[i].startsWith('-')) { - return parts[i]; + return i; } } return undefined; @@ -124,33 +121,32 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str const subCommandsToSuggest = Array.from(new Set(coalesce(unapprovedSubCommands.map(command => { const parts = command.trim().split(/\s+/); const baseCommand = parts[0].toLowerCase(); - // For sub-sub-commands, we need to find the first two non-flag arguments - const firstSubCommand = findNextNonFlagArg(parts, 1); - const baseSubCommand = firstSubCommand ? `${parts[0]} ${firstSubCommand}`.toLowerCase() : ''; // Security check: Never suggest auto-approval for dangerous interpreter commands if (neverAutoApproveCommands.has(baseCommand)) { return undefined; } - if (commandsWithSubSubCommands.has(baseSubCommand)) { - // Look for the second non-flag argument after the first subcommand - // Find the index of the first subcommand in parts - const firstSubCommandIndex = parts.findIndex((part, idx) => idx > 0 && part === firstSubCommand); - if (firstSubCommandIndex !== -1) { - const subSubCommand = findNextNonFlagArg(parts, firstSubCommandIndex + 1); - if (subSubCommand) { - return `${parts[0]} ${firstSubCommand} ${subSubCommand}`; + if (commandsWithSubcommands.has(baseCommand)) { + // Find the first non-flag argument after the command + const subCommandIndex = findNextNonFlagArg(parts, 1); + if (subCommandIndex !== undefined) { + // Check if this is a sub-sub-command case + const baseSubCommand = `${parts[0]} ${parts[subCommandIndex]}`.toLowerCase(); + if (commandsWithSubSubCommands.has(baseSubCommand)) { + // Look for the second non-flag argument after the first subcommand + const subSubCommandIndex = findNextNonFlagArg(parts, subCommandIndex + 1); + if (subSubCommandIndex !== undefined) { + // Include everything from command to sub-sub-command (including flags) + return parts.slice(0, subSubCommandIndex + 1).join(' '); + } + return undefined; + } else { + // Include everything from command to subcommand (including flags) + return parts.slice(0, subCommandIndex + 1).join(' '); } } return undefined; - } else if (commandsWithSubcommands.has(baseCommand)) { - // Look for the first non-flag argument after the command - const subCommand = findNextNonFlagArg(parts, 1); - if (subCommand) { - return `${parts[0]} ${subCommand}`; - } - return undefined; } else { return parts[0]; } diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts index 3ea9cc7e7bf..a66edaf12c7 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { deepStrictEqual, ok, strictEqual } from 'assert'; +import { ok, strictEqual } from 'assert'; import { generateAutoApproveActions, TRUNCATION_MESSAGE, dedupeRules, isPowerShell, sanitizeTerminalOutput, truncateOutputKeepingTail } from '../../browser/runInTerminalHelpers.js'; import { OperatingSystem } from '../../../../../../base/common/platform.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; @@ -292,7 +292,6 @@ suite('generateAutoApproveActions', () => { ensureNoDisposablesAreLeakedInTestSuite(); function createMockRule(sourceText: string): IAutoApproveRule { - // Escape special regex characters for test purposes const escapedText = sourceText.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); return { regex: new RegExp(escapedText), @@ -324,7 +323,7 @@ suite('generateAutoApproveActions', () => { ok(subCommandAction, 'Should suggest mvn test approval'); }); - test('should suggest mvn test when flags appear before subcommand', () => { + test('should suggest mvn -DskipIT test when flags appear before subcommand', () => { const commandLine = 'mvn -DskipIT test'; const subCommands = ['mvn -DskipIT test']; const autoApproveResult = { @@ -333,11 +332,11 @@ suite('generateAutoApproveActions', () => { }; const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); - const subCommandAction = actions.find(action => action.label.includes('mvn test')); - ok(subCommandAction, 'Should suggest mvn test approval even with flags before subcommand'); + const subCommandAction = actions.find(action => action.label.includes('mvn -DskipIT test')); + ok(subCommandAction, 'Should suggest mvn -DskipIT test approval (including flags)'); }); - test('should suggest mvn test when multiple flags appear before subcommand', () => { + test('should suggest mvn -X -DskipIT test when multiple flags appear before subcommand', () => { const commandLine = 'mvn -X -DskipIT test'; const subCommands = ['mvn -X -DskipIT test']; const autoApproveResult = { @@ -346,11 +345,11 @@ suite('generateAutoApproveActions', () => { }; const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); - const subCommandAction = actions.find(action => action.label.includes('mvn test')); - ok(subCommandAction, 'Should suggest mvn test approval with multiple flags'); + const subCommandAction = actions.find(action => action.label.includes('mvn -X -DskipIT test')); + ok(subCommandAction, 'Should suggest mvn -X -DskipIT test approval with multiple flags'); }); - test('should suggest gradle build when flags appear before subcommand', () => { + test('should suggest gradle --info build when flags appear before subcommand', () => { const commandLine = 'gradle --info build'; const subCommands = ['gradle --info build']; const autoApproveResult = { @@ -359,11 +358,11 @@ suite('generateAutoApproveActions', () => { }; const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); - const subCommandAction = actions.find(action => action.label.includes('gradle build')); - ok(subCommandAction, 'Should suggest gradle build approval'); + const subCommandAction = actions.find(action => action.label.includes('gradle --info build')); + ok(subCommandAction, 'Should suggest gradle --info build approval'); }); - test('should suggest npm run test when flags appear before subcommand', () => { + test('should suggest npm --silent run test when flags appear before subcommand', () => { const commandLine = 'npm --silent run test'; const subCommands = ['npm --silent run test']; const autoApproveResult = { @@ -372,12 +371,11 @@ suite('generateAutoApproveActions', () => { }; const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); - // npm run is a sub-sub-command case, should suggest npm run test - const subCommandAction = actions.find(action => action.label.includes('npm run test')); - ok(subCommandAction, 'Should suggest npm run test approval'); + const subCommandAction = actions.find(action => action.label.includes('npm --silent run test')); + ok(subCommandAction, 'Should suggest npm --silent run test approval (sub-sub-command with flags)'); }); - test('should suggest npm run test when flags appear between run and test', () => { + test('should suggest npm --silent run --verbose test when flags appear between subcommands', () => { const commandLine = 'npm --silent run --verbose test'; const subCommands = ['npm --silent run --verbose test']; const autoApproveResult = { @@ -386,8 +384,8 @@ suite('generateAutoApproveActions', () => { }; const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); - const subCommandAction = actions.find(action => action.label.includes('npm run test')); - ok(subCommandAction, 'Should suggest npm run test approval even with flags between run and test'); + const subCommandAction = actions.find(action => action.label.includes('npm --silent run --verbose test')); + ok(subCommandAction, 'Should suggest npm --silent run --verbose test with flags between subcommands'); }); test('should not suggest approval when only flags and no subcommand', () => { @@ -429,9 +427,9 @@ suite('generateAutoApproveActions', () => { const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); const subCommandAction = actions.find(action => - action.label.includes('mvn test') && action.label.includes('gradle build') + action.label.includes('mvn -DskipIT test') && action.label.includes('gradle --info build') ); - ok(subCommandAction, 'Should suggest both mvn test and gradle build'); + ok(subCommandAction, 'Should suggest both mvn -DskipIT test and gradle --info build'); }); test('should not suggest when commands are denied', () => { @@ -456,7 +454,7 @@ suite('generateAutoApproveActions', () => { }; const actions = generateAutoApproveActions(commandLine, subCommands, autoApproveResult); - const subCommandAction = actions.find(action => action.label.includes('mvn test') && action.label.includes('Always Allow Command:')); + const subCommandAction = actions.find(action => action.label.includes('mvn -DskipIT test') && action.label.includes('Always Allow Command:')); strictEqual(subCommandAction, undefined, 'Should not suggest approval for already approved commands'); }); }); From 046772561f70ad5bb3280208d945f696ab9d3dfd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Dec 2025 15:03:09 +0000 Subject: [PATCH 07/76] Add comment explaining regex escaping in test helper Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com> --- .../chatAgentTools/test/browser/runInTerminalHelpers.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts index a66edaf12c7..142183b262b 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts @@ -292,6 +292,7 @@ suite('generateAutoApproveActions', () => { ensureNoDisposablesAreLeakedInTestSuite(); function createMockRule(sourceText: string): IAutoApproveRule { + // Escape special regex characters for test purposes to prevent regex errors const escapedText = sourceText.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); return { regex: new RegExp(escapedText), From 7f7ef2f04b22dca69e231dbc9e4aff46a58db901 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 18 Dec 2025 05:37:50 -0800 Subject: [PATCH 08/76] Add workspace and session allow options in terminal tool Part of #270529 --- .../chatTerminalToolConfirmationSubPart.ts | 1 + .../browser/runInTerminalHelpers.ts | 66 +++++++++++++++++-- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts index ffd78ae9466..56e03e66193 100644 --- a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts +++ b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts @@ -55,6 +55,7 @@ export interface ITerminalNewAutoApproveRule { approve: boolean; matchCommandLine?: boolean; }; + scope: 'session' | 'workspace' | 'user'; } export type TerminalNewAutoApproveButtonData = ( diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts index d36d891f74c..1105fa6949f 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts @@ -136,22 +136,49 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str if (subCommandsToSuggest.length > 0) { let subCommandLabel: string; if (subCommandsToSuggest.length === 1) { - subCommandLabel = localize('autoApprove.baseCommandSingle', 'Always Allow Command: {0}', subCommandsToSuggest[0]); + subCommandLabel = localize('autoApprove.baseCommandSingle', '"{0} ..."', subCommandsToSuggest[0]); } else { const commandSeparated = subCommandsToSuggest.join(', '); - subCommandLabel = localize('autoApprove.baseCommand', 'Always Allow Commands: {0}', commandSeparated); + subCommandLabel = localize('autoApprove.baseCommand', '"{0} ..."', commandSeparated); } actions.push({ - label: subCommandLabel, + label: `Allow ${subCommandLabel} in this Session`, data: { type: 'newRule', rule: subCommandsToSuggest.map(key => ({ key, - value: true + value: true, + scope: 'session' })) } satisfies TerminalNewAutoApproveButtonData }); + actions.push({ + label: `Allow ${subCommandLabel} in this Workspace`, + data: { + type: 'newRule', + rule: subCommandsToSuggest.map(key => ({ + key, + value: true, + scope: 'workspace' + })) + } satisfies TerminalNewAutoApproveButtonData + }); + actions.push({ + label: `Always Allow ${subCommandLabel}`, + data: { + type: 'newRule', + rule: subCommandsToSuggest.map(key => ({ + key, + value: true, + scope: 'user' + })) + } satisfies TerminalNewAutoApproveButtonData + }); + } + + if (actions.length > 0) { + actions.push(new Separator()); } // Allow exact command line, don't do this if it's just the first sub-command's first @@ -162,6 +189,34 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str !commandsWithSubcommands.has(commandLine) && !commandsWithSubSubCommands.has(commandLine) ) { + actions.push({ + label: localize('autoApprove.exactCommand1', 'Allow Exact Command Line in this Session'), + data: { + type: 'newRule', + rule: { + key: `/^${escapeRegExpCharacters(commandLine)}$/`, + value: { + approve: true, + matchCommandLine: true + }, + scope: 'session' + } + } satisfies TerminalNewAutoApproveButtonData + }); + actions.push({ + label: localize('autoApprove.exactCommand2', 'Allow Exact Command Line in this Workspace'), + data: { + type: 'newRule', + rule: { + key: `/^${escapeRegExpCharacters(commandLine)}$/`, + value: { + approve: true, + matchCommandLine: true + }, + scope: 'workspace' + } + } satisfies TerminalNewAutoApproveButtonData + }); actions.push({ label: localize('autoApprove.exactCommand', 'Always Allow Exact Command Line'), data: { @@ -171,7 +226,8 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str value: { approve: true, matchCommandLine: true - } + }, + scope: 'user' } } satisfies TerminalNewAutoApproveButtonData }); From c943585d550f6f6cb32dcd97f1553d2fa35d06bc Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 18 Dec 2025 06:11:46 -0800 Subject: [PATCH 09/76] Implement rules and update presentation --- .../chatTerminalToolConfirmationSubPart.ts | 100 +++++++++++++----- .../contrib/terminal/browser/terminal.ts | 13 +++ .../chat/browser/terminalChatService.ts | 14 +++ .../browser/commandLineAutoApprover.ts | 85 ++++++++++++++- .../browser/runInTerminalHelpers.ts | 5 +- .../commandLineAutoApproveAnalyzer.ts | 26 ++++- 6 files changed, 208 insertions(+), 35 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts index 56e03e66193..4f7c8d3e02d 100644 --- a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts +++ b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts @@ -259,28 +259,64 @@ export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationS } case 'newRule': { const newRules = asArray(data.rule); - const inspect = this.configurationService.inspect(TerminalContribSettingId.AutoApprove); - const oldValue = (inspect.user?.value as Record | undefined) ?? {}; - let newValue: Record; - if (isObject(oldValue)) { - newValue = { ...oldValue }; - for (const newRule of newRules) { - newValue[newRule.key] = newRule.value; - } - } else { - this.preferencesService.openSettings({ - jsonEditor: true, - target: ConfigurationTarget.USER, - revealSetting: { - key: TerminalContribSettingId.AutoApprove - }, - }); - throw new ErrorNoTelemetry(`Cannot add new rule, existing setting is unexpected format`); + + // Group rules by scope + const sessionRules = newRules.filter(r => r.scope === 'session'); + const workspaceRules = newRules.filter(r => r.scope === 'workspace'); + const userRules = newRules.filter(r => r.scope === 'user'); + + // Handle session-scoped rules (temporary, in-memory only) + for (const rule of sessionRules) { + this.terminalChatService.addSessionAutoApproveRule(rule.key, rule.value); } - await this.configurationService.updateValue(TerminalContribSettingId.AutoApprove, newValue, ConfigurationTarget.USER); - function formatRuleLinks(newRules: ITerminalNewAutoApproveRule[]): string { - return newRules.map(e => { - const settingsUri = createCommandUri(TerminalContribCommandId.OpenTerminalSettingsLink, ConfigurationTarget.USER); + + // Handle workspace-scoped rules + if (workspaceRules.length > 0) { + const inspect = this.configurationService.inspect(TerminalContribSettingId.AutoApprove); + const oldValue = (inspect.workspaceValue as Record | undefined) ?? {}; + if (isObject(oldValue)) { + const newValue: Record = { ...oldValue }; + for (const rule of workspaceRules) { + newValue[rule.key] = rule.value; + } + await this.configurationService.updateValue(TerminalContribSettingId.AutoApprove, newValue, ConfigurationTarget.WORKSPACE); + } else { + this.preferencesService.openSettings({ + jsonEditor: true, + target: ConfigurationTarget.WORKSPACE, + revealSetting: { key: TerminalContribSettingId.AutoApprove }, + }); + throw new ErrorNoTelemetry(`Cannot add new rule, existing workspace setting is unexpected format`); + } + } + + // Handle user-scoped rules + if (userRules.length > 0) { + const inspect = this.configurationService.inspect(TerminalContribSettingId.AutoApprove); + const oldValue = (inspect.userValue as Record | undefined) ?? {}; + if (isObject(oldValue)) { + const newValue: Record = { ...oldValue }; + for (const rule of userRules) { + newValue[rule.key] = rule.value; + } + await this.configurationService.updateValue(TerminalContribSettingId.AutoApprove, newValue, ConfigurationTarget.USER); + } else { + this.preferencesService.openSettings({ + jsonEditor: true, + target: ConfigurationTarget.USER, + revealSetting: { key: TerminalContribSettingId.AutoApprove }, + }); + throw new ErrorNoTelemetry(`Cannot add new rule, existing setting is unexpected format`); + } + } + + function formatRuleLinks(rules: ITerminalNewAutoApproveRule[], scope: 'session' | 'workspace' | 'user'): string { + return rules.map(e => { + if (scope === 'session') { + return `\`${e.key}\``; + } + const target = scope === 'workspace' ? ConfigurationTarget.WORKSPACE : ConfigurationTarget.USER; + const settingsUri = createCommandUri(TerminalContribCommandId.OpenTerminalSettingsLink, target); return `[\`${e.key}\`](${settingsUri.toString()} "${localize('ruleTooltip', 'View rule in settings')}")`; }).join(', '); } @@ -289,10 +325,24 @@ export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationS enabledCommands: [TerminalContribCommandId.OpenTerminalSettingsLink] } }; - if (newRules.length === 1) { - terminalData.autoApproveInfo = new MarkdownString(localize('newRule', 'Auto approve rule {0} added', formatRuleLinks(newRules)), mdTrustSettings); - } else if (newRules.length > 1) { - terminalData.autoApproveInfo = new MarkdownString(localize('newRule.plural', 'Auto approve rules {0} added', formatRuleLinks(newRules)), mdTrustSettings); + const parts: string[] = []; + if (sessionRules.length > 0) { + parts.push(sessionRules.length === 1 + ? localize('newRule.session', 'Session rule {0} added', formatRuleLinks(sessionRules, 'session')) + : localize('newRule.session.plural', 'Session rules {0} added', formatRuleLinks(sessionRules, 'session'))); + } + if (workspaceRules.length > 0) { + parts.push(workspaceRules.length === 1 + ? localize('newRule.workspace', 'Workspace rule {0} added', formatRuleLinks(workspaceRules, 'workspace')) + : localize('newRule.workspace.plural', 'Workspace rules {0} added', formatRuleLinks(workspaceRules, 'workspace'))); + } + if (userRules.length > 0) { + parts.push(userRules.length === 1 + ? localize('newRule.user', 'User rule {0} added', formatRuleLinks(userRules, 'user')) + : localize('newRule.user.plural', 'User rules {0} added', formatRuleLinks(userRules, 'user'))); + } + if (parts.length > 0) { + terminalData.autoApproveInfo = new MarkdownString(parts.join(', '), mdTrustSettings); } toolConfirmKind = ToolConfirmKind.UserAction; break; diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index 0798c8feec8..d85cc00157f 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -217,6 +217,19 @@ export interface ITerminalChatService { * @returns True if the session has auto approval enabled */ hasChatSessionAutoApproval(chatSessionId: string): boolean; + + /** + * Add a session-scoped auto-approve rule. + * @param key The rule key (command or regex pattern) + * @param value The rule value (approval boolean or object with approve and matchCommandLine) + */ + addSessionAutoApproveRule(key: string, value: boolean | { approve: boolean; matchCommandLine?: boolean }): void; + + /** + * Get all session-scoped auto-approve rules. + * @returns A record of all session-scoped auto-approve rules + */ + getSessionAutoApproveRules(): Readonly>; } /** diff --git a/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts b/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts index 6e176da9fd6..4cf7e886fbb 100644 --- a/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts +++ b/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts @@ -53,6 +53,12 @@ export class TerminalChatService extends Disposable implements ITerminalChatServ */ private readonly _sessionAutoApprovalEnabled = new Set(); + /** + * Tracks session-scoped auto-approve rules. These are temporary rules that last only for the + * duration of the VS Code session (not persisted to disk). + */ + private readonly _sessionAutoApproveRules: Record = {}; + constructor( @ILogService private readonly _logService: ILogService, @ITerminalService private readonly _terminalService: ITerminalService, @@ -313,4 +319,12 @@ export class TerminalChatService extends Disposable implements ITerminalChatServ hasChatSessionAutoApproval(chatSessionId: string): boolean { return this._sessionAutoApprovalEnabled.has(chatSessionId); } + + addSessionAutoApproveRule(key: string, value: boolean | { approve: boolean; matchCommandLine?: boolean }): void { + this._sessionAutoApproveRules[key] = value; + } + + getSessionAutoApproveRules(): Readonly> { + return this._sessionAutoApproveRules; + } } diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandLineAutoApprover.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandLineAutoApprover.ts index fd0be4041a1..381937619e8 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandLineAutoApprover.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandLineAutoApprover.ts @@ -11,12 +11,20 @@ import { structuralEquals } from '../../../../../base/common/equals.js'; import { ConfigurationTarget, IConfigurationService, type IConfigurationValue } from '../../../../../platform/configuration/common/configuration.js'; import { TerminalChatAgentToolsSettingId } from '../common/terminalChatAgentToolsConfiguration.js'; import { isPowerShell } from './runInTerminalHelpers.js'; +import { ITerminalChatService } from '../../../terminal/browser/terminal.js'; + +export const enum AutoApproveRuleSource { + Default = 'default', + User = 'user', + Workspace = 'workspace', + Session = 'session' +} export interface IAutoApproveRule { regex: RegExp; regexCaseInsensitive: RegExp; sourceText: string; - sourceTarget: ConfigurationTarget; + sourceTarget: ConfigurationTarget | 'session'; isDefaultRule: boolean; } @@ -39,6 +47,7 @@ export class CommandLineAutoApprover extends Disposable { constructor( @IConfigurationService private readonly _configurationService: IConfigurationService, + @ITerminalChatService private readonly _terminalChatService: ITerminalChatService, ) { super(); this.updateConfiguration(); @@ -86,7 +95,7 @@ export class CommandLineAutoApprover extends Disposable { }; } - // Check the deny list to see if this command requires explicit approval + // Check the config deny list to see if this command requires explicit approval for (const rule of this._denyListRules) { if (this._commandMatchesRule(rule, command, shell, os)) { return { @@ -97,7 +106,18 @@ export class CommandLineAutoApprover extends Disposable { } } - // Check the allow list to see if the command is allowed to run without explicit approval + // Check session allow rules (session deny rules can't exist) + for (const rule of this._getSessionRules().allowListRules) { + if (this._commandMatchesRule(rule, command, shell, os)) { + return { + result: 'approved', + rule, + reason: `Command '${command}' is approved by session allow list rule: ${rule.sourceText}` + }; + } + } + + // Check the config allow list to see if the command is allowed to run without explicit approval for (const rule of this._allowListRules) { if (this._commandMatchesRule(rule, command, shell, os)) { return { @@ -118,7 +138,7 @@ export class CommandLineAutoApprover extends Disposable { } isCommandLineAutoApproved(commandLine: string): ICommandApprovalResultWithReason { - // Check the deny list first to see if this command line requires explicit approval + // Check the config deny list first to see if this command line requires explicit approval for (const rule of this._denyListCommandLineRules) { if (rule.regex.test(commandLine)) { return { @@ -129,7 +149,18 @@ export class CommandLineAutoApprover extends Disposable { } } - // Check if the full command line matches any of the allow list command line regexes + // Check session allow list (session deny rules can't exist) + for (const rule of this._getSessionRules().allowListCommandLineRules) { + if (rule.regex.test(commandLine)) { + return { + result: 'approved', + rule, + reason: `Command line '${commandLine}' is approved by session allow list rule: ${rule.sourceText}` + }; + } + } + + // Check if the full command line matches any of the config allow list command line regexes for (const rule of this._allowListCommandLineRules) { if (rule.regex.test(commandLine)) { return { @@ -145,6 +176,50 @@ export class CommandLineAutoApprover extends Disposable { }; } + private _getSessionRules(): { + denyListRules: IAutoApproveRule[]; + allowListRules: IAutoApproveRule[]; + allowListCommandLineRules: IAutoApproveRule[]; + denyListCommandLineRules: IAutoApproveRule[]; + } { + const denyListRules: IAutoApproveRule[] = []; + const allowListRules: IAutoApproveRule[] = []; + const allowListCommandLineRules: IAutoApproveRule[] = []; + const denyListCommandLineRules: IAutoApproveRule[] = []; + + const sessionRulesConfig = this._terminalChatService.getSessionAutoApproveRules(); + for (const [key, value] of Object.entries(sessionRulesConfig)) { + if (typeof value === 'boolean') { + const { regex, regexCaseInsensitive } = this._convertAutoApproveEntryToRegex(key); + if (value === true) { + allowListRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false }); + } else if (value === false) { + denyListRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false }); + } + } else if (typeof value === 'object' && value !== null) { + const objectValue = value as { approve?: boolean; matchCommandLine?: boolean }; + if (typeof objectValue.approve === 'boolean') { + const { regex, regexCaseInsensitive } = this._convertAutoApproveEntryToRegex(key); + if (objectValue.approve === true) { + if (objectValue.matchCommandLine === true) { + allowListCommandLineRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false }); + } else { + allowListRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false }); + } + } else if (objectValue.approve === false) { + if (objectValue.matchCommandLine === true) { + denyListCommandLineRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false }); + } else { + denyListRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false }); + } + } + } + } + } + + return { denyListRules, allowListRules, allowListCommandLineRules, denyListCommandLineRules }; + } + private _commandMatchesRule(rule: IAutoApproveRule, command: string, shell: string, os: OperatingSystem): boolean { const isPwsh = isPowerShell(shell, os); diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts index 1105fa6949f..af1f88c966c 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts @@ -136,10 +136,9 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str if (subCommandsToSuggest.length > 0) { let subCommandLabel: string; if (subCommandsToSuggest.length === 1) { - subCommandLabel = localize('autoApprove.baseCommandSingle', '"{0} ..."', subCommandsToSuggest[0]); + subCommandLabel = `\`${subCommandsToSuggest[0]} \u2026\``; } else { - const commandSeparated = subCommandsToSuggest.join(', '); - subCommandLabel = localize('autoApprove.baseCommand', '"{0} ..."', commandSeparated); + subCommandLabel = `Commands ${subCommandsToSuggest.map(e => `\`${e} \u2026\``).join(', ')}`; } actions.push({ diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts index cb8181c67df..77db325f68f 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts @@ -8,7 +8,7 @@ import { createCommandUri, MarkdownString, type IMarkdownString } from '../../.. import { Disposable } from '../../../../../../../base/common/lifecycle.js'; import type { SingleOrMany } from '../../../../../../../base/common/types.js'; import { localize } from '../../../../../../../nls.js'; -import { IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js'; +import { ConfigurationTarget, IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js'; import { IInstantiationService } from '../../../../../../../platform/instantiation/common/instantiation.js'; import { ITerminalChatService } from '../../../../../terminal/browser/terminal.js'; import { IStorageService, StorageScope } from '../../../../../../../platform/storage/common/storage.js'; @@ -191,8 +191,30 @@ export class CommandLineAutoApproveAnalyzer extends Disposable implements IComma ): IMarkdownString | undefined { const formatRuleLinks = (result: SingleOrMany<{ result: ICommandApprovalResult; rule?: IAutoApproveRule; reason: string }>): string => { return asArray(result).map(e => { + // Session rules cannot be actioned currently so no link + if (e.rule!.sourceTarget === 'session') { + return localize('autoApproveRule.sessionIndicator', '{0} (session)', `\`${e.rule!.sourceText}\``); + } const settingsUri = createCommandUri(TerminalChatCommandId.OpenTerminalSettingsLink, e.rule!.sourceTarget); - return `[\`${e.rule!.sourceText}\`](${settingsUri.toString()} "${localize('ruleTooltip', 'View rule in settings')}")`; + const tooltip = localize('ruleTooltip', 'View rule in settings'); + let label = e.rule!.sourceText; + switch (e.rule?.sourceTarget) { + case ConfigurationTarget.DEFAULT: + label = `${label} (default)`; + break; + case ConfigurationTarget.USER: + case ConfigurationTarget.USER_LOCAL: + label = `${label} (user)`; + break; + case ConfigurationTarget.USER_REMOTE: + label = `${label} (remote)`; + break; + case ConfigurationTarget.WORKSPACE: + case ConfigurationTarget.WORKSPACE_FOLDER: + label = `${label} (workspace)`; + break; + } + return `[\`${label}\`](${settingsUri.toString()} "${tooltip}")`; }).join(', '); }; From 6bcd25a25a578fb93987f575757911a4cc54518d Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 18 Dec 2025 07:05:55 -0800 Subject: [PATCH 10/76] Ensure session rules only apply to a single session --- .../chatTerminalToolConfirmationSubPart.ts | 3 +- .../contrib/terminal/browser/terminal.ts | 10 ++++--- .../chat/browser/terminalChatService.ts | 30 ++++++++++++++----- .../browser/commandLineAutoApprover.ts | 16 ++++++---- .../commandLineAutoApproveAnalyzer.ts | 4 +-- 5 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts index 4f7c8d3e02d..bcc61073330 100644 --- a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts +++ b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts @@ -266,8 +266,9 @@ export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationS const userRules = newRules.filter(r => r.scope === 'user'); // Handle session-scoped rules (temporary, in-memory only) + const chatSessionId = this.context.element.sessionId; for (const rule of sessionRules) { - this.terminalChatService.addSessionAutoApproveRule(rule.key, rule.value); + this.terminalChatService.addSessionAutoApproveRule(chatSessionId, rule.key, rule.value); } // Handle workspace-scoped rules diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index d85cc00157f..c41ab0d9d4f 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -220,16 +220,18 @@ export interface ITerminalChatService { /** * Add a session-scoped auto-approve rule. + * @param chatSessionId The chat session ID to associate the rule with * @param key The rule key (command or regex pattern) * @param value The rule value (approval boolean or object with approve and matchCommandLine) */ - addSessionAutoApproveRule(key: string, value: boolean | { approve: boolean; matchCommandLine?: boolean }): void; + addSessionAutoApproveRule(chatSessionId: string, key: string, value: boolean | { approve: boolean; matchCommandLine?: boolean }): void; /** - * Get all session-scoped auto-approve rules. - * @returns A record of all session-scoped auto-approve rules + * Get all session-scoped auto-approve rules for a specific chat session. + * @param chatSessionId The chat session ID to get rules for + * @returns A record of all session-scoped auto-approve rules for the session */ - getSessionAutoApproveRules(): Readonly>; + getSessionAutoApproveRules(chatSessionId: string): Readonly>; } /** diff --git a/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts b/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts index 4cf7e886fbb..438c345051b 100644 --- a/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts +++ b/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts @@ -54,10 +54,11 @@ export class TerminalChatService extends Disposable implements ITerminalChatServ private readonly _sessionAutoApprovalEnabled = new Set(); /** - * Tracks session-scoped auto-approve rules. These are temporary rules that last only for the - * duration of the VS Code session (not persisted to disk). + * Tracks session-scoped auto-approve rules per chat session. These are temporary rules that + * last only for the duration of the chat session (not persisted to disk). + * Map> */ - private readonly _sessionAutoApproveRules: Record = {}; + private readonly _sessionAutoApproveRules = new Map>(); constructor( @ILogService private readonly _logService: ILogService, @@ -72,6 +73,16 @@ export class TerminalChatService extends Disposable implements ITerminalChatServ this._hasHiddenToolTerminalContext = TerminalChatContextKeys.hasHiddenChatTerminals.bindTo(this._contextKeyService); this._restoreFromStorage(); + + // Clear session auto-approve rules when chat sessions end + this._register(this._chatService.onDidDisposeSession(e => { + for (const resource of e.sessionResource) { + const sessionId = LocalChatSessionUri.parseLocalSessionId(resource); + if (sessionId) { + this._sessionAutoApproveRules.delete(sessionId); + } + } + })); } registerTerminalInstanceWithToolSession(terminalToolSessionId: string | undefined, instance: ITerminalInstance): void { @@ -320,11 +331,16 @@ export class TerminalChatService extends Disposable implements ITerminalChatServ return this._sessionAutoApprovalEnabled.has(chatSessionId); } - addSessionAutoApproveRule(key: string, value: boolean | { approve: boolean; matchCommandLine?: boolean }): void { - this._sessionAutoApproveRules[key] = value; + addSessionAutoApproveRule(chatSessionId: string, key: string, value: boolean | { approve: boolean; matchCommandLine?: boolean }): void { + let sessionRules = this._sessionAutoApproveRules.get(chatSessionId); + if (!sessionRules) { + sessionRules = {}; + this._sessionAutoApproveRules.set(chatSessionId, sessionRules); + } + sessionRules[key] = value; } - getSessionAutoApproveRules(): Readonly> { - return this._sessionAutoApproveRules; + getSessionAutoApproveRules(chatSessionId: string): Readonly> { + return this._sessionAutoApproveRules.get(chatSessionId) ?? {}; } } diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandLineAutoApprover.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandLineAutoApprover.ts index 381937619e8..61d5cda35f7 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandLineAutoApprover.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandLineAutoApprover.ts @@ -85,7 +85,7 @@ export class CommandLineAutoApprover extends Disposable { this._denyListCommandLineRules = denyListCommandLineRules; } - isCommandAutoApproved(command: string, shell: string, os: OperatingSystem): ICommandApprovalResultWithReason { + isCommandAutoApproved(command: string, shell: string, os: OperatingSystem, chatSessionId?: string): ICommandApprovalResultWithReason { // Check if the command has a transient environment variable assignment prefix which we // always deny for now as it can easily lead to execute other commands if (transientEnvVarRegex.test(command)) { @@ -107,7 +107,7 @@ export class CommandLineAutoApprover extends Disposable { } // Check session allow rules (session deny rules can't exist) - for (const rule of this._getSessionRules().allowListRules) { + for (const rule of this._getSessionRules(chatSessionId).allowListRules) { if (this._commandMatchesRule(rule, command, shell, os)) { return { result: 'approved', @@ -137,7 +137,7 @@ export class CommandLineAutoApprover extends Disposable { }; } - isCommandLineAutoApproved(commandLine: string): ICommandApprovalResultWithReason { + isCommandLineAutoApproved(commandLine: string, chatSessionId?: string): ICommandApprovalResultWithReason { // Check the config deny list first to see if this command line requires explicit approval for (const rule of this._denyListCommandLineRules) { if (rule.regex.test(commandLine)) { @@ -150,7 +150,7 @@ export class CommandLineAutoApprover extends Disposable { } // Check session allow list (session deny rules can't exist) - for (const rule of this._getSessionRules().allowListCommandLineRules) { + for (const rule of this._getSessionRules(chatSessionId).allowListCommandLineRules) { if (rule.regex.test(commandLine)) { return { result: 'approved', @@ -176,7 +176,7 @@ export class CommandLineAutoApprover extends Disposable { }; } - private _getSessionRules(): { + private _getSessionRules(chatSessionId?: string): { denyListRules: IAutoApproveRule[]; allowListRules: IAutoApproveRule[]; allowListCommandLineRules: IAutoApproveRule[]; @@ -187,7 +187,11 @@ export class CommandLineAutoApprover extends Disposable { const allowListCommandLineRules: IAutoApproveRule[] = []; const denyListCommandLineRules: IAutoApproveRule[] = []; - const sessionRulesConfig = this._terminalChatService.getSessionAutoApproveRules(); + if (!chatSessionId) { + return { denyListRules, allowListRules, allowListCommandLineRules, denyListCommandLineRules }; + } + + const sessionRulesConfig = this._terminalChatService.getSessionAutoApproveRules(chatSessionId); for (const [key, value] of Object.entries(sessionRulesConfig)) { if (typeof value === 'boolean') { const { regex, regexCaseInsensitive } = this._convertAutoApproveEntryToRegex(key); diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts index 77db325f68f..86a45af8248 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts @@ -87,8 +87,8 @@ export class CommandLineAutoApproveAnalyzer extends Disposable implements IComma }; } - const subCommandResults = subCommands.map(e => this._commandLineAutoApprover.isCommandAutoApproved(e, options.shell, options.os)); - const commandLineResult = this._commandLineAutoApprover.isCommandLineAutoApproved(options.commandLine); + const subCommandResults = subCommands.map(e => this._commandLineAutoApprover.isCommandAutoApproved(e, options.shell, options.os, options.chatSessionId)); + const commandLineResult = this._commandLineAutoApprover.isCommandLineAutoApproved(options.commandLine, options.chatSessionId); const autoApproveReasons: string[] = [ ...subCommandResults.map(e => e.reason), commandLineResult.reason, From 47de3f5d9d106c550100dabbb94724cf80ff1a1e Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 18 Dec 2025 12:54:35 -0800 Subject: [PATCH 11/76] Fix test expectations --- .../runInTerminalTool.test.ts | 184 +++++++++++++----- 1 file changed, 138 insertions(+), 46 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts index 233f5eb5d95..353e8e5b3fd 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts @@ -10,7 +10,7 @@ import { Emitter } from '../../../../../../base/common/event.js'; import { Schemas } from '../../../../../../base/common/network.js'; import { isLinux, isWindows, OperatingSystem } from '../../../../../../base/common/platform.js'; import { count } from '../../../../../../base/common/strings.js'; -import type { SingleOrMany } from '../../../../../../base/common/types.js'; +import { hasKey, type SingleOrMany } from '../../../../../../base/common/types.js'; import { URI } from '../../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { ITreeSitterLibraryService } from '../../../../../../editor/common/services/treeSitter/treeSitterLibraryService.js'; @@ -82,6 +82,9 @@ suite('RunInTerminalTool', () => { fileService: () => fileService, }, store); + instantiationService.stub(IChatService, { + onDidDisposeSession: chatServiceDisposeEmitter.event + }); instantiationService.stub(ITerminalChatService, store.add(instantiationService.createInstance(TerminalChatService))); instantiationService.stub(IWorkspaceContextService, workspaceContextService); instantiationService.stub(IHistoryService, { @@ -101,9 +104,6 @@ suite('RunInTerminalTool', () => { onDidDisposeInstance: terminalServiceDisposeEmitter.event, setNextCommandId: async () => { } }); - instantiationService.stub(IChatService, { - onDidDisposeSession: chatServiceDisposeEmitter.event - }); instantiationService.stub(ITerminalProfileResolverService, { getDefaultProfile: async () => ({ path: 'bash' } as ITerminalProfile) }); @@ -475,7 +475,9 @@ suite('RunInTerminalTool', () => { suite('prepareToolInvocation - custom actions for dropdown', () => { - function assertDropdownActions(result: IPreparedToolInvocation | undefined, items: ({ subCommand: SingleOrMany } | 'commandLine' | '---' | 'configure' | 'sessionApproval')[]) { + type ActionItemType = { subCommand: SingleOrMany; scope: 'session' | 'workspace' | 'user' } | { commandLine: true; scope: 'session' | 'workspace' | 'user' } | '---' | 'configure' | 'sessionApproval'; + + function assertDropdownActions(result: IPreparedToolInvocation | undefined, items: ActionItemType[]) { const actions = result?.confirmationMessages?.terminalCustomActions!; ok(actions, 'Expected custom actions to be defined'); @@ -493,16 +495,21 @@ suite('RunInTerminalTool', () => { } else if (item === 'sessionApproval') { strictEqual(action.label, 'Allow All Commands in this Session'); strictEqual(action.data.type, 'sessionApproval'); - } else if (item === 'commandLine') { - strictEqual(action.label, 'Always Allow Exact Command Line'); + } else if (hasKey(item, { commandLine: true })) { + const expectedLabel = item.scope === 'session' ? 'Allow Exact Command Line in this Session' + : item.scope === 'workspace' ? 'Allow Exact Command Line in this Workspace' + : 'Always Allow Exact Command Line'; + strictEqual(action.label, expectedLabel); strictEqual(action.data.type, 'newRule'); ok(!Array.isArray(action.data.rule), 'Expected rule to be an object'); } else { - if (Array.isArray(item.subCommand)) { - strictEqual(action.label, `Always Allow Commands: ${item.subCommand.join(', ')}`); - } else { - strictEqual(action.label, `Always Allow Command: ${item.subCommand}`); - } + const subCommandLabel = Array.isArray(item.subCommand) + ? `Commands ${item.subCommand.map(e => `\`${e} \u2026\``).join(', ')}` + : `\`${item.subCommand} \u2026\``; + const expectedLabel = item.scope === 'session' ? `Allow ${subCommandLabel} in this Session` + : item.scope === 'workspace' ? `Allow ${subCommandLabel} in this Workspace` + : `Always Allow ${subCommandLabel}`; + strictEqual(action.label, expectedLabel); strictEqual(action.data.type, 'newRule'); ok(Array.isArray(action.data.rule), 'Expected rule to be an array'); } @@ -521,8 +528,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result, 'Run `bash` command?'); assertDropdownActions(result, [ - { subCommand: 'npm run build' }, - 'commandLine', + { subCommand: 'npm run build', scope: 'session' }, + { subCommand: 'npm run build', scope: 'workspace' }, + { subCommand: 'npm run build', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -538,7 +550,10 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: 'foo' }, + { subCommand: 'foo', scope: 'session' }, + { subCommand: 'foo', scope: 'workspace' }, + { subCommand: 'foo', scope: 'user' }, + '---', '---', 'sessionApproval', '---', @@ -583,8 +598,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result, 'Run `bash` command?'); assertDropdownActions(result, [ - { subCommand: ['npm install', 'npm run build'] }, - 'commandLine', + { subCommand: ['npm install', 'npm run build'], scope: 'session' }, + { subCommand: ['npm install', 'npm run build'], scope: 'workspace' }, + { subCommand: ['npm install', 'npm run build'], scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -603,8 +623,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result, 'Run `bash` command?'); assertDropdownActions(result, [ - { subCommand: 'foo' }, - 'commandLine', + { subCommand: 'foo', scope: 'session' }, + { subCommand: 'foo', scope: 'workspace' }, + { subCommand: 'foo', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -637,8 +662,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result, 'Run `bash` command?'); assertDropdownActions(result, [ - { subCommand: ['foo', 'bar'] }, - 'commandLine', + { subCommand: ['foo', 'bar'], scope: 'session' }, + { subCommand: ['foo', 'bar'], scope: 'workspace' }, + { subCommand: ['foo', 'bar'], scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -654,8 +684,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: 'git status' }, - 'commandLine', + { subCommand: 'git status', scope: 'session' }, + { subCommand: 'git status', scope: 'workspace' }, + { subCommand: 'git status', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -671,8 +706,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: 'npm test' }, - 'commandLine', + { subCommand: 'npm test', scope: 'session' }, + { subCommand: 'npm test', scope: 'workspace' }, + { subCommand: 'npm test', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -688,8 +728,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: 'npm run build' }, - 'commandLine', + { subCommand: 'npm run build', scope: 'session' }, + { subCommand: 'npm run build', scope: 'workspace' }, + { subCommand: 'npm run build', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -705,8 +750,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: 'yarn run test' }, - 'commandLine', + { subCommand: 'yarn run test', scope: 'session' }, + { subCommand: 'yarn run test', scope: 'workspace' }, + { subCommand: 'yarn run test', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -722,8 +772,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: 'foo' }, - 'commandLine', + { subCommand: 'foo', scope: 'session' }, + { subCommand: 'foo', scope: 'workspace' }, + { subCommand: 'foo', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -739,8 +794,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: 'npm run abc' }, - 'commandLine', + { subCommand: 'npm run abc', scope: 'session' }, + { subCommand: 'npm run abc', scope: 'workspace' }, + { subCommand: 'npm run abc', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -756,8 +816,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: ['npm run build', 'git status'] }, - 'commandLine', + { subCommand: ['npm run build', 'git status'], scope: 'session' }, + { subCommand: ['npm run build', 'git status'], scope: 'workspace' }, + { subCommand: ['npm run build', 'git status'], scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -773,8 +838,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: ['git push', 'echo'] }, - 'commandLine', + { subCommand: ['git push', 'echo'], scope: 'session' }, + { subCommand: ['git push', 'echo'], scope: 'workspace' }, + { subCommand: ['git push', 'echo'], scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -790,8 +860,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: ['git status', 'git log'] }, - 'commandLine', + { subCommand: ['git status', 'git log'], scope: 'session' }, + { subCommand: ['git status', 'git log'], scope: 'workspace' }, + { subCommand: ['git status', 'git log'], scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -807,8 +882,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: 'foo' }, - 'commandLine', + { subCommand: 'foo', scope: 'session' }, + { subCommand: 'foo', scope: 'workspace' }, + { subCommand: 'foo', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -838,8 +918,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: 'npm test' }, - 'commandLine', + { subCommand: 'npm test', scope: 'session' }, + { subCommand: 'npm test', scope: 'workspace' }, + { subCommand: 'npm test', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -855,8 +940,13 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - { subCommand: 'foo' }, - 'commandLine', + { subCommand: 'foo', scope: 'session' }, + { subCommand: 'foo', scope: 'workspace' }, + { subCommand: 'foo', scope: 'user' }, + '---', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', @@ -872,7 +962,9 @@ suite('RunInTerminalTool', () => { assertConfirmationRequired(result); assertDropdownActions(result, [ - 'commandLine', + { commandLine: true, scope: 'session' }, + { commandLine: true, scope: 'workspace' }, + { commandLine: true, scope: 'user' }, '---', 'sessionApproval', '---', From e7d9448e24a2ed25f4291054366a44118a72642b Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Sun, 28 Dec 2025 01:00:22 -0800 Subject: [PATCH 12/76] Avoid setting state-related ARIA attributes on separators --- src/vs/base/browser/ui/actionbar/actionViewItems.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/vs/base/browser/ui/actionbar/actionViewItems.ts b/src/vs/base/browser/ui/actionbar/actionViewItems.ts index ea705bcaa68..4bc0f0a15d3 100644 --- a/src/vs/base/browser/ui/actionbar/actionViewItems.ts +++ b/src/vs/base/browser/ui/actionbar/actionViewItems.ts @@ -402,6 +402,10 @@ export class ActionViewItem extends BaseActionViewItem { } protected override updateEnabled(): void { + if (this.action.id === Separator.ID) { + return; + } + if (this.action.enabled) { if (this.label) { this.label.removeAttribute('aria-disabled'); @@ -427,6 +431,10 @@ export class ActionViewItem extends BaseActionViewItem { } protected override updateChecked(): void { + if (this.action.id === Separator.ID) { + return; + } + if (this.label) { if (this.action.checked !== undefined) { this.label.classList.toggle('checked', this.action.checked); From a8390ddc80ff27b8b5a9f597024325cee642a9c1 Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Sun, 28 Dec 2025 21:14:29 -0800 Subject: [PATCH 13/76] Add view range padding for cursor when cursorSurroundingLines set set --- src/vs/editor/browser/coreCommands.ts | 4 ++- src/vs/editor/common/viewModel.ts | 1 + .../editor/common/viewModel/viewModelImpl.ts | 26 +++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/vs/editor/browser/coreCommands.ts b/src/vs/editor/browser/coreCommands.ts index a1d6137f875..9331eea48ff 100644 --- a/src/vs/editor/browser/coreCommands.ts +++ b/src/vs/editor/browser/coreCommands.ts @@ -1358,11 +1358,13 @@ export namespace CoreNavigationCommands { if (args.revealCursor) { // must ensure cursor is in new visible range const desiredVisibleViewRange = viewModel.getCompletelyVisibleViewRangeAtScrollTop(desiredScrollTop); + const paddedRange = viewModel.getViewRangeWithCursorPadding(desiredVisibleViewRange); + viewModel.setCursorStates( source, CursorChangeReason.Explicit, [ - CursorMoveCommands.findPositionInViewportIfOutside(viewModel, viewModel.getPrimaryCursorState(), desiredVisibleViewRange, args.select) + CursorMoveCommands.findPositionInViewportIfOutside(viewModel, viewModel.getPrimaryCursorState(), paddedRange, args.select) ] ); } diff --git a/src/vs/editor/common/viewModel.ts b/src/vs/editor/common/viewModel.ts index 646d41f0f98..4c1aeff7482 100644 --- a/src/vs/editor/common/viewModel.ts +++ b/src/vs/editor/common/viewModel.ts @@ -57,6 +57,7 @@ export interface IViewModel extends ICursorSimpleModel, ISimpleModel { getMinimapLinesRenderingData(startLineNumber: number, endLineNumber: number, needed: boolean[]): MinimapLinesRenderingData; getCompletelyVisibleViewRange(): Range; getCompletelyVisibleViewRangeAtScrollTop(scrollTop: number): Range; + getViewRangeWithCursorPadding(viewRange: Range): Range; getHiddenAreas(): Range[]; diff --git a/src/vs/editor/common/viewModel/viewModelImpl.ts b/src/vs/editor/common/viewModel/viewModelImpl.ts index 284bbc487a4..3fab2ddee2e 100644 --- a/src/vs/editor/common/viewModel/viewModelImpl.ts +++ b/src/vs/editor/common/viewModel/viewModelImpl.ts @@ -693,6 +693,32 @@ export class ViewModel extends Disposable implements IViewModel { ); } + /** + * Applies `cursorSurroundingLines` and `stickyScroll` padding to the given view range. + */ + public getViewRangeWithCursorPadding(viewRange: Range): Range { + const options = this._configuration.options; + const cursorSurroundingLines = options.get(EditorOption.cursorSurroundingLines); + const stickyScroll = options.get(EditorOption.stickyScroll); + + let { startLineNumber, endLineNumber } = viewRange; + const padding = Math.min( + Math.max(cursorSurroundingLines, stickyScroll.enabled ? stickyScroll.maxLineCount : 0), + Math.floor((endLineNumber - startLineNumber + 1) / 2)); + + startLineNumber += padding; + endLineNumber -= Math.max(0, padding - 1); + + if (padding === 0 || startLineNumber > endLineNumber) { + return viewRange; + } + + return new Range( + startLineNumber, this.getLineMinColumn(startLineNumber), + endLineNumber, this.getLineMaxColumn(endLineNumber) + ); + } + public saveState(): IViewState { const compatViewState = this.viewLayout.saveState(); From 55076dfa94d31e4e1e6ff060588a9c7f34ae5cff Mon Sep 17 00:00:00 2001 From: mizdra Date: Tue, 30 Dec 2025 16:54:05 +0900 Subject: [PATCH 14/76] fix tests that failed in environments where `XDG_DATA_HOME` is set --- .../history/test/common/history.test.ts | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/history/test/common/history.test.ts b/src/vs/workbench/contrib/terminalContrib/history/test/common/history.test.ts index 784a72d3f13..be2a8f19ff5 100644 --- a/src/vs/workbench/contrib/terminalContrib/history/test/common/history.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/history/test/common/history.test.ts @@ -484,10 +484,11 @@ suite('Terminal history', () => { if (!isWindows) { suite('local', () => { - let originalEnvValues: { HOME: string | undefined }; + let originalEnvValues: { HOME: string | undefined; XDG_DATA_HOME: string | undefined }; setup(() => { - originalEnvValues = { HOME: env['HOME'] }; + originalEnvValues = { HOME: env['HOME'], XDG_DATA_HOME: env['XDG_DATA_HOME'] }; env['HOME'] = '/home/user'; + delete env['XDG_DATA_HOME']; remoteConnection = { remoteAuthority: 'some-remote' }; fileScheme = Schemas.vscodeRemote; filePath = '/home/user/.local/share/fish/fish_history'; @@ -498,6 +499,11 @@ suite('Terminal history', () => { } else { env['HOME'] = originalEnvValues['HOME']; } + if (originalEnvValues['XDG_DATA_HOME'] === undefined) { + delete env['XDG_DATA_HOME']; + } else { + env['XDG_DATA_HOME'] = originalEnvValues['XDG_DATA_HOME']; + } }); test('current OS', async () => { filePath = '/home/user/.local/share/fish/fish_history'; @@ -528,10 +534,11 @@ suite('Terminal history', () => { }); } suite('remote', () => { - let originalEnvValues: { HOME: string | undefined }; + let originalEnvValues: { HOME: string | undefined; XDG_DATA_HOME: string | undefined }; setup(() => { - originalEnvValues = { HOME: env['HOME'] }; + originalEnvValues = { HOME: env['HOME'], XDG_DATA_HOME: env['XDG_DATA_HOME'] }; env['HOME'] = '/home/user'; + delete env['XDG_DATA_HOME']; remoteConnection = { remoteAuthority: 'some-remote' }; fileScheme = Schemas.vscodeRemote; filePath = '/home/user/.local/share/fish/fish_history'; @@ -542,6 +549,11 @@ suite('Terminal history', () => { } else { env['HOME'] = originalEnvValues['HOME']; } + if (originalEnvValues['XDG_DATA_HOME'] === undefined) { + delete env['XDG_DATA_HOME']; + } else { + env['XDG_DATA_HOME'] = originalEnvValues['XDG_DATA_HOME']; + } }); test('Windows', async () => { remoteEnvironment = { os: OperatingSystem.Windows }; From fc6010a9f0205ba87621ce7b28ee6eeb5575fa2f Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Tue, 30 Dec 2025 04:01:50 -0800 Subject: [PATCH 15/76] PR feedback --- src/vs/base/browser/ui/actionbar/actionViewItems.ts | 8 -------- src/vs/base/common/actions.ts | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/vs/base/browser/ui/actionbar/actionViewItems.ts b/src/vs/base/browser/ui/actionbar/actionViewItems.ts index 4bc0f0a15d3..ea705bcaa68 100644 --- a/src/vs/base/browser/ui/actionbar/actionViewItems.ts +++ b/src/vs/base/browser/ui/actionbar/actionViewItems.ts @@ -402,10 +402,6 @@ export class ActionViewItem extends BaseActionViewItem { } protected override updateEnabled(): void { - if (this.action.id === Separator.ID) { - return; - } - if (this.action.enabled) { if (this.label) { this.label.removeAttribute('aria-disabled'); @@ -431,10 +427,6 @@ export class ActionViewItem extends BaseActionViewItem { } protected override updateChecked(): void { - if (this.action.id === Separator.ID) { - return; - } - if (this.label) { if (this.action.checked !== undefined) { this.label.classList.toggle('checked', this.action.checked); diff --git a/src/vs/base/common/actions.ts b/src/vs/base/common/actions.ts index 9660e763095..6d3e3f2b3db 100644 --- a/src/vs/base/common/actions.ts +++ b/src/vs/base/common/actions.ts @@ -228,7 +228,7 @@ export class Separator implements IAction { readonly tooltip: string = ''; readonly class: string = 'separator'; readonly enabled: boolean = false; - readonly checked: boolean = false; + readonly checked: undefined = undefined; async run() { } } From b1b5c1e6811411a31bb8fb8bd3708c4cbf673667 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 31 Dec 2025 07:11:50 -0800 Subject: [PATCH 16/76] Auto approve npm scripts by default Fixes #285509 --- ...commandLineNpmScriptAutoApproveAnalyzer.ts | 265 ++++++++++++++ .../browser/tools/runInTerminalTool.ts | 2 + .../terminalChatAgentToolsConfiguration.ts | 8 + ...ndLineNpmScriptAutoApproveAnalyzer.test.ts | 329 ++++++++++++++++++ 4 files changed, 604 insertions(+) create mode 100644 src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineNpmScriptAutoApproveAnalyzer.ts create mode 100644 src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineNpmScriptAutoApproveAnalyzer.test.ts diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineNpmScriptAutoApproveAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineNpmScriptAutoApproveAnalyzer.ts new file mode 100644 index 00000000000..6b913ce3716 --- /dev/null +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineNpmScriptAutoApproveAnalyzer.ts @@ -0,0 +1,265 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { visit, type JSONVisitor } from '../../../../../../../base/common/json.js'; +import { Disposable } from '../../../../../../../base/common/lifecycle.js'; +import { URI } from '../../../../../../../base/common/uri.js'; +import { IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js'; +import { IFileService } from '../../../../../../../platform/files/common/files.js'; +import { IWorkspaceContextService } from '../../../../../../../platform/workspace/common/workspace.js'; +import { TerminalChatAgentToolsSettingId } from '../../../common/terminalChatAgentToolsConfiguration.js'; +import type { TreeSitterCommandParser } from '../../treeSitterCommandParser.js'; +import type { ICommandLineAnalyzer, ICommandLineAnalyzerOptions, ICommandLineAnalyzerResult } from './commandLineAnalyzer.js'; +import { MarkdownString } from '../../../../../../../base/common/htmlContent.js'; +import { localize } from '../../../../../../../nls.js'; +import { IStorageService, StorageScope } from '../../../../../../../platform/storage/common/storage.js'; +import { TerminalToolConfirmationStorageKeys } from '../../../../../chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.js'; + +/** + * Regex patterns to match npm/yarn/pnpm run commands and extract the script name. + * Captures the script name in group 1. + */ +const npmRunPatterns = [ + // npm run