Skip to content

Commit 5364255

Browse files
justschenCopilot
andauthored
chat: allow leaving global Allow All mode (#340780)
* chat: allow leaving global Allow All mode Disable the global auto-approve override when a user explicitly selects a lower Agent Host permission level, then persist the session choice across both picker implementations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chat: respect enforced global Allow All policy Keep lower Agent Host permission choices unavailable when ChatToolsAutoApprove is forced true, and recheck policy before persisting stale picker selections. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chat: keep global Allow All user-controlled Remove the enterprise-enforced Allow All handling and keep the picker behavior scoped to turning off a user-enabled global auto-approve setting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent efa27b3 commit 5364255

5 files changed

Lines changed: 53 additions & 3 deletions

File tree

‎src/vs/sessions/contrib/providers/agentHost/browser/agentHostPermissionPickerDelegate.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { narrowClaudePermissionMode } from '../../../../../platform/agentHost/co
1818
import { narrowCodexPermissionsPreset } from '../../../../../platform/agentHost/common/codexSessionConfigKeys.js';
1919
import { SessionConfigPropertySchema } from '../../../../../platform/agentHost/common/state/protocol/commands.js';
2020
import { ChatConfiguration, ChatPermissionLevel, isChatPermissionLevel } from '../../../../../workbench/contrib/chat/common/constants.js';
21-
import { isAutoApprovePolicyRestricted } from '../../../../../workbench/contrib/chat/common/agentHostConfigPolicy.js';
21+
import { disableGlobalAutoApproveForPermissionSelection, isAutoApprovePolicyRestricted } from '../../../../../workbench/contrib/chat/common/agentHostConfigPolicy.js';
2222
import { IPermissionLevelMeta, IPermissionPickerDelegate } from '../../copilotChatSessions/browser/permissionPicker.js';
2323
import { getSessionConfigProvider, IAgentHostSessionsProvider, isAgentHostProvider, LOCAL_AGENT_HOST_PROVIDER_ID } from '../../../../common/agentHostSessionsProvider.js';
2424
import { ISessionConfigProvider, ISessionsProvider } from '../../../../services/sessions/common/sessionsProvider.js';
@@ -274,6 +274,7 @@ export class AgentHostPermissionPickerDelegate extends Disposable implements IPe
274274
if (!approvalProperty || value === undefined || !isSessionConfigWritable(approvalProperty.schema, provider.getCreateSessionConfig(session.sessionId) !== undefined)) {
275275
throw new Error('Approval configuration is unavailable for this session');
276276
}
277+
await disableGlobalAutoApproveForPermissionSelection(this._configurationService, level);
277278
const operation = provider.setSessionConfigValue(session.sessionId, approvalProperty.key, value);
278279
provider.trackSessionConfigOperation?.(session.sessionId, operation);
279280
await operation.catch(onUnexpectedError);

‎src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostPermissionPickerDelegate.test.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import { AgentHostCustomTerminalToolEnabledSettingId } from '../../../../../../.
4040
import { SessionConfigKey } from '../../../../../../../platform/agentHost/common/sessionConfigKeys.js';
4141
import type { ISessionSandboxPolicy } from '../../../../../../../platform/agentHost/common/meta/agentSandboxPolicyMeta.js';
4242
import type { RootConfigState } from '../../../../../../../platform/agentHost/common/state/protocol/state.js';
43-
import { ChatPermissionLevel } from '../../../../../../../workbench/contrib/chat/common/constants.js';
43+
import { ChatConfiguration, ChatPermissionLevel } from '../../../../../../../workbench/contrib/chat/common/constants.js';
4444
import { AgentHostPermissionPickerDelegate, isWellKnownAutoApproveSchema, isWellKnownClaudePermissionModeSchema, isWellKnownModeSchema, isWellKnownModeValue } from '../../../browser/agentHostPermissionPickerDelegate.js';
4545
import { getPermissionLevelMeta } from '../../../../copilotChatSessions/browser/permissionPicker.js';
4646
import { IAgentHostSessionsProvider } from '../../../../../../common/agentHostSessionsProvider.js';
@@ -283,6 +283,25 @@ suite('AgentHostPermissionPickerDelegate', () => {
283283
});
284284
});
285285

