Skip to content

Commit d423b24

Browse files
TyriarTyriar
committed
Handle things like npm test and npm run build specially
Fixes microsoft#261126
1 parent eb649d0 commit d423b24

File tree

3 files changed

+257
-18
lines changed

3 files changed

+257
-18
lines changed

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

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -892,25 +892,41 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
892892
return autoApproveResult.subCommandResults[index].result !== 'approved';
893893
});
894894

895-
const subCommandsFirstWordOnly = Array.from(new Set(unapprovedSubCommands.map(command => command.split(' ')[0])));
896-
let subCommandLabel: string;
897-
let subCommandTooltip: string;
898-
if (subCommandsFirstWordOnly.length === 1) {
899-
subCommandLabel = localize('autoApprove.baseCommandSingle', 'Always Allow Command: {0}', subCommandsFirstWordOnly[0]);
900-
subCommandTooltip = localize('autoApprove.baseCommandSingleTooltip', 'Always allow command starting with `{0}` to run without confirmation', subCommandsFirstWordOnly[0]);
901-
} else {
902-
const commandSeparated = subCommandsFirstWordOnly.join(', ');
903-
subCommandLabel = localize('autoApprove.baseCommand', 'Always Allow Commands: {0}', commandSeparated);
904-
subCommandTooltip = localize('autoApprove.baseCommandTooltip', 'Always allow commands starting with `{0}` to run without confirmation', commandSeparated);
905-
}
895+
// For each unapproved sub-command (within the overall command line), decide whether to
896+
// suggest just the commnad or sub-command (with that sub-command line) to always allow.
897+
const commandsWithSubcommands = new Set(['git', 'npm', 'yarn', 'docker', 'kubectl', 'cargo', 'dotnet', 'mvn', 'gradle']);
898+
const commandsWithSubSubCommands = new Set(['npm run', 'yarn run']);
899+
const subCommandsToSuggest = Array.from(new Set(unapprovedSubCommands.map(command => {
900+
const parts = command.trim().split(/\s+/);
901+
const baseCommand = parts[0].toLowerCase();
902+
903+
if (commandsWithSubSubCommands.has(baseCommand) && parts.length >= 3) {
904+
return `${parts[0]} ${parts[1]} ${parts[2]}`;
905+
} else if (commandsWithSubcommands.has(baseCommand) && parts.length >= 2) {
906+
return `${parts[0]} ${parts[1]}`;
907+
} else {
908+
return parts[0];
909+
}
910+
})));
911+
912+
if (subCommandsToSuggest.length > 0) {
913+
let subCommandLabel: string;
914+
let subCommandTooltip: string;
915+
if (subCommandsToSuggest.length === 1) {
916+
subCommandLabel = localize('autoApprove.baseCommandSingle', 'Always Allow Command: {0}', subCommandsToSuggest[0]);
917+
subCommandTooltip = localize('autoApprove.baseCommandSingleTooltip', 'Always allow command starting with `{0}` to run without confirmation', subCommandsToSuggest[0]);
918+
} else {
919+
const commandSeparated = subCommandsToSuggest.join(', ');
920+
subCommandLabel = localize('autoApprove.baseCommand', 'Always Allow Commands: {0}', commandSeparated);
921+
subCommandTooltip = localize('autoApprove.baseCommandTooltip', 'Always allow commands starting with `{0}` to run without confirmation', commandSeparated);
922+
}
906923

907-
if (unapprovedSubCommands.length > 0) {
908924
actions.push({
909925
label: subCommandLabel,
910926
tooltip: subCommandTooltip,
911927
data: {
912928
type: 'newRule',
913-
rule: subCommandsFirstWordOnly.map(key => ({
929+
rule: subCommandsToSuggest.map(key => ({
914930
key,
915931
value: true
916932
}))
@@ -920,12 +936,13 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
920936

921937
// Allow exact command line, don't do this if it's just the first sub-command's first
922938
// word
923-
if (subCommandsFirstWordOnly[0] !== commandLine) {
939+
const firstSubcommandFirstWord = unapprovedSubCommands.length > 0 ? unapprovedSubCommands[0].split(' ')[0] : '';
940+
if (firstSubcommandFirstWord !== commandLine) {
924941
const truncatedCommandLine = commandLine.length > 40 ? commandLine.substring(0, 40) + '\u2026' : commandLine;
925942
actions.push({
926943
// Add an extra & since it's treated as a mnemonic
927-
label: localize('autoApprove.exactCommand', 'Always Allow Full Command Line: {0}', truncatedCommandLine.replaceAll('&&', '&&&')),
928-
tooltip: localize('autoApprove.exactCommandTooltip', 'Always allow this exact command to run without confirmation'),
944+
label: localize('autoApprove.exactCommand', 'Always Allow Exact Command Line: {0}', truncatedCommandLine.replaceAll('&&', '&&&')),
945+
tooltip: localize('autoApprove.exactCommandTooltip', 'Always allow this exact command line to run without confirmation'),
929946
data: {
930947
type: 'newRule',
931948
rule: {

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export const terminalChatAgentToolsConfiguration: IStringDictionary<IConfigurati
8787
]
8888
},
8989
default: {
90-
// Safe and common readonly commands (automatically approved)
90+
// Safe and common readonly commands
9191
cd: true,
9292
echo: true,
9393
ls: true,
@@ -125,6 +125,27 @@ export const terminalChatAgentToolsConfiguration: IStringDictionary<IConfigurati
125125
jq: true,
126126
sleep: true,
127127
'Start-Sleep': true,
128+
129+
// Safe and common sub-commands
130+
'git status': true,
131+
'git log': true,
132+
'git show': true,
133+
'git diff': true,
134+
135+
// Common npm/yarn run commands that are typically safe
136+
'npm run build': true,
137+
'npm run test': true,
138+
'npm run dev': true,
139+
'npm run start': true,
140+
'npm run lint': true,
141+
'npm run watch': true,
142+
'yarn run build': true,
143+
'yarn run test': true,
144+
'yarn run dev': true,
145+
'yarn run start': true,
146+
'yarn run lint': true,
147+
'yarn run watch': true,
148+
128149
// While these PowerShell verbs can have side effects, they are generally innocuous (eg.
129150
// updating OS-level file access info) and and often have prompts if they're more
130151
// involved (eg. Get-Credential)

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

Lines changed: 202 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ suite('RunInTerminalTool', () => {
384384
strictEqual(customActions.length, 4);
385385

386386
ok(!isSeparator(customActions[0]));
387-
strictEqual(customActions[0].label, 'Always Allow Command: npm');
387+
strictEqual(customActions[0].label, 'Always Allow Command: npm install, npm run build');
388388
strictEqual(customActions[0].data.type, 'newRule');
389389

390390
ok(!isSeparator(customActions[1]));
@@ -475,6 +475,207 @@ suite('RunInTerminalTool', () => {
475475
strictEqual(customActions[3].data.type, 'configure');
476476
});
477477

478+
test('should suggest subcommand for git commands', async () => {
479+
const result = await executeToolTest({
480+
command: 'git status',
481+
explanation: 'Check git status'
482+
});
483+
484+
assertConfirmationRequired(result);
485+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
486+
487+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
488+
strictEqual(customActions.length, 4);
489+
490+
ok(!isSeparator(customActions[0]));
491+
strictEqual(customActions[0].label, 'Always Allow Command: git status');
492+
strictEqual(customActions[0].data.type, 'newRule');
493+
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
494+
strictEqual((customActions[0].data.rule as any)[0].key, 'git status');
495+
});
496+
497+
test('should suggest subcommand for npm commands', async () => {
498+
const result = await executeToolTest({
499+
command: 'npm test',
500+
explanation: 'Run npm tests'
501+
});
502+
503+
assertConfirmationRequired(result);
504+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
505+
506+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
507+
strictEqual(customActions.length, 4);
508+
509+
ok(!isSeparator(customActions[0]));
510+
strictEqual(customActions[0].label, 'Always Allow Command: npm test');
511+
strictEqual(customActions[0].data.type, 'newRule');
512+
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
513+
strictEqual((customActions[0].data.rule as any)[0].key, 'npm test');
514+
});
515+
516+
test('should suggest 3-part subcommand for npm run commands', async () => {
517+
const result = await executeToolTest({
518+
command: 'npm run build',
519+
explanation: 'Run build script'
520+
});
521+
522+
assertConfirmationRequired(result);
523+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
524+
525+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
526+
strictEqual(customActions.length, 4);
527+
528+
ok(!isSeparator(customActions[0]));
529+
strictEqual(customActions[0].label, 'Always Allow Command: npm run build');
530+
strictEqual(customActions[0].data.type, 'newRule');
531+
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
532+
strictEqual((customActions[0].data.rule as any)[0].key, 'npm run build');
533+
});
534+
535+
test('should suggest 3-part subcommand for yarn run commands', async () => {
536+
const result = await executeToolTest({
537+
command: 'yarn run test',
538+
explanation: 'Run test script'
539+
});
540+
541+
assertConfirmationRequired(result);
542+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
543+
544+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
545+
strictEqual(customActions.length, 4);
546+
547+
ok(!isSeparator(customActions[0]));
548+
strictEqual(customActions[0].label, 'Always Allow Command: yarn run test');
549+
strictEqual(customActions[0].data.type, 'newRule');
550+
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
551+
strictEqual((customActions[0].data.rule as any)[0].key, 'yarn run test');
552+
});
553+
554+
test('should handle mixed npm run and other commands', async () => {
555+
const result = await executeToolTest({
556+
command: 'npm run build && git status',
557+
explanation: 'Build and check status'
558+
});
559+
560+
assertConfirmationRequired(result);
561+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
562+
563+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
564+
strictEqual(customActions.length, 4);
565+
566+
ok(!isSeparator(customActions[0]));
567+
strictEqual(customActions[0].label, 'Always Allow Commands: npm run build, git status');
568+
strictEqual(customActions[0].data.type, 'newRule');
569+
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
570+
const rules = customActions[0].data.rule as any;
571+
strictEqual(rules.length, 2);
572+
strictEqual(rules[0].key, 'npm run build');
573+
strictEqual(rules[1].key, 'git status');
574+
});
575+
576+
test('should suggest mixed subcommands and base commands', async () => {
577+
const result = await executeToolTest({
578+
command: 'git push && echo "done"',
579+
explanation: 'Push and print done'
580+
});
581+
582+
assertConfirmationRequired(result);
583+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
584+
585+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
586+
strictEqual(customActions.length, 4);
587+
588+
ok(!isSeparator(customActions[0]));
589+
strictEqual(customActions[0].label, 'Always Allow Commands: git push, echo');
590+
strictEqual(customActions[0].data.type, 'newRule');
591+
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
592+
const rules = customActions[0].data.rule as any;
593+
strictEqual(rules.length, 2);
594+
strictEqual(rules[0].key, 'git push');
595+
strictEqual(rules[1].key, 'echo');
596+
});
597+
598+
test('should suggest subcommands for multiple git commands', async () => {
599+
const result = await executeToolTest({
600+
command: 'git status && git log --oneline',
601+
explanation: 'Check status and log'
602+
});
603+
604+
assertConfirmationRequired(result);
605+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
606+
607+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
608+
strictEqual(customActions.length, 4);
609+
610+
ok(!isSeparator(customActions[0]));
611+
strictEqual(customActions[0].label, 'Always Allow Commands: git status, git log');
612+
strictEqual(customActions[0].data.type, 'newRule');
613+
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
614+
const rules = customActions[0].data.rule as any;
615+
strictEqual(rules.length, 2);
616+
strictEqual(rules[0].key, 'git status');
617+
strictEqual(rules[1].key, 'git log');
618+
});
619+
620+
test('should suggest base command for non-subcommand tools', async () => {
621+
const result = await executeToolTest({
622+
command: 'curl https://example.com',
623+
explanation: 'Download from example.com'
624+
});
625+
626+
assertConfirmationRequired(result);
627+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
628+
629+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
630+
strictEqual(customActions.length, 4);
631+
632+
ok(!isSeparator(customActions[0]));
633+
strictEqual(customActions[0].label, 'Always Allow Command: curl');
634+
strictEqual(customActions[0].data.type, 'newRule');
635+
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
636+
strictEqual((customActions[0].data.rule as any)[0].key, 'curl');
637+
});
638+
639+
test('should handle single word commands from subcommand-aware tools', async () => {
640+
const result = await executeToolTest({
641+
command: 'git',
642+
explanation: 'Run git command'
643+
});
644+
645+
assertConfirmationRequired(result);
646+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
647+
648+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
649+
strictEqual(customActions.length, 3); // No full command line suggestion for single word
650+
651+
ok(!isSeparator(customActions[0]));
652+
strictEqual(customActions[0].label, 'Always Allow Command: git');
653+
strictEqual(customActions[0].data.type, 'newRule');
654+
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
655+
strictEqual((customActions[0].data.rule as any)[0].key, 'git');
656+
});
657+
658+
test('should deduplicate identical subcommand suggestions', async () => {
659+
const result = await executeToolTest({
660+
command: 'npm test && npm test --verbose',
661+
explanation: 'Run tests twice'
662+
});
663+
664+
assertConfirmationRequired(result);
665+
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
666+
667+
const customActions = result!.confirmationMessages!.terminalCustomActions!;
668+
strictEqual(customActions.length, 4);
669+
670+
ok(!isSeparator(customActions[0]));
671+
strictEqual(customActions[0].label, 'Always Allow Command: npm test');
672+
strictEqual(customActions[0].data.type, 'newRule');
673+
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
674+
const rules = customActions[0].data.rule as any;
675+
strictEqual(rules.length, 1); // Should be deduplicated
676+
strictEqual(rules[0].key, 'npm test');
677+
});
678+
478679
});
479680

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

0 commit comments

Comments
 (0)