Skip to content

Commit e8c3cb2

Browse files
joshspicerCopilot
andauthored
release: eligibleForAutoApproval (microsoft#277943)
* Add `eligibleForAutoApproval` (microsoft#277043) * first pass at eligibleForAutoApproval * add policy object * tidy * add default confirmationMessages and prevent globally auto-approving * --amend * do not show the allow button dropdown when menu is empty! * update description * compile test * update test * polish * remove policy for now * polish fix not taking in autoapprove * Add policy configuration for chat.tools.eligibleForAutoApproval (microsoft#277238) * Initial plan * Add policy configuration for chat.tools.eligibleForAutoApproval setting Co-authored-by: joshspicer <[email protected]> * Update policy generation tests for chat.tools.eligibleForAutoApproval Co-authored-by: joshspicer <[email protected]> * restore fixtures * restore test * fix minimumVersion * do not add chat prompt files * rerun --export-policy-data * polish --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: joshspicer <[email protected]> * fix edge case with previously approved tools `chat.tools.eligibleForAutoApproval` (microsoft#277590) fix loophole where already approved tools continues to be approved * Disclaimer when `eligibleForAutoApproval` halts tool call (microsoft#277592) * add disclaimer for isToolEligibleForAutoApproval * no markdown * decrement minimum version update policydata * set allowAutoConfirm for terminal tool (microsoft#277957) runInTerminalTool evaluate isEligibleForAutoApproval * prevent empty dropdown (from f78695d) * spread prepared.confirmationMessages before setting new one * fix fetch tool when prohibited via EligibleForAutoApproval boolean check * improve disclaimer ux make settings link clickable --------- Co-authored-by: Copilot <[email protected]>
1 parent 86f5a62 commit e8c3cb2

File tree

9 files changed

+249
-8
lines changed

9 files changed

+249
-8
lines changed

build/lib/policies/policyData.jsonc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,20 @@
9494
"type": "boolean",
9595
"default": false
9696
},
97+
{
98+
"key": "chat.tools.eligibleForAutoApproval",
99+
"name": "ChatToolsEligibleForAutoApproval",
100+
"category": "InteractiveSession",
101+
"minimumVersion": "1.106",
102+
"localization": {
103+
"description": {
104+
"key": "chat.tools.eligibleForAutoApproval",
105+
"value": "Controls which tools are eligible for automatic approval. Tools set to 'false' will always present a confirmation and will never offer the option to auto-approve. The default behavior (or setting a tool to 'true') may result in the tool offering auto-approval options."
106+
}
107+
},
108+
"type": "object",
109+
"default": {}
110+
},
97111
{
98112
"key": "chat.mcp.access",
99113
"name": "ChatMCP",

src/vs/workbench/contrib/chat/browser/chat.contribution.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,32 @@ configurationRegistry.registerConfiguration({
276276
type: 'boolean',
277277
}
278278
},
279+
[ChatConfiguration.EligibleForAutoApproval]: {
280+
default: {},
281+
markdownDescription: nls.localize('chat.tools.eligibleForAutoApproval', 'Controls which tools are eligible for automatic approval. Tools set to \'false\' will always present a confirmation and will never offer the option to auto-approve. The default behavior (or setting a tool to \'true\') may result in the tool offering auto-approval options.'),
282+
type: 'object',
283+
additionalProperties: {
284+
type: 'boolean',
285+
},
286+
tags: ['experimental'],
287+
examples: [
288+
{
289+
'fetch': false,
290+
'runTests': false
291+
}
292+
],
293+
policy: {
294+
name: 'ChatToolsEligibleForAutoApproval',
295+
category: PolicyCategory.InteractiveSession,
296+
minimumVersion: '1.106',
297+
localization: {
298+
description: {
299+
key: 'chat.tools.eligibleForAutoApproval',
300+
value: nls.localize('chat.tools.eligibleForAutoApproval', 'Controls which tools are eligible for automatic approval. Tools set to \'false\' will always present a confirmation and will never offer the option to auto-approve. The default behavior (or setting a tool to \'true\') may result in the tool offering auto-approval options.')
301+
}
302+
},
303+
}
304+
},
279305
'chat.sendElementsToChat.enabled': {
280306
default: true,
281307
description: nls.localize('chat.sendElementsToChat.enabled', "Controls whether elements can be sent to chat from the Simple Browser."),

src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/abstractToolConfirmationSubPart.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,15 @@ export abstract class AbstractToolConfirmationSubPart extends BaseChatToolInvoca
6060
const skipTooltip = skipKeybinding ? `${config.skipLabel} (${skipKeybinding})` : config.skipLabel;
6161

6262

63+
const additionalActions = this.additionalPrimaryActions();
6364
const buttons: IChatConfirmationButton<(() => void)>[] = [
6465
{
6566
label: config.allowLabel,
6667
tooltip: allowTooltip,
6768
data: () => {
6869
this.confirmWith(toolInvocation, { type: ToolConfirmKind.UserAction });
6970
},
70-
moreActions: this.additionalPrimaryActions(),
71+
moreActions: additionalActions.length > 0 ? additionalActions : undefined,
7172
},
7273
{
7374
label: localize('skip', "Skip"),

src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationS
128128
if (terminalCustomActions) {
129129
moreActions.push(...terminalCustomActions);
130130
}
131+
if (moreActions.length === 0) {
132+
moreActions = undefined;
133+
}
134+
131135
}
132136

133137
const codeBlockRenderOptions: ICodeBlockRenderOptions = {

src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export class ToolConfirmationSubPart extends AbstractToolConfirmationSubPart {
129129
const { message, disclaimer } = this.toolInvocation.confirmationMessages!;
130130
const toolInvocation = this.toolInvocation as IChatToolInvocation;
131131

132-
if (typeof message === 'string') {
132+
if (typeof message === 'string' && !disclaimer) {
133133
return message;
134134
} else {
135135
const codeBlockRenderOptions: ICodeBlockRenderOptions = {

src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { Codicon } from '../../../../base/common/codicons.js';
1212
import { toErrorMessage } from '../../../../base/common/errorMessage.js';
1313
import { CancellationError, isCancellationError } from '../../../../base/common/errors.js';
1414
import { Emitter, Event } from '../../../../base/common/event.js';
15-
import { MarkdownString } from '../../../../base/common/htmlContent.js';
15+
import { markdownCommandLink, MarkdownString } from '../../../../base/common/htmlContent.js';
1616
import { Iterable } from '../../../../base/common/iterator.js';
1717
import { combinedDisposable, Disposable, DisposableStore, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js';
1818
import { IObservable, ObservableSet } from '../../../../base/common/observable.js';
@@ -445,9 +445,33 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
445445
prepared = await preparePromise;
446446
}
447447

448+
const isEligibleForAutoApproval = this.isToolEligibleForAutoApproval(tool.data);
449+
450+
// Default confirmation messages if tool is not eligible for auto-approval
451+
if (!isEligibleForAutoApproval && !prepared?.confirmationMessages?.title) {
452+
if (!prepared) {
453+
prepared = {};
454+
}
455+
456+
const toolReferenceName = getToolReferenceName(tool.data);
457+
// TODO: This should be more detailed per tool.
458+
prepared.confirmationMessages = {
459+
...prepared.confirmationMessages,
460+
title: localize('defaultToolConfirmation.title', 'Allow tool to execute?'),
461+
message: localize('defaultToolConfirmation.message', 'Run the \'{0}\' tool?', toolReferenceName),
462+
disclaimer: new MarkdownString(localize('defaultToolConfirmation.disclaimer', 'Auto approval for \'{0}\' is restricted via {1}.', getToolReferenceName(tool.data), markdownCommandLink({ title: '`' + ChatConfiguration.EligibleForAutoApproval + '`', id: 'workbench.action.openSettings', arguments: [ChatConfiguration.EligibleForAutoApproval] }, false)), { isTrusted: true }),
463+
allowAutoConfirm: false,
464+
};
465+
}
466+
467+
if (!isEligibleForAutoApproval && prepared?.confirmationMessages?.title) {
468+
// Always overwrite the disclaimer if not eligible for auto-approval
469+
prepared.confirmationMessages.disclaimer = new MarkdownString(localize('defaultToolConfirmation.disclaimer', 'Auto approval for \'{0}\' is restricted via {1}.', getToolReferenceName(tool.data), markdownCommandLink({ title: '`' + ChatConfiguration.EligibleForAutoApproval + '`', id: 'workbench.action.openSettings', arguments: [ChatConfiguration.EligibleForAutoApproval] }, false)), { isTrusted: true });
470+
}
471+
448472
if (prepared?.confirmationMessages?.title) {
449-
if (prepared.toolSpecificData?.kind !== 'terminal' && typeof prepared.confirmationMessages.allowAutoConfirm !== 'boolean') {
450-
prepared.confirmationMessages.allowAutoConfirm = true;
473+
if (prepared.toolSpecificData?.kind !== 'terminal' && prepared.confirmationMessages.allowAutoConfirm !== false) {
474+
prepared.confirmationMessages.allowAutoConfirm = isEligibleForAutoApproval;
451475
}
452476

453477
if (!prepared.toolSpecificData && tool.data.alwaysDisplayInputOutput) {
@@ -504,7 +528,35 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
504528
});
505529
}
506530

531+
private getEligbleForAutoApprovalSpecialCase(toolData: IToolData): string | undefined {
532+
if (toolData.id === 'vscode_fetchWebPage_internal') {
533+
return 'fetch';
534+
}
535+
return undefined;
536+
}
537+
538+
private isToolEligibleForAutoApproval(toolData: IToolData): boolean {
539+
const toolReferenceName = this.getEligbleForAutoApprovalSpecialCase(toolData) ?? getToolReferenceName(toolData);
540+
if (toolData.id === 'copilot_fetchWebPage') {
541+
// Special case, this fetch will call an internal tool 'vscode_fetchWebPage_internal'
542+
return true;
543+
}
544+
const eligibilityConfig = this._configurationService.getValue<Record<string, boolean>>(ChatConfiguration.EligibleForAutoApproval);
545+
return eligibilityConfig && typeof eligibilityConfig === 'object' && toolReferenceName
546+
? (eligibilityConfig[toolReferenceName] ?? true) // Default to true if not specified
547+
: true; // Default to eligible if the setting is not an object or no reference name
548+
}
549+
507550
private async shouldAutoConfirm(toolId: string, runsInWorkspace: boolean | undefined, source: ToolDataSource, parameters: unknown): Promise<ConfirmedReason | undefined> {
551+
const tool = this._tools.get(toolId);
552+
if (!tool) {
553+
return undefined;
554+
}
555+
556+
if (!this.isToolEligibleForAutoApproval(tool.data)) {
557+
return undefined;
558+
}
559+
508560
const reason = this._confirmationService.getPreConfirmAction({ toolId, source, parameters });
509561
if (reason) {
510562
return reason;

src/vs/workbench/contrib/chat/common/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export enum ChatConfiguration {
1515
EditRequests = 'chat.editRequests',
1616
GlobalAutoApprove = 'chat.tools.global.autoApprove',
1717
AutoApproveEdits = 'chat.tools.edits.autoApprove',
18+
EligibleForAutoApproval = 'chat.tools.eligibleForAutoApproval',
1819
EnableMath = 'chat.math.enabled',
1920
CheckpointsEnabled = 'chat.checkpoints.enabled',
2021
AgentSessionsViewLocation = 'chat.agentSessionsViewLocation',

src/vs/workbench/contrib/chat/test/browser/languageModelToolsService.test.ts

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,6 +1062,81 @@ suite('LanguageModelToolsService', () => {
10621062
assert.strictEqual(unspecifiedResult.content[0].value, 'unspecified');
10631063
});
10641064

1065+
test('eligibleForAutoApproval setting controls tool eligibility', async () => {
1066+
// Test the new eligibleForAutoApproval setting
1067+
const testConfigService = new TestConfigurationService();
1068+
testConfigService.setUserConfiguration('chat.tools.eligibleForAutoApproval', {
1069+
'eligibleToolRef': true,
1070+
'ineligibleToolRef': false
1071+
});
1072+
1073+
const instaService = workbenchInstantiationService({
1074+
contextKeyService: () => store.add(new ContextKeyService(testConfigService)),
1075+
configurationService: () => testConfigService
1076+
}, store);
1077+
instaService.stub(IChatService, chatService);
1078+
instaService.stub(ILanguageModelToolsConfirmationService, new MockLanguageModelToolsConfirmationService());
1079+
const testService = store.add(instaService.createInstance(LanguageModelToolsService));
1080+
1081+
// Tool explicitly marked as eligible (using toolReferenceName) - no confirmation needed
1082+
const eligibleTool = registerToolForTest(testService, store, 'eligibleTool', {
1083+
prepareToolInvocation: async () => ({}),
1084+
invoke: async () => ({ content: [{ kind: 'text', value: 'eligible tool ran' }] })
1085+
}, {
1086+
toolReferenceName: 'eligibleToolRef'
1087+
});
1088+
1089+
const sessionId = 'test-eligible';
1090+
stubGetSession(chatService, sessionId, { requestId: 'req1' });
1091+
1092+
// Eligible tool should not get default confirmation messages injected
1093+
const eligibleResult = await testService.invokeTool(
1094+
eligibleTool.makeDto({ test: 1 }, { sessionId }),
1095+
async () => 0,
1096+
CancellationToken.None
1097+
);
1098+
assert.strictEqual(eligibleResult.content[0].value, 'eligible tool ran');
1099+
1100+
// Tool explicitly marked as ineligible (using toolReferenceName) - must require confirmation
1101+
const ineligibleTool = registerToolForTest(testService, store, 'ineligibleTool', {
1102+
prepareToolInvocation: async () => ({}),
1103+
invoke: async () => ({ content: [{ kind: 'text', value: 'ineligible requires confirmation' }] })
1104+
}, {
1105+
toolReferenceName: 'ineligibleToolRef'
1106+
});
1107+
1108+
const capture: { invocation?: any } = {};
1109+
stubGetSession(chatService, sessionId + '2', { requestId: 'req2', capture });
1110+
const ineligiblePromise = testService.invokeTool(
1111+
ineligibleTool.makeDto({ test: 2 }, { sessionId: sessionId + '2' }),
1112+
async () => 0,
1113+
CancellationToken.None
1114+
);
1115+
const published = await waitForPublishedInvocation(capture);
1116+
assert.ok(published?.confirmationMessages, 'ineligible tool should require confirmation');
1117+
assert.ok(published?.confirmationMessages?.title, 'should have default confirmation title');
1118+
assert.strictEqual(published?.confirmationMessages?.allowAutoConfirm, false, 'should not allow auto confirm');
1119+
1120+
IChatToolInvocation.confirmWith(published, { type: ToolConfirmKind.UserAction });
1121+
const ineligibleResult = await ineligiblePromise;
1122+
assert.strictEqual(ineligibleResult.content[0].value, 'ineligible requires confirmation');
1123+
1124+
// Tool not specified should default to eligible - no confirmation needed
1125+
const unspecifiedTool = registerToolForTest(testService, store, 'unspecifiedTool', {
1126+
prepareToolInvocation: async () => ({}),
1127+
invoke: async () => ({ content: [{ kind: 'text', value: 'unspecified defaults to eligible' }] })
1128+
}, {
1129+
toolReferenceName: 'unspecifiedToolRef'
1130+
});
1131+
1132+
const unspecifiedResult = await testService.invokeTool(
1133+
unspecifiedTool.makeDto({ test: 3 }, { sessionId }),
1134+
async () => 0,
1135+
CancellationToken.None
1136+
);
1137+
assert.strictEqual(unspecifiedResult.content[0].value, 'unspecified defaults to eligible');
1138+
});
1139+
10651140
test('tool content formatting with alwaysDisplayInputOutput', async () => {
10661141
// Test ensureToolDetails, formatToolInput, and toolResultToIO
10671142
const toolData: IToolData = {
@@ -1755,6 +1830,70 @@ suite('LanguageModelToolsService', () => {
17551830

17561831
});
17571832

1833+
test('eligibleForAutoApproval setting can be configured via policy', async () => {
1834+
// Test that policy configuration works for eligibleForAutoApproval
1835+
// Policy values should be JSON strings for object-type settings
1836+
const testConfigService = new TestConfigurationService();
1837+
1838+
// Simulate policy configuration (would come from policy file)
1839+
const policyValue = {
1840+
'toolA': true,
1841+
'toolB': false
1842+
};
1843+
testConfigService.setUserConfiguration('chat.tools.eligibleForAutoApproval', policyValue);
1844+
1845+
const instaService = workbenchInstantiationService({
1846+
contextKeyService: () => store.add(new ContextKeyService(testConfigService)),
1847+
configurationService: () => testConfigService
1848+
}, store);
1849+
instaService.stub(IChatService, chatService);
1850+
instaService.stub(ILanguageModelToolsConfirmationService, new MockLanguageModelToolsConfirmationService());
1851+
const testService = store.add(instaService.createInstance(LanguageModelToolsService));
1852+
1853+
// Tool A is eligible (true in policy)
1854+
const toolA = registerToolForTest(testService, store, 'toolA', {
1855+
prepareToolInvocation: async () => ({}),
1856+
invoke: async () => ({ content: [{ kind: 'text', value: 'toolA executed' }] })
1857+
}, {
1858+
toolReferenceName: 'toolA'
1859+
});
1860+
1861+
// Tool B is ineligible (false in policy)
1862+
const toolB = registerToolForTest(testService, store, 'toolB', {
1863+
prepareToolInvocation: async () => ({}),
1864+
invoke: async () => ({ content: [{ kind: 'text', value: 'toolB executed' }] })
1865+
}, {
1866+
toolReferenceName: 'toolB'
1867+
});
1868+
1869+
const sessionId = 'test-policy';
1870+
stubGetSession(chatService, sessionId, { requestId: 'req1' });
1871+
1872+
// Tool A should execute without confirmation (eligible)
1873+
const resultA = await testService.invokeTool(
1874+
toolA.makeDto({ test: 1 }, { sessionId }),
1875+
async () => 0,
1876+
CancellationToken.None
1877+
);
1878+
assert.strictEqual(resultA.content[0].value, 'toolA executed');
1879+
1880+
// Tool B should require confirmation (ineligible)
1881+
const capture: { invocation?: any } = {};
1882+
stubGetSession(chatService, sessionId + '2', { requestId: 'req2', capture });
1883+
const promiseB = testService.invokeTool(
1884+
toolB.makeDto({ test: 2 }, { sessionId: sessionId + '2' }),
1885+
async () => 0,
1886+
CancellationToken.None
1887+
);
1888+
const published = await waitForPublishedInvocation(capture);
1889+
assert.ok(published?.confirmationMessages, 'toolB should require confirmation due to policy');
1890+
assert.strictEqual(published?.confirmationMessages?.allowAutoConfirm, false, 'should not allow auto confirm');
1891+
1892+
IChatToolInvocation.confirmWith(published, { type: ToolConfirmKind.UserAction });
1893+
const resultB = await promiseB;
1894+
assert.strictEqual(resultB.content[0].value, 'toolB executed');
1895+
});
1896+
17581897

17591898

17601899
});

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,12 @@ import { CommandLineCdPrefixRewriter } from './commandLineRewriter/commandLineCd
5050
import { CommandLinePwshChainOperatorRewriter } from './commandLineRewriter/commandLinePwshChainOperatorRewriter.js';
5151
import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js';
5252
import { IHistoryService } from '../../../../../services/history/common/history.js';
53+
import { ChatConfiguration } from '../../../../chat/common/constants.js';
5354

5455
// #region Tool data
5556

57+
const TOOL_REFERENCE_NAME = 'runInTerminal';
58+
5659
function createPowerShellModelDescription(shell: string): string {
5760
const isWinPwsh = isWindowsPowerShell(shell);
5861
return [
@@ -188,7 +191,7 @@ export async function createRunInTerminalToolData(
188191

189192
return {
190193
id: 'run_in_terminal',
191-
toolReferenceName: 'runInTerminal',
194+
toolReferenceName: TOOL_REFERENCE_NAME,
192195
displayName: localize('runInTerminalTool.displayName', 'Run in Terminal'),
193196
modelDescription,
194197
userDescription: localize('runInTerminalTool.userDescription', 'Tool for running commands in the terminal'),
@@ -397,9 +400,10 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
397400
// commands that would be auto approved if it were enabled.
398401
const commandLine = rewrittenCommand ?? args.command;
399402

403+
const isEligibleForAutoApproval = this._configurationService.getValue<Record<string, boolean>>(ChatConfiguration.EligibleForAutoApproval)?.[TOOL_REFERENCE_NAME] ?? true;
400404
const isAutoApproveEnabled = this._configurationService.getValue(TerminalChatAgentToolsSettingId.EnableAutoApprove) === true;
401405
const isAutoApproveWarningAccepted = this._storageService.getBoolean(TerminalToolConfirmationStorageKeys.TerminalAutoApproveWarningAccepted, StorageScope.APPLICATION, false);
402-
const isAutoApproveAllowed = isAutoApproveEnabled && isAutoApproveWarningAccepted;
406+
const isAutoApproveAllowed = isEligibleForAutoApproval && isAutoApproveEnabled && isAutoApproveWarningAccepted;
403407

404408
const commandLineAnalyzerOptions: ICommandLineAnalyzerOptions = {
405409
commandLine,
@@ -417,7 +421,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
417421
disclaimer = new MarkdownString(`$(${Codicon.info.id}) ` + disclaimersRaw.join(' '), { supportThemeIcons: true });
418422
}
419423

420-
const customActions = commandLineAnalyzerResults.map(e => e.customActions ?? []).flat();
424+
const customActions = isEligibleForAutoApproval ? commandLineAnalyzerResults.map(e => e.customActions ?? []).flat() : undefined;
421425
toolSpecificData.autoApproveInfo = commandLineAnalyzerResults.find(e => e.autoApproveInfo)?.autoApproveInfo;
422426

423427
let shellType = basename(shell, '.exe');

0 commit comments

Comments
 (0)