286+
test('turns off global auto approve before selecting lower permissions', async () => {
287+
const configuration = new class extends TestConfigurationService {
288+
override updateValue(key: string, value: unknown): Promise<void> {
289+
return this.setUserConfiguration(key, value);
290+
}
291+
}({ [ChatConfiguration.GlobalAutoApprove]: true });
292+
const { delegate, provider } = setup(store, makeActiveSession('copilot'), 'autoApprove', undefined, undefined, configuration);
293+
294+
await delegate.setPermissionLevel(ChatPermissionLevel.Default);
295+
296+
assert.deepStrictEqual({
297+
globalAutoApprove: configuration.getValue(ChatConfiguration.GlobalAutoApprove),
298+
writes: provider.setCalls,
299+
}, {
300+
globalAutoApprove: false,
301+
writes: [[SESSION_ID, SessionConfigKey.AutoApprove, 'default']],
302+
});
303+
});
304+
286305
test('Copilot approval schema defaults are shown without inventing VS approval defaults', () => {
287306
const { delegate, provider } = setup(store, makeActiveSession('copilot'));
288307
provider.config = {

‎src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ import { IWorkspaceContextService } from '../../../../../../platform/workspace/c
5151
import type { IChatWidget } from '../../chat.js';
5252
import { ChatConfiguration, ChatPermissionLevel, isChatPermissionLevel } from '../../../common/constants.js';
5353
import { SessionType } from '../../../common/chatSessionsService.js';
54-
import { isAutoApprovePolicyRestricted, isAutoApproveValuePolicyRestricted, normalizeSessionConfigValue } from '../../../common/agentHostConfigPolicy.js';
54+
import { disableGlobalAutoApproveForPermissionSelection, isAutoApprovePolicyRestricted, isAutoApproveValuePolicyRestricted, normalizeSessionConfigValue } from '../../../common/agentHostConfigPolicy.js';
5555
import { maybeConfirmElevatedPermissionLevel } from '../../../common/chatPermissionWarnings.js';
5656
import { getChatSessionType, isUntitledChatSession } from '../../../common/model/chatUri.js';
5757
import { withChatInputPickerMotion } from '../../widget/input/chatInputPickerActionItem.js';
@@ -1185,6 +1185,9 @@ export class AgentHostChatInputPicker extends Disposable {
11851185
: approval ? writeSessionApprovalLevel(approval, approvalValue)
11861186
: normalizeSessionConfigValue(ctx.key, value, isAutoApprovePolicyRestricted(this._configurationService, ctx.configSchema));
11871187
validateSessionConfigWrite(ctx.configSchema, ctx.values, ctx.key, normalizedValue, isUntitledChatSession(sessionResource));
1188+
if (property === SessionConfigKey.AutoApprove && !this._generic && isChatPermissionLevel(value)) {
1189+
await disableGlobalAutoApproveForPermissionSelection(this._configurationService, value);
1190+
}
11881191
const partial = { [ctx.key]: normalizedValue };
11891192
const nextConfig = filterSessionConfigValues(ctx.configSchema, { ...this._readCurrentValues(), ...partial });
11901193

‎src/vs/workbench/contrib/chat/common/agentHostConfigPolicy.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ export function isAutoApproveValuePolicyRestricted(value: unknown, policyRestric
3939
return policyRestricted && value !== ChatPermissionLevel.Default;
4040
}
4141

42+
/** Disables the global override when the user explicitly selects a lower session permission. */
43+
export async function disableGlobalAutoApproveForPermissionSelection(configurationService: IConfigurationService, level: ChatPermissionLevel): Promise<void> {
44+
if (level !== ChatPermissionLevel.AutoApprove && configurationService.getValue<boolean>(ChatConfiguration.GlobalAutoApprove) === true) {
45+
await configurationService.updateValue(ChatConfiguration.GlobalAutoApprove, false);
46+
}
47+
}
48+
4249
export function normalizeSessionConfigValue(property: string, value: string, policyRestricted: boolean): string;
4350
export function normalizeSessionConfigValue(property: string, value: unknown, policyRestricted: boolean): unknown;
4451
export function normalizeSessionConfigValue(property: string, value: unknown, policyRestricted: boolean): unknown {

‎src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ suite('AgentHostChatInputPicker - combined mode and permissions', () => {
109109
const result = super.inspect<T>(key);
110110
return { ...result, policyValue: this.policyRestricted && key === ChatConfiguration.GlobalAutoApprove ? result.value : undefined };
111111
}
112+
override updateValue(key: string, value: unknown): Promise<void> {
113+
return this.setUserConfiguration(key, value);
114+
}
112115
}({
113116
[ChatConfiguration.ExperimentalModePermissionsPicker]: combined,
114117
[ChatConfiguration.GlobalAutoApprove]: false,
@@ -1003,6 +1006,23 @@ suite('AgentHostChatInputPicker - combined mode and permissions', () => {
10031006
});
10041007

10051008
for (const combined of [false, true]) {
1009+
test(`turns off global auto approve when selecting lower permissions (${combined ? 'combined' : 'separate'} picker)`, async () => {
1010+
const { modePicker, permissionPicker, configuration, config, actionWidget, dispatches } = setup(combined);
1011+
await configuration.setUserConfiguration(ChatConfiguration.GlobalAutoApprove, true);
1012+
config.values.autoApprove = 'autoApprove';
1013+
const picker = combined ? modePicker : permissionPicker;
1014+
await picker['_showPicker'](document.createElement('div'), combined);
1015+
await actionWidget.select('Manual permissions');
1016+
1017+
assert.deepStrictEqual({
1018+
globalAutoApprove: configuration.getValue(ChatConfiguration.GlobalAutoApprove),
1019+
dispatches,
1020+
}, {
1021+
globalAutoApprove: false,
1022+
dispatches: [{ type: ActionType.SessionConfigChanged, config: { autoApprove: 'default' } }],
1023+
});
1024+
});
1025+
10061026
test(`offers experimental Assisted permissions without an opt-in setting (${combined ? 'combined' : 'separate'} picker)`, async () => {
10071027
const { modePicker, permissionPicker, configuration, config, actionWidget, dispatches } = setup(combined);
10081028
await configuration.setUserConfiguration('chat.assistedPermissions.enabled', false);

0 commit comments

Comments
 (0)