Add eligibleForAutoApproval (#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
This commit is contained in:
Josh Spicer
2025-11-12 18:50:30 -08:00
committed by GitHub
parent b7f34c9e14
commit e3d1e4f115
5 changed files with 119 additions and 2 deletions

View File

@@ -293,6 +293,21 @@ configurationRegistry.registerConfiguration({
] ]
} }
}, },
[ChatConfiguration.EligibleForAutoApproval]: {
default: {},
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.'),
type: 'object',
additionalProperties: {
type: 'boolean',
},
tags: ['experimental'],
examples: [
{
'fetch': false,
'runTests': false
}
]
},
'chat.sendElementsToChat.enabled': { 'chat.sendElementsToChat.enabled': {
default: true, default: true,
description: nls.localize('chat.sendElementsToChat.enabled', "Controls whether elements can be sent to chat from the Simple Browser."), description: nls.localize('chat.sendElementsToChat.enabled', "Controls whether elements can be sent to chat from the Simple Browser."),

View File

@@ -60,6 +60,7 @@ export abstract class AbstractToolConfirmationSubPart extends BaseChatToolInvoca
const skipTooltip = skipKeybinding ? `${config.skipLabel} (${skipKeybinding})` : config.skipLabel; const skipTooltip = skipKeybinding ? `${config.skipLabel} (${skipKeybinding})` : config.skipLabel;
const additionalActions = this.additionalPrimaryActions();
const buttons: IChatConfirmationButton<(() => void)>[] = [ const buttons: IChatConfirmationButton<(() => void)>[] = [
{ {
label: config.allowLabel, label: config.allowLabel,
@@ -67,7 +68,7 @@ export abstract class AbstractToolConfirmationSubPart extends BaseChatToolInvoca
data: () => { data: () => {
this.confirmWith(toolInvocation, { type: ToolConfirmKind.UserAction }); this.confirmWith(toolInvocation, { type: ToolConfirmKind.UserAction });
}, },
moreActions: this.additionalPrimaryActions(), moreActions: additionalActions.length > 0 ? additionalActions : undefined,
}, },
{ {
label: localize('skip', "Skip"), label: localize('skip', "Skip"),

View File

@@ -445,9 +445,26 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
prepared = await preparePromise; prepared = await preparePromise;
} }
// TODO: If the user has _previously_ auto-approved this tool, I don't think we make it to this check.
const isEligibleForAutoApproval = this.isToolEligibleForAutoApproval(tool.data);
// Default confirmation messages if tool is not eligible for auto-approval
if (!isEligibleForAutoApproval && !prepared?.confirmationMessages?.title) {
if (!prepared) {
prepared = {};
}
const toolReferenceName = getToolReferenceName(tool.data);
// TODO: This should be more detailed per tool.
prepared.confirmationMessages = {
title: localize('defaultToolConfirmation.title', 'Allow tool to execute?'),
message: localize('defaultToolConfirmation.message', 'Run the "{0}" tool.', toolReferenceName),
allowAutoConfirm: false,
};
}
if (prepared?.confirmationMessages?.title) { if (prepared?.confirmationMessages?.title) {
if (prepared.toolSpecificData?.kind !== 'terminal' && typeof prepared.confirmationMessages.allowAutoConfirm !== 'boolean') { if (prepared.toolSpecificData?.kind !== 'terminal' && typeof prepared.confirmationMessages.allowAutoConfirm !== 'boolean') {
prepared.confirmationMessages.allowAutoConfirm = true; prepared.confirmationMessages.allowAutoConfirm = isEligibleForAutoApproval;
} }
if (!prepared.toolSpecificData && tool.data.alwaysDisplayInputOutput) { if (!prepared.toolSpecificData && tool.data.alwaysDisplayInputOutput) {
@@ -504,6 +521,14 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
}); });
} }
private isToolEligibleForAutoApproval(toolData: IToolData): boolean {
const toolReferenceName = getToolReferenceName(toolData);
const eligibilityConfig = this._configurationService.getValue<Record<string, boolean>>(ChatConfiguration.EligibleForAutoApproval);
return eligibilityConfig && typeof eligibilityConfig === 'object' && toolReferenceName
? (eligibilityConfig[toolReferenceName] ?? true) // Default to true if not specified
: true; // Default to eligible if the setting is not an object or no reference name
}
private async shouldAutoConfirm(toolId: string, runsInWorkspace: boolean | undefined, source: ToolDataSource, parameters: unknown): Promise<ConfirmedReason | undefined> { private async shouldAutoConfirm(toolId: string, runsInWorkspace: boolean | undefined, source: ToolDataSource, parameters: unknown): Promise<ConfirmedReason | undefined> {
const reason = this._confirmationService.getPreConfirmAction({ toolId, source, parameters }); const reason = this._confirmationService.getPreConfirmAction({ toolId, source, parameters });
if (reason) { if (reason) {

View File

@@ -16,6 +16,7 @@ export enum ChatConfiguration {
GlobalAutoApprove = 'chat.tools.global.autoApprove', GlobalAutoApprove = 'chat.tools.global.autoApprove',
AutoApproveEdits = 'chat.tools.edits.autoApprove', AutoApproveEdits = 'chat.tools.edits.autoApprove',
AutoApprovedUrls = 'chat.tools.urls.autoApprove', AutoApprovedUrls = 'chat.tools.urls.autoApprove',
EligibleForAutoApproval = 'chat.tools.eligibleForAutoApproval',
EnableMath = 'chat.math.enabled', EnableMath = 'chat.math.enabled',
CheckpointsEnabled = 'chat.checkpoints.enabled', CheckpointsEnabled = 'chat.checkpoints.enabled',
AgentSessionsViewLocation = 'chat.agentSessionsViewLocation', AgentSessionsViewLocation = 'chat.agentSessionsViewLocation',

View File

@@ -1062,6 +1062,81 @@ suite('LanguageModelToolsService', () => {
assert.strictEqual(unspecifiedResult.content[0].value, 'unspecified'); assert.strictEqual(unspecifiedResult.content[0].value, 'unspecified');
}); });
test('eligibleForAutoApproval setting controls tool eligibility', async () => {
// Test the new eligibleForAutoApproval setting
const testConfigService = new TestConfigurationService();
testConfigService.setUserConfiguration('chat.tools.eligibleForAutoApproval', {
'eligibleToolRef': true,
'ineligibleToolRef': false
});
const instaService = workbenchInstantiationService({
contextKeyService: () => store.add(new ContextKeyService(testConfigService)),
configurationService: () => testConfigService
}, store);
instaService.stub(IChatService, chatService);
instaService.stub(ILanguageModelToolsConfirmationService, new MockLanguageModelToolsConfirmationService());
const testService = store.add(instaService.createInstance(LanguageModelToolsService));
// Tool explicitly marked as eligible (using toolReferenceName) - no confirmation needed
const eligibleTool = registerToolForTest(testService, store, 'eligibleTool', {
prepareToolInvocation: async () => ({}),
invoke: async () => ({ content: [{ kind: 'text', value: 'eligible tool ran' }] })
}, {
toolReferenceName: 'eligibleToolRef'
});
const sessionId = 'test-eligible';
stubGetSession(chatService, sessionId, { requestId: 'req1' });
// Eligible tool should not get default confirmation messages injected
const eligibleResult = await testService.invokeTool(
eligibleTool.makeDto({ test: 1 }, { sessionId }),
async () => 0,
CancellationToken.None
);
assert.strictEqual(eligibleResult.content[0].value, 'eligible tool ran');
// Tool explicitly marked as ineligible (using toolReferenceName) - must require confirmation
const ineligibleTool = registerToolForTest(testService, store, 'ineligibleTool', {
prepareToolInvocation: async () => ({}),
invoke: async () => ({ content: [{ kind: 'text', value: 'ineligible requires confirmation' }] })
}, {
toolReferenceName: 'ineligibleToolRef'
});
const capture: { invocation?: any } = {};
stubGetSession(chatService, sessionId + '2', { requestId: 'req2', capture });
const ineligiblePromise = testService.invokeTool(
ineligibleTool.makeDto({ test: 2 }, { sessionId: sessionId + '2' }),
async () => 0,
CancellationToken.None
);
const published = await waitForPublishedInvocation(capture);
assert.ok(published?.confirmationMessages, 'ineligible tool should require confirmation');
assert.ok(published?.confirmationMessages?.title, 'should have default confirmation title');
assert.strictEqual(published?.confirmationMessages?.allowAutoConfirm, false, 'should not allow auto confirm');
IChatToolInvocation.confirmWith(published, { type: ToolConfirmKind.UserAction });
const ineligibleResult = await ineligiblePromise;
assert.strictEqual(ineligibleResult.content[0].value, 'ineligible requires confirmation');
// Tool not specified should default to eligible - no confirmation needed
const unspecifiedTool = registerToolForTest(testService, store, 'unspecifiedTool', {
prepareToolInvocation: async () => ({}),
invoke: async () => ({ content: [{ kind: 'text', value: 'unspecified defaults to eligible' }] })
}, {
toolReferenceName: 'unspecifiedToolRef'
});
const unspecifiedResult = await testService.invokeTool(
unspecifiedTool.makeDto({ test: 3 }, { sessionId }),
async () => 0,
CancellationToken.None
);
assert.strictEqual(unspecifiedResult.content[0].value, 'unspecified defaults to eligible');
});
test('tool content formatting with alwaysDisplayInputOutput', async () => { test('tool content formatting with alwaysDisplayInputOutput', async () => {
// Test ensureToolDetails, formatToolInput, and toolResultToIO // Test ensureToolDetails, formatToolInput, and toolResultToIO
const toolData: IToolData = { const toolData: IToolData = {