Skip to content

Commit 86f1b8f

Browse files
committed
Revert "Warn about global auto approve, rename setting"
1 parent 285002a commit 86f1b8f

File tree

7 files changed

+25
-97
lines changed

7 files changed

+25
-97
lines changed

src/vs/workbench/contrib/chat/browser/actions/chatAccessibilityHelp.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { IKeybindingService } from '../../../../../platform/keybinding/common/ke
1515
import { AccessibilityVerbositySettingId } from '../../../accessibility/browser/accessibilityConfiguration.js';
1616
import { INLINE_CHAT_ID } from '../../../inlineChat/common/inlineChat.js';
1717
import { ChatContextKeyExprs, ChatContextKeys } from '../../common/chatContextKeys.js';
18-
import { ChatAgentLocation, ChatConfiguration, ChatModeKind } from '../../common/constants.js';
18+
import { ChatAgentLocation, ChatModeKind } from '../../common/constants.js';
1919
import { IChatWidgetService } from '../chat.js';
2020

2121
export class PanelChatAccessibilityHelp implements IAccessibleViewImplementation {
@@ -99,7 +99,7 @@ export function getAccessibilityHelpText(type: 'panelChat' | 'inlineChat' | 'qui
9999
if (type === 'agentView') {
100100
content.push(localize('chatAgent.userActionRequired', 'An alert will indicate when user action is required. For example, if the agent wants to run something in the terminal, you will hear Action Required: Run Command in Terminal.'));
101101
content.push(localize('chatAgent.runCommand', 'To take the action, use the accept tool command{0}.', '<keybinding:workbench.action.chat.acceptTool>'));
102-
content.push(localize('chatAgent.autoApprove', 'To automatically approve tool actions without manual confirmation, set {0} to {1} in your settings.', ChatConfiguration.GlobalAutoApprove, 'true'));
102+
content.push(localize('chatAgent.autoApprove', 'To automatically approve tool actions without manual confirmation, set chat.tools.autoApprove to true in your settings.'));
103103
content.push(localize('chatAgent.acceptTool', 'To accept a tool action, use the Accept Tool Confirmation command{0}.', '<keybinding:workbench.action.chat.acceptTool>'));
104104
content.push(localize('chatAgent.openEditedFilesSetting', 'By default, when edits are made to files, they will be opened. To change this behavior, set accessibility.openChatEditedFiles to false in your settings.'));
105105
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ import './contrib/chatInputCompletions.js';
110110
import './contrib/chatInputEditorContrib.js';
111111
import './contrib/chatInputEditorHover.js';
112112
import { ChatRelatedFilesContribution } from './contrib/chatInputRelatedFilesContrib.js';
113-
import { globalAutoApproveDescription, LanguageModelToolsService } from './languageModelToolsService.js';
113+
import { LanguageModelToolsService } from './languageModelToolsService.js';
114114
import './promptSyntax/promptCodingAgentActionContribution.js';
115115
import './promptSyntax/promptToolsCodeLensProvider.js';
116116
import { PromptUrlHandler } from './promptSyntax/promptUrlHandler.js';
@@ -235,11 +235,11 @@ configurationRegistry.registerConfiguration({
235235
description: nls.localize('chat.notifyWindowOnConfirmation', "Controls whether a chat session should notify the user when a confirmation is needed while the window is not in focus. This includes a window badge as well as notification toast."),
236236
default: true,
237237
},
238-
[ChatConfiguration.GlobalAutoApprove]: {
238+
'chat.tools.autoApprove': {
239239
default: false,
240240
// Description is added in for policy parser. See https://github.com/microsoft/vscode/issues/254526
241-
description: globalAutoApproveDescription.value,
242-
markdownDescription: globalAutoApproveDescription.value,
241+
description: nls.localize('chat.tools.autoApprove.description', "Controls whether tool use should be automatically approved. Allow all tools to run automatically without user confirmation, overriding any tool-specific settings such as terminal auto-approval. Use with caution: carefully review selected tools and be extra wary of possible sources of prompt injection!"),
242+
markdownDescription: nls.localize('chat.tools.autoApprove.markdownDescription', "Controls whether tool use should be automatically approved.\n\nAllows _all_ tools to run automatically without user confirmation, overriding any tool-specific settings such as terminal auto-approval.\n\nUse with caution: carefully review selected tools and be extra wary of possible sources of prompt injection!"),
243243
type: 'boolean',
244244
scope: ConfigurationScope.APPLICATION_MACHINE,
245245
tags: ['experimental'],

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import { TerminalContribSettingId } from '../../../../terminal/terminalContribEx
2424
import { ConfigurationTarget } from '../../../../../../platform/configuration/common/configuration.js';
2525
import { matchesScheme, Schemas } from '../../../../../../base/common/network.js';
2626
import type { ICodeBlockRenderOptions } from '../../codeBlockPart.js';
27-
import { ChatConfiguration } from '../../../common/constants.js';
2827

2928
export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart {
3029
public readonly domNode: HTMLElement;
@@ -97,7 +96,7 @@ export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart
9796
case 'settings': {
9897
if (scopeRaw === 'global') {
9998
preferencesService.openSettings({
100-
query: `@id:${ChatConfiguration.GlobalAutoApprove}`
99+
query: `@id:chat.tools.autoApprove`
101100
});
102101
} else {
103102
const scope = parseInt(scopeRaw);

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

Lines changed: 8 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,13 @@ import { CancellationToken, CancellationTokenSource } from '../../../../base/com
1111
import { Codicon } from '../../../../base/common/codicons.js';
1212
import { toErrorMessage } from '../../../../base/common/errorMessage.js';
1313
import { CancellationError, isCancellationError } from '../../../../base/common/errors.js';
14-
import { Emitter, Event } from '../../../../base/common/event.js';
15-
import { MarkdownString } from '../../../../base/common/htmlContent.js';
14+
import { Emitter } from '../../../../base/common/event.js';
1615
import { Iterable } from '../../../../base/common/iterator.js';
1716
import { Lazy } from '../../../../base/common/lazy.js';
1817
import { combinedDisposable, Disposable, DisposableStore, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js';
1918
import { LRUCache } from '../../../../base/common/map.js';
2019
import { IObservable, ObservableSet } from '../../../../base/common/observable.js';
21-
import Severity from '../../../../base/common/severity.js';
2220
import { ThemeIcon } from '../../../../base/common/themables.js';
23-
import { localize, localize2 } from '../../../../nls.js';
2421
import { IAccessibilityService } from '../../../../platform/accessibility/common/accessibility.js';
2522
import { AccessibilitySignal, IAccessibilitySignalService } from '../../../../platform/accessibilitySignal/browser/accessibilitySignalService.js';
2623
import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
@@ -53,23 +50,6 @@ interface ITrackedCall {
5350
store: IDisposable;
5451
}
5552

56-
const enum AutoApproveStorageKeys {
57-
GlobalAutoApproveOptIn = 'chat.tools.global.autoApprove.optIn'
58-
}
59-
60-
export const globalAutoApproveDescription = localize2(
61-
{
62-
key: 'autoApprove2.markdown',
63-
comment: [
64-
'{Locked=\'](https://github.com/features/codespaces)\'}',
65-
'{Locked=\'](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers)\'}',
66-
'{Locked=\'](https://code.visualstudio.com/docs/copilot/security)\'}',
67-
'{Locked=\'**\'}',
68-
]
69-
},
70-
'Global auto approve also known as "YOLO mode" disables manual approval completely for _all tools in all workspaces_, allowing the agent to act fully autonomously. This is extremely dangerous and is *never* recommended, even containerized environments like [Codespaces](https://github.com/features/codespaces) and [Dev Containers](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers) have user keys forwarded into the container that could be compromised.\n\n**This feature disables [critical security protections](https://code.visualstudio.com/docs/copilot/security) and makes it much easier for an attacker to compromise the machine.**'
71-
);
72-
7353
export class LanguageModelToolsService extends Disposable implements ILanguageModelToolsService {
7454
_serviceBrand: undefined;
7555

@@ -99,8 +79,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
9979
@ILogService private readonly _logService: ILogService,
10080
@IConfigurationService private readonly _configurationService: IConfigurationService,
10181
@IAccessibilityService private readonly _accessibilityService: IAccessibilityService,
102-
@IAccessibilitySignalService private readonly _accessibilitySignalService: IAccessibilitySignalService,
103-
@IStorageService private readonly _storageService: IStorageService,
82+
@IAccessibilitySignalService private readonly _accessibilitySignalService: IAccessibilitySignalService
10483
) {
10584
super();
10685

@@ -120,15 +99,6 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
12099
}
121100
}));
122101

123-
// Clear out warning accepted state if the setting is disabled
124-
this._register(Event.runAndSubscribe(this._configurationService.onDidChangeConfiguration, e => {
125-
if (!e || e.affectsConfiguration(ChatConfiguration.GlobalAutoApprove)) {
126-
if (this._configurationService.getValue(ChatConfiguration.GlobalAutoApprove) !== true) {
127-
this._storageService.remove(AutoApproveStorageKeys.GlobalAutoApproveOptIn, StorageScope.APPLICATION);
128-
}
129-
}
130-
}));
131-
132102
this._ctxToolsCount = ChatContextKeys.Tools.toolsCount.bindTo(_contextKeyService);
133103
}
134104
override dispose(): void {
@@ -320,7 +290,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
320290
const prepared = await this.prepareToolInvocation(tool, dto, token);
321291
toolInvocation = new ChatToolInvocation(prepared, tool.data, dto.callId);
322292
trackedCall.invocation = toolInvocation;
323-
const autoConfirmed = await this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace);
293+
const autoConfirmed = this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace);
324294
if (autoConfirmed) {
325295
toolInvocation.confirmed.complete(autoConfirmed);
326296
}
@@ -354,7 +324,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
354324
}
355325
} else {
356326
const prepared = await this.prepareToolInvocation(tool, dto, token);
357-
if (prepared?.confirmationMessages && !(await this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace))) {
327+
if (prepared?.confirmationMessages && !this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace)) {
358328
const result = await this._dialogService.confirm({ message: renderAsPlaintext(prepared.confirmationMessages.title), detail: renderAsPlaintext(prepared.confirmationMessages.message) });
359329
if (!result.confirmed) {
360330
throw new CancellationError();
@@ -441,7 +411,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
441411
}
442412

443413
private playAccessibilitySignal(toolInvocations: ChatToolInvocation[]): void {
444-
const autoApproved = this._configurationService.getValue(ChatConfiguration.GlobalAutoApprove);
414+
const autoApproved = this._configurationService.getValue('chat.tools.autoApprove');
445415
if (autoApproved) {
446416
return;
447417
}
@@ -483,7 +453,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
483453
});
484454
}
485455

486-
private async shouldAutoConfirm(toolId: string, runsInWorkspace: boolean | undefined): Promise<ConfirmedReason | undefined> {
456+
private shouldAutoConfirm(toolId: string, runsInWorkspace: boolean | undefined): ConfirmedReason | undefined {
487457
if (this._workspaceToolConfirmStore.value.getAutoConfirm(toolId)) {
488458
return { type: ToolConfirmKind.LmServicePerTool, scope: 'workspace' };
489459
}
@@ -494,7 +464,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
494464
return { type: ToolConfirmKind.LmServicePerTool, scope: 'session' };
495465
}
496466

497-
const config = this._configurationService.inspect<boolean | Record<string, boolean>>(ChatConfiguration.GlobalAutoApprove);
467+
const config = this._configurationService.inspect<boolean | Record<string, boolean>>('chat.tools.autoApprove');
498468

499469
// If we know the tool runs at a global level, only consider the global config.
500470
// If we know the tool runs at a workspace level, use those specific settings when appropriate.
@@ -508,51 +478,12 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
508478

509479
const autoConfirm = value === true || (typeof value === 'object' && value.hasOwnProperty(toolId) && value[toolId] === true);
510480
if (autoConfirm) {
511-
if (await this._checkGlobalAutoApprove()) {
512-
return { type: ToolConfirmKind.Setting, id: ChatConfiguration.GlobalAutoApprove };
513-
}
481+
return { type: ToolConfirmKind.Setting, id: 'chat.tools.autoApprove' };
514482
}
515483

516484
return undefined;
517485
}
518486

519-
private async _checkGlobalAutoApprove(): Promise<boolean> {
520-
const optedIn = this._storageService.getBoolean(AutoApproveStorageKeys.GlobalAutoApproveOptIn, StorageScope.APPLICATION, false);
521-
if (optedIn) {
522-
return true;
523-
}
524-
525-
const promptResult = await this._dialogService.prompt({
526-
type: Severity.Warning,
527-
message: localize('autoApprove2.title', 'Enable global auto approve?'),
528-
buttons: [
529-
{
530-
label: localize('autoApprove2.button.enable', 'Enable'),
531-
run: () => true
532-
},
533-
{
534-
label: localize('autoApprove2.button.disable', 'Disable'),
535-
run: () => false
536-
},
537-
],
538-
custom: {
539-
icon: Codicon.warning,
540-
disableCloseAction: true,
541-
markdownDetails: [{
542-
markdown: new MarkdownString(globalAutoApproveDescription.value),
543-
}],
544-
}
545-
});
546-
547-
if (promptResult.result !== true) {
548-
await this._configurationService.updateValue(ChatConfiguration.GlobalAutoApprove, false);
549-
return false;
550-
}
551-
552-
this._storageService.store(AutoApproveStorageKeys.GlobalAutoApproveOptIn, true, StorageScope.APPLICATION, StorageTarget.USER);
553-
return true;
554-
}
555-
556487
private cleanupCallDisposables(requestId: string | undefined, store: DisposableStore): void {
557488
if (requestId) {
558489
const disposables = this._callsByRequestId.get(requestId);

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ export enum ChatConfiguration {
1111
Edits2Enabled = 'chat.edits2.enabled',
1212
ExtensionToolsEnabled = 'chat.extensionTools.enabled',
1313
EditRequests = 'chat.editRequests',
14-
GlobalAutoApprove = 'chat.tools.global.autoApprove',
1514
AutoApproveEdits = 'chat.tools.edits.autoApprove',
1615
EnableMath = 'chat.math.enabled',
1716
CheckpointsEnabled = 'chat.checkpoints.enabled',

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ suite('LanguageModelToolsService', () => {
513513
test('accessibility signal for tool confirmation', async () => {
514514
// Create a test configuration service with proper settings
515515
const testConfigService = new TestConfigurationService();
516-
testConfigService.setUserConfiguration('chat.tools.global.autoApprove', false);
516+
testConfigService.setUserConfiguration('chat.tools.autoApprove', false);
517517
testConfigService.setUserConfiguration('accessibility.signals.chatUserActionRequired', { sound: 'auto', announcement: 'auto' });
518518

519519
// Create a test accessibility service that simulates screen reader being enabled
@@ -574,7 +574,7 @@ suite('LanguageModelToolsService', () => {
574574
test('accessibility signal respects autoApprove configuration', async () => {
575575
// Create a test configuration service with auto-approve enabled
576576
const testConfigService = new TestConfigurationService();
577-
testConfigService.setUserConfiguration('chat.tools.global.autoApprove', true);
577+
testConfigService.setUserConfiguration('chat.tools.autoApprove', true);
578578
testConfigService.setUserConfiguration('accessibility.signals.chatUserActionRequired', { sound: 'auto', announcement: 'auto' });
579579

580580
// Create a test accessibility service that simulates screen reader being enabled
@@ -624,7 +624,7 @@ suite('LanguageModelToolsService', () => {
624624
test('shouldAutoConfirm with basic configuration', async () => {
625625
// Test basic shouldAutoConfirm behavior with simple configuration
626626
const testConfigService = new TestConfigurationService();
627-
testConfigService.setUserConfiguration('chat.tools.global.autoApprove', true); // Global enabled
627+
testConfigService.setUserConfiguration('chat.tools.autoApprove', true); // Global enabled
628628

629629
const instaService = workbenchInstantiationService({
630630
contextKeyService: () => store.add(new ContextKeyService(testConfigService)),
@@ -654,7 +654,7 @@ suite('LanguageModelToolsService', () => {
654654
test('shouldAutoConfirm with per-tool configuration object', async () => {
655655
// Test per-tool configuration: { toolId: true/false }
656656
const testConfigService = new TestConfigurationService();
657-
testConfigService.setUserConfiguration('chat.tools.global.autoApprove', {
657+
testConfigService.setUserConfiguration('chat.tools.autoApprove', {
658658
'approvedTool': true,
659659
'deniedTool': false
660660
});
@@ -839,7 +839,7 @@ suite('LanguageModelToolsService', () => {
839839

840840
// Test case 1: Sound enabled, announcement disabled, screen reader off
841841
const testConfigService1 = new TestConfigurationService();
842-
testConfigService1.setUserConfiguration('chat.tools.global.autoApprove', false);
842+
testConfigService1.setUserConfiguration('chat.tools.autoApprove', false);
843843
testConfigService1.setUserConfiguration('accessibility.signals.chatUserActionRequired', { sound: 'on', announcement: 'off' });
844844

845845
const testAccessibilityService1 = new class extends TestAccessibilityService {
@@ -879,7 +879,7 @@ suite('LanguageModelToolsService', () => {
879879

880880
// Test case 2: Sound auto, announcement auto, screen reader on
881881
const testConfigService2 = new TestConfigurationService();
882-
testConfigService2.setUserConfiguration('chat.tools.global.autoApprove', false);
882+
testConfigService2.setUserConfiguration('chat.tools.autoApprove', false);
883883
testConfigService2.setUserConfiguration('accessibility.signals.chatUserActionRequired', { sound: 'auto', announcement: 'auto' });
884884

885885
const testAccessibilityService2 = new class extends TestAccessibilityService {
@@ -920,7 +920,7 @@ suite('LanguageModelToolsService', () => {
920920

921921
// Test case 3: Sound off, announcement off - no signal
922922
const testConfigService3 = new TestConfigurationService();
923-
testConfigService3.setUserConfiguration('chat.tools.global.autoApprove', false);
923+
testConfigService3.setUserConfiguration('chat.tools.autoApprove', false);
924924
testConfigService3.setUserConfiguration('accessibility.signals.chatUserActionRequired', { sound: 'off', announcement: 'off' });
925925

926926
const testAccessibilityService3 = new class extends TestAccessibilityService {
@@ -1318,7 +1318,7 @@ suite('LanguageModelToolsService', () => {
13181318
test('shouldAutoConfirm with workspace-specific tool configuration', async () => {
13191319
const testConfigService = new TestConfigurationService();
13201320
// Configure per-tool settings at different scopes
1321-
testConfigService.setUserConfiguration('chat.tools.global.autoApprove', { 'workspaceTool': true });
1321+
testConfigService.setUserConfiguration('chat.tools.autoApprove', { 'workspaceTool': true });
13221322

13231323
const instaService = workbenchInstantiationService({
13241324
contextKeyService: () => store.add(new ContextKeyService(testConfigService)),

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ import { Event } from '../../../../../../base/common/event.js';
4545
import { TerminalToolConfirmationStorageKeys } from '../../../../chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.js';
4646
import { IPollingResult, OutputMonitorState } from './monitoring/types.js';
4747
import { getOutput } from '../outputHelpers.js';
48-
import { ChatConfiguration } from '../../../../chat/common/constants.js';
4948

5049
const enum TerminalToolStorageKeysInternal {
5150
TerminalSession = 'chat.terminalSessions'
@@ -780,10 +779,10 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
780779
}).join(', ');
781780
}
782781

783-
const config = this._configurationService.inspect<boolean | Record<string, boolean>>(ChatConfiguration.GlobalAutoApprove);
782+
const config = this._configurationService.inspect<boolean | Record<string, boolean>>('chat.tools.autoApprove');
784783
const isGlobalAutoApproved = config?.value ?? config.defaultValue;
785784
if (isGlobalAutoApproved) {
786-
return new MarkdownString(`_${localize('autoApprove.global', 'Auto approved by setting {0}', `[\`${ChatConfiguration.GlobalAutoApprove}\`](settings_global "${localize('ruleTooltip.global', 'View settings')}")`)}_`);
785+
return new MarkdownString(`_${localize('autoApprove.global', 'Auto approved by setting {0}', `[\`chat.tools.autoApprove\`](settings_global "${localize('ruleTooltip.global', 'View settings')}")`)}_`);
787786
}
788787

789788
if (isAutoApproved) {

0 commit comments

Comments
 (0)