From e3d1e4f115d57f534ed9178560702ee010e1fe67 Mon Sep 17 00:00:00 2001 From: Josh Spicer <23246594+joshspicer@users.noreply.github.com> Date: Wed, 12 Nov 2025 18:50:30 -0800 Subject: [PATCH] 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 --- .../contrib/chat/browser/chat.contribution.ts | 15 ++++ .../abstractToolConfirmationSubPart.ts | 3 +- .../chat/browser/languageModelToolsService.ts | 27 ++++++- .../contrib/chat/common/constants.ts | 1 + .../browser/languageModelToolsService.test.ts | 75 +++++++++++++++++++ 5 files changed, 119 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts index f024f7a0ed2..7ed23509074 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts @@ -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': { default: true, description: nls.localize('chat.sendElementsToChat.enabled', "Controls whether elements can be sent to chat from the Simple Browser."), diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/abstractToolConfirmationSubPart.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/abstractToolConfirmationSubPart.ts index a1ed537cf4a..c2ac795f45e 100644 --- a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/abstractToolConfirmationSubPart.ts +++ b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/abstractToolConfirmationSubPart.ts @@ -60,6 +60,7 @@ export abstract class AbstractToolConfirmationSubPart extends BaseChatToolInvoca const skipTooltip = skipKeybinding ? `${config.skipLabel} (${skipKeybinding})` : config.skipLabel; + const additionalActions = this.additionalPrimaryActions(); const buttons: IChatConfirmationButton<(() => void)>[] = [ { label: config.allowLabel, @@ -67,7 +68,7 @@ export abstract class AbstractToolConfirmationSubPart extends BaseChatToolInvoca data: () => { this.confirmWith(toolInvocation, { type: ToolConfirmKind.UserAction }); }, - moreActions: this.additionalPrimaryActions(), + moreActions: additionalActions.length > 0 ? additionalActions : undefined, }, { label: localize('skip', "Skip"), diff --git a/src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts b/src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts index 4cfe60b1ec8..e26b54a8c98 100644 --- a/src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts +++ b/src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts @@ -445,9 +445,26 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo 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.toolSpecificData?.kind !== 'terminal' && typeof prepared.confirmationMessages.allowAutoConfirm !== 'boolean') { - prepared.confirmationMessages.allowAutoConfirm = true; + prepared.confirmationMessages.allowAutoConfirm = isEligibleForAutoApproval; } 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>(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 { const reason = this._confirmationService.getPreConfirmAction({ toolId, source, parameters }); if (reason) { diff --git a/src/vs/workbench/contrib/chat/common/constants.ts b/src/vs/workbench/contrib/chat/common/constants.ts index 67c7f39eb38..8822da3cc2b 100644 --- a/src/vs/workbench/contrib/chat/common/constants.ts +++ b/src/vs/workbench/contrib/chat/common/constants.ts @@ -16,6 +16,7 @@ export enum ChatConfiguration { GlobalAutoApprove = 'chat.tools.global.autoApprove', AutoApproveEdits = 'chat.tools.edits.autoApprove', AutoApprovedUrls = 'chat.tools.urls.autoApprove', + EligibleForAutoApproval = 'chat.tools.eligibleForAutoApproval', EnableMath = 'chat.math.enabled', CheckpointsEnabled = 'chat.checkpoints.enabled', AgentSessionsViewLocation = 'chat.agentSessionsViewLocation', diff --git a/src/vs/workbench/contrib/chat/test/browser/languageModelToolsService.test.ts b/src/vs/workbench/contrib/chat/test/browser/languageModelToolsService.test.ts index 8dca180719e..29a709ef7d0 100644 --- a/src/vs/workbench/contrib/chat/test/browser/languageModelToolsService.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/languageModelToolsService.test.ts @@ -1062,6 +1062,81 @@ suite('LanguageModelToolsService', () => { 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 ensureToolDetails, formatToolInput, and toolResultToIO const toolData: IToolData = {