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 1/6] 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 2/6] 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 d36d891f74cb8..fac649fdddd6c 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 fc802aafa1b41..3cb093a23155e 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 3/6] 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 fac649fdddd6c..fcf576e37e90a 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 3cb093a23155e..3ea9cc7e7bfe2 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 4/6] 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 fcf576e37e90a..dc093f7250b52 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 3ea9cc7e7bfe2..a66edaf12c741 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 5/6] 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 a66edaf12c741..142183b262b25 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 ebe0bd76439e8f6112aee9f2e60a135104501c8f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 2 Jan 2026 02:47:46 -0800 Subject: [PATCH 6/6] Explain flag detection in comment --- .../chatAgentTools/browser/runInTerminalHelpers.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts index d5f9d0d763895..983f3472780c5 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts @@ -118,6 +118,10 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str // 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. + // + // This includes support for detecting flags between the commands, so `mvn -DskipIT test a` + // would suggest `mvn -DskipIT test` as that's more useful than only suggesting the exact + // command line. const subCommandsToSuggest = Array.from(new Set(coalesce(unapprovedSubCommands.map(command => { const parts = command.trim().split(/\s+/); const baseCommand = parts[0].toLowerCase();