diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts index 6e2c39552619f..983f3472780c5 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts @@ -105,27 +105,50 @@ 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): number | undefined => { + for (let i = startIndex; i < parts.length; i++) { + if (!parts[i].startsWith('-')) { + return 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. + // + // 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(); - const baseSubCommand = parts.length > 1 ? `${parts[0]} ${parts[1]}`.toLowerCase() : ''; // Security check: Never suggest auto-approval for dangerous interpreter commands if (neverAutoApproveCommands.has(baseCommand)) { return undefined; } - if (commandsWithSubSubCommands.has(baseSubCommand)) { - if (parts.length >= 3 && !parts[2].startsWith('-')) { - return `${parts[0]} ${parts[1]} ${parts[2]}`; - } - return undefined; - } else if (commandsWithSubcommands.has(baseCommand)) { - if (parts.length >= 2 && !parts[1].startsWith('-')) { - return `${parts[0]} ${parts[1]}`; + 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 { 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..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 @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { ok, strictEqual } from 'assert'; -import { TRUNCATION_MESSAGE, dedupeRules, isPowerShell, sanitizeTerminalOutput, truncateOutputKeepingTail } from '../../browser/runInTerminalHelpers.js'; +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,175 @@ suite('sanitizeTerminalOutput', () => { ok(result.endsWith('line')); }); }); + +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), + regexCaseInsensitive: new RegExp(escapedText, '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 -DskipIT 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 -DskipIT test')); + ok(subCommandAction, 'Should suggest mvn -DskipIT test approval (including flags)'); + }); + + 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 = { + 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 -X -DskipIT test')); + ok(subCommandAction, 'Should suggest mvn -X -DskipIT test approval with multiple flags'); + }); + + test('should suggest gradle --info 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 --info build')); + ok(subCommandAction, 'Should suggest gradle --info build approval'); + }); + + 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 = { + 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 --silent run test')); + ok(subCommandAction, 'Should suggest npm --silent run test approval (sub-sub-command with flags)'); + }); + + 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 = { + 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 --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', () => { + 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 -DskipIT test') && action.label.includes('gradle --info build') + ); + ok(subCommandAction, 'Should suggest both mvn -DskipIT test and gradle --info 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 -DskipIT test') && action.label.includes('Always Allow Command:')); + strictEqual(subCommandAction, undefined, 'Should not suggest approval for already approved commands'); + }); +});