Skip to content

Commit e130cac

Browse files
authored
Merge pull request #260865 from microsoft/tyriar/260860
Ensure auto allow commands only shows those not already approved
2 parents b0b8781 + f83077a commit e130cac

File tree

2 files changed

+86
-13
lines changed

2 files changed

+86
-13
lines changed

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -887,8 +887,11 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
887887
// wouldn't get auto approved with a new rule
888888
const canCreateAutoApproval = autoApproveResult.subCommandResults.some(e => e.result !== 'denied') || autoApproveResult.commandLineResult.result === 'denied';
889889
if (canCreateAutoApproval) {
890-
// Allow all sub-commands
891-
const subCommandsFirstWordOnly = Array.from(new Set(subCommands.map(command => command.split(' ')[0])));
890+
const unapprovedSubCommands = subCommands.filter((_, index) => {
891+
return autoApproveResult.subCommandResults[index].result !== 'approved';
892+
});
893+
894+
const subCommandsFirstWordOnly = Array.from(new Set(unapprovedSubCommands.map(command => command.split(' ')[0])));
892895
let subCommandLabel: string;
893896
let subCommandTooltip: string;
894897
if (subCommandsFirstWordOnly.length === 1) {
@@ -899,17 +902,20 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
899902
subCommandLabel = localize('autoApprove.baseCommand', 'Always Allow Commands: {0}', commandSeparated);
900903
subCommandTooltip = localize('autoApprove.baseCommandTooltip', 'Always allow commands starting with `{0}` to run without confirmation', commandSeparated);
901904
}
902-
actions.push({
903-
label: subCommandLabel,
904-
tooltip: subCommandTooltip,
905-
data: {
906-
type: 'newRule',
907-
rule: subCommandsFirstWordOnly.map(key => ({
908-
key,
909-
value: true
910-
}))
911-
} satisfies TerminalNewAutoApproveButtonData
912-
});
905+
906+
if (unapprovedSubCommands.length > 0) {
907+
actions.push({
908+
label: subCommandLabel,
909+
tooltip: subCommandTooltip,
910+
data: {
911+
type: 'newRule',
912+
rule: subCommandsFirstWordOnly.map(key => ({
913+
key,
914+
value: true
915+
}))
916+
} satisfies TerminalNewAutoApproveButtonData
917+
});
918+
}
913919

914920
// Allow exact command line, don't do this if it's just the first sub-command's first
915921
// word

src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalTool.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,73 @@ suite('RunInTerminalTool', () => {
377377
strictEqual(customActions[2].data.type, 'configure');
378378
});
379379

380+
test('should not show approved commands in custom actions dropdown', async () => {
381+
setAutoApprove({
382+
head: true // head is approved by default in real scenario
383+
});
384+
385+
const result = await executeToolTest({
386+
command: 'foo | head -20',
387+
explanation: 'Run foo command and show first 20 lines'
388+
});
389+
390+
assertConfirmationRequired(result, 'Run command in terminal');
391+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
392+
393+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
394+
strictEqual(customActions.length, 3, 'Expected 3 custom actions');
395+
396+
strictEqual(customActions[0].label, 'Always Allow Command: foo', 'Should only show \'foo\' since \'head\' is auto-approved');
397+
strictEqual(customActions[0].data.type, 'newRule');
398+
399+
strictEqual(customActions[1].label, 'Always Allow Full Command Line: foo | head -20');
400+
strictEqual(customActions[1].data.type, 'newRule');
401+
402+
strictEqual(customActions[2].label, 'Configure Auto Approve...');
403+
strictEqual(customActions[2].data.type, 'configure');
404+
});
405+
406+
test('should not show any command-specific actions when all sub-commands are approved', async () => {
407+
setAutoApprove({
408+
foo: true,
409+
head: true
410+
});
411+
412+
const result = await executeToolTest({
413+
command: 'foo | head -20',
414+
explanation: 'Run foo command and show first 20 lines'
415+
});
416+
417+
assertAutoApproved(result);
418+
});
419+
420+
test('should handle mixed approved and unapproved commands correctly', async () => {
421+
setAutoApprove({
422+
head: true,
423+
tail: true
424+
});
425+
426+
const result = await executeToolTest({
427+
command: 'foo | head -20 && bar | tail -10',
428+
explanation: 'Run multiple piped commands'
429+
});
430+
431+
assertConfirmationRequired(result, 'Run command in terminal');
432+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
433+
434+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
435+
strictEqual(customActions.length, 3, 'Expected 3 custom actions');
436+
437+
strictEqual(customActions[0].label, 'Always Allow Commands: foo, bar', 'Should only show \'foo, bar\' since \'head\' and \'tail\' are auto-approved');
438+
strictEqual(customActions[0].data.type, 'newRule');
439+
440+
strictEqual(customActions[1].label, 'Always Allow Full Command Line: foo | head -20 &&& bar | tail -10');
441+
strictEqual(customActions[1].data.type, 'newRule');
442+
443+
strictEqual(customActions[2].label, 'Configure Auto Approve...');
444+
strictEqual(customActions[2].data.type, 'configure');
445+
});
446+
380447
});
381448

382449
suite('command re-writing', () => {

0 commit comments

Comments
 (0)