From 3e210b33ae814fcd221b36d4f083e3daf197693d Mon Sep 17 00:00:00 2001 From: Josh Spicer <23246594+joshspicer@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:30:12 -0700 Subject: [PATCH 1/5] Agent Host: Require confirmation for managed permission asks Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 18f1bbc1-6001-43e2-b293-724505087f6a --- .../node/copilot/copilotAgentSession.ts | 19 +++++----- .../node/copilot/copilotToolDisplay.ts | 2 + .../test/node/copilotAgentSession.test.ts | 37 +++++++++++++++++++ 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index 5dd3914cc7a0d4..227e4c8f8ef7d4 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -2141,7 +2141,8 @@ export class CopilotAgentSession extends Disposable { return { kind: 'reject' }; } - const autoApproval = this._lastAppliedPermissionMode === 'auto' + const managedApprovalRequired = request.managedApprovalRequired === true; + const autoApproval = !managedApprovalRequired && this._lastAppliedPermissionMode === 'auto' ? await this._takeAutoApproval(toolCallId) : undefined; const recommendation = autoApproval?.recommendation; @@ -2182,14 +2183,14 @@ export class CopilotAgentSession extends Disposable { const approvedSignature = this._approvedDuplicablePermissionSignatures.get(toolCallId); if (approvedSignature !== undefined) { this._approvedDuplicablePermissionSignatures.delete(toolCallId); - if ((request.kind === 'write' || request.kind === 'read') && safeStringify(request) === approvedSignature) { + if (!managedApprovalRequired && (request.kind === 'write' || request.kind === 'read') && safeStringify(request) === approvedSignature) { this._logService.info(`[Copilot:${this.sessionId}] Auto-approving duplicate ${request.kind} permission request for tool call ${toolCallId}`); return { kind: 'approve-once' }; } } const sessionResourcePath = this._getInternalSessionResourcePath(request); - if (sessionResourcePath) { + if (!managedApprovalRequired && sessionResourcePath) { this._logService.info(`[Copilot:${this.sessionId}] Auto-approving internal session resource ${sessionResourcePath}`); return { kind: 'approve-once' }; } @@ -2201,7 +2202,7 @@ export class CopilotAgentSession extends Disposable { // read those same files back, and prompting the user to // approve a read of bytes they themselves attached is // redundant. - if (request.kind === 'read' && typeof request.path === 'string' + if (!managedApprovalRequired && request.kind === 'read' && typeof request.path === 'string' && this._isSessionAttachmentPath(request.path) ) { this._logService.info(`[Copilot:${this.sessionId}] Auto-approving session attachment ${request.path}`); @@ -2212,7 +2213,7 @@ export class CopilotAgentSession extends Disposable { // Copilot SDK itself. The SDK spills oversized tool results to // `os.tmpdir()/copilot-tool-output-…txt` and then asks the model // to read them back in a follow-up turn — no need to confirm. - if (request.kind === 'read' && typeof request.path === 'string') { + if (!managedApprovalRequired && request.kind === 'read' && typeof request.path === 'string') { if (isCopilotSdkToolOutputTempFile(request.path, this._environmentService.tmpDir.fsPath)) { this._logService.info(`[Copilot:${this.sessionId}] Auto-approving Copilot SDK tool-output temp file ${request.path}`); return { kind: 'approve-once' }; @@ -2224,7 +2225,7 @@ export class CopilotAgentSession extends Disposable { // workspace, shell, or network, so prompting for them is redundant // noise. Tools that explicitly require confirmation (e.g. revealing // unreviewed review comments) are excluded so the user is prompted. - if (request.kind === 'custom-tool' && typeof request.toolName === 'string' + if (!managedApprovalRequired && request.kind === 'custom-tool' && typeof request.toolName === 'string' && this._serverToolHost?.toolNames.includes(request.toolName) && !this._serverToolHost.requiresConfirmation(request.toolName) ) { @@ -2235,7 +2236,7 @@ export class CopilotAgentSession extends Disposable { const isShellRequest = request.kind === 'shell' || (request.kind === 'custom-tool' && typeof request.toolName === 'string' && isShellTool(request.toolName)); - if (request.kind === 'custom-tool' + if (!managedApprovalRequired && request.kind === 'custom-tool' && typeof request.toolName === 'string' && this._clientToolNames.has(this._clientToolName(request.toolName)) && this._pendingClientToolCalls.hasBufferedResult(toolCallId) @@ -2255,7 +2256,7 @@ export class CopilotAgentSession extends Disposable { // fall through to the normal confirmation flow — otherwise enabling // `sandbox.allowBypass` would let the model escape the sandbox with no // prompt at all. - if (isShellRequest && !request.requestSandboxBypass && await this._isShellSandboxedByDefault()) { + if (!managedApprovalRequired && isShellRequest && !request.requestSandboxBypass && await this._isShellSandboxedByDefault()) { // Session may have been disposed while we awaited the engine // check; if so the deferred has already been settled and // removed, so leave it alone. @@ -2321,7 +2322,7 @@ export class CopilotAgentSession extends Disposable { const result = await deferred.p; this._logService.info(`[Copilot:${this.sessionId}] Permission response: toolCallId=${toolCallId}, result=${result.kind}`); - if (result.kind === 'approve-once' && (request.kind === 'write' || request.kind === 'read')) { + if (!managedApprovalRequired && result.kind === 'approve-once' && (request.kind === 'write' || request.kind === 'read')) { this._approvedDuplicablePermissionSignatures.set(toolCallId, safeStringify(request)); } return result; diff --git a/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts b/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts index a1a4a498621ccb..4f76a9a10915d9 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts @@ -1051,6 +1051,8 @@ export function tryStringify(value: unknown): string | undefined { export interface ITypedPermissionRequest { /** Permission kind discriminator from the SDK. */ kind: PermissionRequest['kind']; + /** Whether managed policy requires a human response and forbids host auto-approval. */ + managedApprovalRequired?: boolean; /** Tool call ID that triggered this permission request, when available. */ toolCallId?: string; /** File path — set for `read` permission requests. */ diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index ca27d36791478a..1eaafb635191b5 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -2146,6 +2146,43 @@ suite('CopilotAgentSession', () => { }); }); + test('managed approval requires confirmation despite an approve recommendation', async () => { + const { session, runtime, mockSession, signals, waitForSignal } = await createAgentSession(disposables, { + configValues: { [SessionConfigKey.AutoApprove]: 'assisted' }, + }); + await session.syncPermissionMode('turn-start'); + mockSession.fire('permission.requested', { + requestId: 'request-managed', + permissionRequest: { + kind: 'read', + path: '/workspace/src/file.ts', + intention: 'Read the file', + toolCallId: 'tc-managed', + managedApprovalRequired: true, + }, + promptRequest: { + kind: 'path', + accessKind: 'read', + paths: ['/workspace/src/file.ts'], + toolCallId: 'tc-managed', + managedApprovalRequired: true, + autoApproval: { recommendation: 'approve', reason: 'Low risk' }, + }, + }); + + const resultPromise = runtime.handlePermissionRequest({ + kind: 'read', + path: '/workspace/src/file.ts', + toolCallId: 'tc-managed', + managedApprovalRequired: true, + }); + + await waitForSignal(signal => signal.kind === 'pending_confirmation'); + assert.strictEqual(signals.length, 1); + assert.ok(session.respondToPermissionRequest('tc-managed', true)); + assert.strictEqual((await resultPromise).kind, 'approve-once'); + }); + test('Approve When Safe correlates a recommendation event that arrives after the permission callback', async () => { const { session, runtime, mockSession, signals } = await createAgentSession(disposables, { configValues: { [SessionConfigKey.AutoApprove]: 'assisted' }, From a9276595f606b037f6f4da0d01e8dcc4f8f0dc14 Mon Sep 17 00:00:00 2001 From: Josh Spicer <23246594+joshspicer@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:01:52 -0700 Subject: [PATCH 2/5] Agent Host: Route managed asks to client confirmation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 18f1bbc1-6001-43e2-b293-724505087f6a --- .../platform/agentHost/common/agentService.ts | 2 + .../agentHost/node/agentSideEffects.ts | 4 +- .../node/copilot/copilotAgentSession.ts | 1 + .../test/node/agentSideEffects.test.ts | 48 +++++++++++++++++++ .../test/node/copilotAgentSession.test.ts | 1 + 5 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/vs/platform/agentHost/common/agentService.ts b/src/vs/platform/agentHost/common/agentService.ts index d07b370326e687..802820d393c87a 100644 --- a/src/vs/platform/agentHost/common/agentService.ts +++ b/src/vs/platform/agentHost/common/agentService.ts @@ -1259,6 +1259,8 @@ export interface IAgentToolPendingConfirmationSignal { readonly permissionKind?: 'shell' | 'write' | 'mcp' | 'read' | 'url' | 'skill' | 'custom-tool' | 'hook' | 'memory' | 'extension-management' | 'extension-permission-access'; /** Host-only auto-approval path target (not part of the dispatched action). */ readonly permissionPath?: string; + /** Host-only flag requiring the client to show a confirmation instead of applying host auto-approval. */ + readonly managedApprovalRequired?: boolean; /** * Host-only flag (not part of the dispatched action): the model requested * this shell command run OUTSIDE the sandbox (and the host opted in via diff --git a/src/vs/platform/agentHost/node/agentSideEffects.ts b/src/vs/platform/agentHost/node/agentSideEffects.ts index 86ef43860ddcbf..0f0fcbeb76dafd 100644 --- a/src/vs/platform/agentHost/node/agentSideEffects.ts +++ b/src/vs/platform/agentHost/node/agentSideEffects.ts @@ -1060,7 +1060,9 @@ export class AgentSideEffects extends Disposable { toolInput: e.state.toolInput, requestSandboxBypass: e.requestSandboxBypass, }; - const autoApproval = await this._permissionManager.getAutoApproval(approvalEvent, sessionKey); + const autoApproval = e.managedApprovalRequired + ? undefined + : await this._permissionManager.getAutoApproval(approvalEvent, sessionKey); const part = this._stateManager.getSessionState(sessionKey)?.activeTurn?.responseParts.find(part => part.kind === ResponsePartKind.ToolCall && part.toolCall.toolCallId === e.state.toolCallId); const toolCall = part?.kind === ResponsePartKind.ToolCall ? part.toolCall : undefined; if (toolCall diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index 227e4c8f8ef7d4..0a7b0a336213ed 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -2316,6 +2316,7 @@ export class CopilotAgentSession extends Disposable { }, permissionKind, permissionPath, + managedApprovalRequired, requestSandboxBypass: request.requestSandboxBypass, parentToolCallId, }); diff --git a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts index 84cfbf2aaff2b8..e1fb538f59ff07 100644 --- a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts @@ -4745,6 +4745,54 @@ suite('AgentSideEffects', () => { ]); }); + test('managed approval bypasses session auto-approval and reaches the client confirmation UI', async () => { + setupSession(); + stateManager.setSessionConfig(sessionUri.toString(), { + schema: { type: 'object', properties: {} }, + values: { permissions: { allow: ['CustomTool'], deny: [] } }, + }); + startTurn('turn-1'); + disposables.add(sideEffects.registerProgressListener(agent)); + + agent.fireProgress({ + kind: 'action', resource: URI.parse(defaultChatUri), + action: { + type: ActionType.ChatToolCallStart, turnId: 'turn-1', + toolCallId: 'tc-managed', toolName: 'CustomTool', displayName: 'Custom Tool', contributor: undefined, + }, + }); + agent.fireProgress({ + kind: 'pending_confirmation', chat: URI.parse(defaultChatUri), + state: { + status: ToolCallStatus.PendingConfirmation, + toolCallId: 'tc-managed', toolName: 'CustomTool', displayName: 'Custom Tool', + invocationMessage: 'Run managed custom tool', toolInput: undefined, + confirmationTitle: 'Run managed custom tool', edits: undefined, + }, + permissionKind: 'custom-tool', + managedApprovalRequired: true, + }); + + const toolCall = await waitForState(stateManager, () => { + const part = stateManager.getSessionState(sessionUri.toString())?.activeTurn?.responseParts.find( + responsePart => responsePart.kind === ResponsePartKind.ToolCall && responsePart.toolCall.toolCallId === 'tc-managed' + ); + return part?.kind === ResponsePartKind.ToolCall && part.toolCall.status === ToolCallStatus.PendingConfirmation + ? part.toolCall + : undefined; + }); + + assert.deepStrictEqual({ + status: toolCall.status, + options: toolCall.options?.map(option => option.id), + responses: agent.respondToPermissionCalls, + }, { + status: ToolCallStatus.PendingConfirmation, + options: ['allow-session', 'allow-once', 'skip'], + responses: [], + }); + }); + test('subagent tool calls inherit parent session permissions', async () => { setupSession(); stateManager.setSessionConfig(sessionUri.toString(), { diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index 1eaafb635191b5..e22d73f792fc31 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -2179,6 +2179,7 @@ suite('CopilotAgentSession', () => { await waitForSignal(signal => signal.kind === 'pending_confirmation'); assert.strictEqual(signals.length, 1); + assert.strictEqual(signals[0].kind === 'pending_confirmation' && signals[0].managedApprovalRequired, true); assert.ok(session.respondToPermissionRequest('tc-managed', true)); assert.strictEqual((await resultPromise).kind, 'approve-once'); }); From 1bf226b95baa798459730da107d210ab4fd172ab Mon Sep 17 00:00:00 2001 From: Josh Spicer <23246594+joshspicer@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:30:51 -0700 Subject: [PATCH 3/5] docs: align managed selector name with Domain Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c7f00b84-b0a7-4cdf-aca9-ffd49737f26e --- src/vs/platform/agentHost/common/agentService.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/vs/platform/agentHost/common/agentService.ts b/src/vs/platform/agentHost/common/agentService.ts index 802820d393c87a..3ed04702b113df 100644 --- a/src/vs/platform/agentHost/common/agentService.ts +++ b/src/vs/platform/agentHost/common/agentService.ts @@ -1259,7 +1259,10 @@ export interface IAgentToolPendingConfirmationSignal { readonly permissionKind?: 'shell' | 'write' | 'mcp' | 'read' | 'url' | 'skill' | 'custom-tool' | 'hook' | 'memory' | 'extension-management' | 'extension-permission-access'; /** Host-only auto-approval path target (not part of the dispatched action). */ readonly permissionPath?: string; - /** Host-only flag requiring the client to show a confirmation instead of applying host auto-approval. */ + /** + * Host-only flag requiring the client to show a confirmation instead of applying host auto-approval. + * The runtime currently sets it for managed Shell, Read, Edit, and Domain selector asks. + */ readonly managedApprovalRequired?: boolean; /** * Host-only flag (not part of the dispatched action): the model requested From 8e76a036344f94c7108a7804c1304718e8e617b1 Mon Sep 17 00:00:00 2001 From: Josh Spicer <23246594+joshspicer@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:09:09 -0700 Subject: [PATCH 4/5] Agent Host: Keep managed approvals one-time Remove session-scoped confirmation options for managed asks and ignore persisted allow-session responses defensively. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c7f00b84-b0a7-4cdf-aca9-ffd49737f26e --- .../agentHost/node/agentSideEffects.ts | 17 ++++-- .../agentHost/node/sessionPermissions.ts | 19 ++++--- .../test/node/agentSideEffects.test.ts | 54 ++++++++++++++++++- 3 files changed, 79 insertions(+), 11 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentSideEffects.ts b/src/vs/platform/agentHost/node/agentSideEffects.ts index 0f0fcbeb76dafd..9d19b90d93c160 100644 --- a/src/vs/platform/agentHost/node/agentSideEffects.ts +++ b/src/vs/platform/agentHost/node/agentSideEffects.ts @@ -131,6 +131,8 @@ export class AgentSideEffects extends Disposable { /** Maps tool call IDs to the agent that owns them, for routing confirmations. */ private readonly _toolCallAgents = new Map(); + /** Managed confirmations are human-only and must never seed host-side session permissions. */ + private readonly _managedApprovalToolCalls = new Set(); private _lastAgentInfos: readonly AgentInfo[] = []; private readonly _permissionManager: SessionPermissionManager; @@ -1075,21 +1077,27 @@ export class AgentSideEffects extends Disposable { } const contributor = e.state.contributor ?? toolCall?.contributor; let effective = e; + const toolCallKey = `${sessionKey}:${e.state.toolCallId}`; + if (e.managedApprovalRequired) { + this._managedApprovalToolCalls.add(toolCallKey); + } else { + this._managedApprovalToolCalls.delete(toolCallKey); + } const clientShouldAutoApprove = autoApproval !== undefined && contributor?.kind === ToolCallContributorKind.Client && !!e.state.confirmationTitle; if (clientShouldAutoApprove) { - this._toolCallAgents.set(`${sessionKey}:${e.state.toolCallId}`, agent.id); + this._toolCallAgents.set(toolCallKey, agent.id); effective = { ...e, state: { ...e.state, _meta: { ...toolCall?._meta, ...e.state._meta, ...toToolCallMeta({ autoApproveBySetting: true }) } } }; } else if (autoApproval !== undefined) { - this._toolCallAgents.delete(`${sessionKey}:${e.state.toolCallId}`); + this._toolCallAgents.delete(toolCallKey); agent.respondToPermissionRequest(e.state.toolCallId, true); // Strip confirmationTitle so createToolReadyAction emits the // auto-approved (no-options) action. effective = { ...e, state: { ...e.state, confirmationTitle: undefined } }; } else if (effective.state.confirmationTitle) { // Make sure the agent is registered for the eventual `ChatToolCallConfirmed` response. - this._toolCallAgents.set(`${sessionKey}:${e.state.toolCallId}`, agent.id); + this._toolCallAgents.set(toolCallKey, agent.id); } this._stateManager.dispatchServerAction( sessionKey, @@ -1157,6 +1165,7 @@ export class AgentSideEffects extends Disposable { throw new Error(`ChatToolCallConfirmed must be handled on an AHP chat channel: ${channel}`); } const toolCallKey = `${channel}:${action.toolCallId}`; + const managedApprovalRequired = this._managedApprovalToolCalls.delete(toolCallKey); const agentId = this._toolCallAgents.get(toolCallKey); if (agentId) { this._toolCallAgents.delete(toolCallKey); @@ -1168,7 +1177,7 @@ export class AgentSideEffects extends Disposable { // When the user chose "Allow in this Session", add the tool // to the session's permissions so future calls are auto-approved. - if (action.approved) { + if (action.approved && !managedApprovalRequired) { this._permissionManager.handleToolCallConfirmed(channel, action.toolCallId, action.selectedOptionId); } break; diff --git a/src/vs/platform/agentHost/node/sessionPermissions.ts b/src/vs/platform/agentHost/node/sessionPermissions.ts index eb531e16c3a269..b3feaa6a7cee7e 100644 --- a/src/vs/platform/agentHost/node/sessionPermissions.ts +++ b/src/vs/platform/agentHost/node/sessionPermissions.ts @@ -49,11 +49,14 @@ export interface IToolApprovalEvent { /** Standard per-tool confirmation options presented to the user. */ const ALLOW_SESSION_OPTION_ID = 'allow-session'; +const ALLOW_ONCE_OPTION: ConfirmationOption = { id: 'allow-once', label: localize('sessionPermissions.allowOnce', "Allow Once"), kind: ConfirmationOptionKind.Approve }; +const SKIP_OPTION: ConfirmationOption = { id: 'skip', label: localize('sessionPermissions.skip', "Skip"), kind: ConfirmationOptionKind.Deny, group: 2 }; const CONFIRMATION_OPTIONS: readonly ConfirmationOption[] = [ { id: ALLOW_SESSION_OPTION_ID, label: localize('sessionPermissions.allowSession', "Allow in this Session"), kind: ConfirmationOptionKind.Approve, group: 1 }, - { id: 'allow-once', label: localize('sessionPermissions.allowOnce', "Allow Once"), kind: ConfirmationOptionKind.Approve }, - { id: 'skip', label: localize('sessionPermissions.skip', "Skip"), kind: ConfirmationOptionKind.Deny, group: 2 }, + ALLOW_ONCE_OPTION, + SKIP_OPTION, ]; +const MANAGED_CONFIRMATION_OPTIONS: readonly ConfirmationOption[] = [ALLOW_ONCE_OPTION, SKIP_OPTION]; /** Default write-path glob rules applied to auto-approved edits. */ const DEFAULT_EDIT_AUTO_APPROVE_PATTERNS: Readonly> = { @@ -355,10 +358,14 @@ export class SessionPermissionManager extends Disposable { edits: state.edits, editable: state.editable, ...(state._meta ? { _meta: state._meta } : {}), - // Agents can supply tool-specific buttons (e.g. ExitPlanMode's - // `Approve`/`Deny`) by populating `state.options`. The standard - // `Allow Once / Allow in this Session / Skip` set is the default. - options: state.options ? state.options.slice() : CONFIRMATION_OPTIONS.slice(), + // Managed asks are one-time only. Other agents can supply tool-specific + // buttons (e.g. ExitPlanMode's `Approve`/`Deny`) via `state.options`; + // otherwise the standard session/once/skip set is used. + options: e.managedApprovalRequired + ? MANAGED_CONFIRMATION_OPTIONS.slice() + : state.options + ? state.options.slice() + : CONFIRMATION_OPTIONS.slice(), }; } return { diff --git a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts index e1fb538f59ff07..a29241a1d4d50a 100644 --- a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts @@ -4788,11 +4788,63 @@ suite('AgentSideEffects', () => { responses: agent.respondToPermissionCalls, }, { status: ToolCallStatus.PendingConfirmation, - options: ['allow-session', 'allow-once', 'skip'], + options: ['allow-once', 'skip'], responses: [], }); }); + test('managed approval does not persist allow-session from the client', async () => { + setupSession(); + stateManager.setSessionConfig(sessionUri.toString(), { + schema: { type: 'object', properties: {} }, + values: { permissions: { allow: ['ExistingTool'], deny: [] } }, + }); + startTurn('turn-1'); + disposables.add(sideEffects.registerProgressListener(agent)); + + agent.fireProgress({ + kind: 'action', resource: URI.parse(defaultChatUri), + action: { + type: ActionType.ChatToolCallStart, turnId: 'turn-1', + toolCallId: 'tc-managed', toolName: 'ManagedTool', displayName: 'Managed Tool', contributor: undefined, + }, + }); + agent.fireProgress({ + kind: 'pending_confirmation', chat: URI.parse(defaultChatUri), + state: { + status: ToolCallStatus.PendingConfirmation, + toolCallId: 'tc-managed', toolName: 'ManagedTool', displayName: 'Managed Tool', + invocationMessage: 'Run managed tool', toolInput: undefined, + confirmationTitle: 'Run managed tool', edits: undefined, + }, + permissionKind: 'custom-tool', + managedApprovalRequired: true, + }); + + await waitForState(stateManager, () => { + const part = stateManager.getSessionState(sessionUri.toString())?.activeTurn?.responseParts.find( + responsePart => responsePart.kind === ResponsePartKind.ToolCall && responsePart.toolCall.toolCallId === 'tc-managed' + ); + return part?.kind === ResponsePartKind.ToolCall && part.toolCall.status === ToolCallStatus.PendingConfirmation; + }); + sideEffects.handleAction(defaultChatUri, { + type: ActionType.ChatToolCallConfirmed, + turnId: 'turn-1', + toolCallId: 'tc-managed', + approved: true, + confirmed: 'user-action', + selectedOptionId: 'allow-session', + } as ChatAction); + + assert.deepStrictEqual(agent.respondToPermissionCalls, [ + { requestId: 'tc-managed', approved: true }, + ]); + assert.deepStrictEqual( + stateManager.getSessionState(sessionUri.toString())?.config?.values[SessionConfigKey.Permissions], + { allow: ['ExistingTool'], deny: [] }, + ); + }); + test('subagent tool calls inherit parent session permissions', async () => { setupSession(); stateManager.setSessionConfig(sessionUri.toString(), { From d46c3931ccaace0ad4e4d183d5058b2d5bbd5e46 Mon Sep 17 00:00:00 2001 From: Josh Spicer <23246594+joshspicer@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:29:38 -0700 Subject: [PATCH 5/5] Agent Host: Preserve managed client tool confirmations Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00fdaf07-fd6e-43e3-bcb8-dfc812c50913 --- .../agentHost/node/agentSideEffects.ts | 5 +- .../node/copilot/copilotAgentSession.ts | 25 +-- .../test/node/agentSideEffects.test.ts | 15 +- .../test/node/copilotAgentSession.test.ts | 157 ++++++++++++++++++ 4 files changed, 187 insertions(+), 15 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentSideEffects.ts b/src/vs/platform/agentHost/node/agentSideEffects.ts index 9d19b90d93c160..3bb0efeb68b564 100644 --- a/src/vs/platform/agentHost/node/agentSideEffects.ts +++ b/src/vs/platform/agentHost/node/agentSideEffects.ts @@ -1071,7 +1071,9 @@ export class AgentSideEffects extends Disposable { && toolCall.status !== ToolCallStatus.Streaming && toolCall.status !== ToolCallStatus.Running && toolCall.status !== ToolCallStatus.PendingConfirmation) { - this._toolCallAgents.delete(`${sessionKey}:${e.state.toolCallId}`); + const toolCallKey = `${sessionKey}:${e.state.toolCallId}`; + this._toolCallAgents.delete(toolCallKey); + this._managedApprovalToolCalls.delete(toolCallKey); this._logService.trace(`[AgentSideEffects] Dropping stale tool ready for ${e.state.toolCallId}: status=${toolCall.status}`); return; } @@ -1660,6 +1662,7 @@ export class AgentSideEffects extends Disposable { override dispose(): void { this._toolCallAgents.clear(); + this._managedApprovalToolCalls.clear(); this._toolCallTracker.clear(); super.dispose(); } diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index 0a7b0a336213ed..d971c4a5496a86 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -536,7 +536,10 @@ export class CopilotAgentSession extends Disposable { private readonly _autoApprovals = new Map(); private readonly _pendingAutoApprovals = new Map>(); /** Pending permission requests awaiting a renderer-side decision. */ - private readonly _pendingPermissions = new Map>(); + private readonly _pendingPermissions = new Map; + readonly managedApprovalRequired: boolean; + }>(); /** * Signatures ({@link safeStringify}) of user-approved `read`/`write` * permission requests, keyed by tool call id. The Copilot CLI runtime emits @@ -1266,7 +1269,9 @@ export class CopilotAgentSession extends Disposable { // Still pending permission, so this call may have errored while getting permission. // Go ahead and allow the call which will immediately see the buffered value. - this.respondToPermissionRequest(toolCallId, true); + if (this._pendingPermissions.get(toolCallId)?.managedApprovalRequired !== true) { + this.respondToPermissionRequest(toolCallId, true); + } } private _cancelMcpAuthenticationForToolCall(toolCallId: string): boolean { @@ -2248,7 +2253,7 @@ export class CopilotAgentSession extends Disposable { this._logService.info(`[Copilot:${this.sessionId}] Requesting confirmation for tool call: ${toolCallId}`); const deferred = new DeferredPromise(); - this._pendingPermissions.set(toolCallId, deferred); + this._pendingPermissions.set(toolCallId, { deferred, managedApprovalRequired }); // Auto-approve shell commands that run sandboxed by default, since the // sandbox already contains them. Commands that opted OUT of the sandbox @@ -2260,7 +2265,7 @@ export class CopilotAgentSession extends Disposable { // Session may have been disposed while we awaited the engine // check; if so the deferred has already been settled and // removed, so leave it alone. - if (this._pendingPermissions.get(toolCallId) === deferred) { + if (this._pendingPermissions.get(toolCallId)?.deferred === deferred) { this._pendingPermissions.delete(toolCallId); this._logService.info(`[Copilot:${this.sessionId}] Auto-approving sandboxed shell command for tool call ${toolCallId}`); return { kind: 'approve-once' }; @@ -2609,11 +2614,11 @@ export class CopilotAgentSession extends Disposable { } respondToPermissionRequest(requestId: string, approved: boolean): boolean { - const deferred = this._pendingPermissions.get(requestId); - if (deferred) { + const pending = this._pendingPermissions.get(requestId); + if (pending) { this._pendingPermissions.delete(requestId); this._deletePendingEditContent(requestId); - deferred.complete(approved ? { kind: 'approve-once' } : { kind: 'denied-interactively-by-user' }); + pending.deferred.complete(approved ? { kind: 'approve-once' } : { kind: 'denied-interactively-by-user' }); return true; } return false; @@ -2621,7 +2626,7 @@ export class CopilotAgentSession extends Disposable { private async _requestUnsandboxedCommandConfirmation(request: IUnsandboxedCommandConfirmationRequest): Promise { const deferred = new DeferredPromise(); - this._pendingPermissions.set(request.toolCallId, deferred); + this._pendingPermissions.set(request.toolCallId, { deferred, managedApprovalRequired: false }); const displayName = getToolDisplayName(request.toolName); const blockedDomains = request.blockedDomains?.length ? request.blockedDomains.join(', ') : undefined; @@ -4528,9 +4533,9 @@ export class CopilotAgentSession extends Disposable { // ---- cleanup ------------------------------------------------------------ private _denyPendingPermissions(): void { - for (const [toolCallId, deferred] of this._pendingPermissions) { + for (const [toolCallId, pending] of this._pendingPermissions) { this._deletePendingEditContent(toolCallId); - deferred.complete({ kind: 'reject' }); + pending.deferred.complete({ kind: 'reject' }); } this._pendingPermissions.clear(); this._approvedDuplicablePermissionSignatures.clear(); diff --git a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts index a29241a1d4d50a..36bdcd7a36280f 100644 --- a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts @@ -26,11 +26,11 @@ import { SessionConfigKey } from '../../common/sessionConfigKeys.js'; import type { RootConfigChangedAction } from '../../common/state/protocol/actions.js'; import { ChangesSummary, ChatOriginKind, CustomizationType, McpAuthRequiredReason, SessionInputRequestKind } from '../../common/state/protocol/state.js'; import { ActionType, ActionEnvelope, type ChatAction, type INotification, type SessionAction } from '../../common/state/sessionActions.js'; -import { buildSubagentChatUri, buildChatUri, buildDefaultChatUri, ChatInputAnswerState, ChatInputAnswerValueKind, ChatInputQuestionKind, ChatInteractivity, CustomizationLoadStatus, MessageAttachmentKind, MessageKind, PendingMessageKind, ResponsePartKind, SessionInputResponseKind, SessionLifecycle, SessionStatus, ToolCallConfirmationReason, ToolCallContributorKind, ToolCallStatus, ToolResultContentType, TurnState, customizationId, type ClientPluginCustomization, type Customization, type PluginCustomization } from '../../common/state/sessionState.js'; +import { buildSubagentChatUri, buildChatUri, buildDefaultChatUri, ChatInputAnswerState, ChatInputAnswerValueKind, ChatInputQuestionKind, ChatInteractivity, CustomizationLoadStatus, MessageAttachmentKind, MessageKind, PendingMessageKind, ResponsePartKind, ROOT_STATE_URI, SessionInputResponseKind, SessionLifecycle, SessionStatus, ToolCallConfirmationReason, ToolCallContributorKind, ToolCallStatus, ToolResultContentType, TurnState, customizationId, type ClientPluginCustomization, type Customization, type PluginCustomization } from '../../common/state/sessionState.js'; import { IProductService } from '../../../product/common/productService.js'; import { ITelemetryService, TelemetryLevel } from '../../../telemetry/common/telemetry.js'; import { NullTelemetryService } from '../../../telemetry/common/telemetryUtils.js'; -import { AgentHostTelemetryLevelConfigKey, telemetryLevelToAgentHostConfigValue } from '../../common/agentHostSchema.js'; +import { AgentHostGlobalAutoApproveEnabledConfigKey, AgentHostTelemetryLevelConfigKey, telemetryLevelToAgentHostConfigValue } from '../../common/agentHostSchema.js'; import { AgentConfigurationService, IAgentConfigurationService } from '../../node/agentConfigurationService.js'; import { AgentHostTelemetryService } from '../../node/agentHostTelemetryService.js'; import { IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE } from '../../common/agentHostCheckpointService.js'; @@ -4745,11 +4745,18 @@ suite('AgentSideEffects', () => { ]); }); - test('managed approval bypasses session auto-approval and reaches the client confirmation UI', async () => { + test('managed approval bypasses global, session, and per-tool auto-approval', async () => { setupSession(); + stateManager.dispatchServerAction(ROOT_STATE_URI, { + type: ActionType.RootConfigChanged, + config: { [AgentHostGlobalAutoApproveEnabledConfigKey]: true }, + }); stateManager.setSessionConfig(sessionUri.toString(), { schema: { type: 'object', properties: {} }, - values: { permissions: { allow: ['CustomTool'], deny: [] } }, + values: { + [SessionConfigKey.AutoApprove]: 'autoApprove', + [SessionConfigKey.Permissions]: { allow: ['CustomTool'], deny: [] }, + }, }); startTurn('turn-1'); disposables.add(sideEffects.registerProgressListener(agent)); diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index e22d73f792fc31..1e800702cdee19 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -2184,6 +2184,88 @@ suite('CopilotAgentSession', () => { assert.strictEqual((await resultPromise).kind, 'approve-once'); }); + test('managed approval requires confirmation under global and session allow-all modes', async () => { + const testCases = [ + { + name: 'global', + options: { rootValues: { [AgentHostGlobalAutoApproveEnabledConfigKey]: true } }, + }, + { + name: 'session', + options: { configValues: { [SessionConfigKey.AutoApprove]: 'autoApprove' } }, + }, + ]; + + for (const testCase of testCases) { + const { session, runtime, signals, waitForSignal } = await createAgentSession(disposables, testCase.options); + await session.syncPermissionMode('turn-start'); + const toolCallId = `tc-managed-${testCase.name}`; + const resultPromise = runtime.handlePermissionRequest({ + kind: 'read', + path: '/workspace/src/file.ts', + toolCallId, + managedApprovalRequired: true, + }); + + await waitForSignal(signal => signal.kind === 'pending_confirmation'); + assert.deepStrictEqual({ + managedApprovalRequired: signals[0].kind === 'pending_confirmation' ? signals[0].managedApprovalRequired : undefined, + responded: session.respondToPermissionRequest(toolCallId, false), + result: await resultPromise, + }, { + managedApprovalRequired: true, + responded: true, + result: { kind: 'denied-interactively-by-user' }, + }, testCase.name); + disposables.clear(); + } + }); + + test('managed read and write approvals do not auto-approve duplicate requests', async () => { + const testCases = [ + { + name: 'read', + request: { + kind: 'read' as const, + path: '/workspace/src/file.ts', + toolCallId: 'tc-managed-duplicate-read', + managedApprovalRequired: true, + }, + }, + { + name: 'write', + request: { + kind: 'write' as const, + fileName: '/workspace/src/file.ts', + toolCallId: 'tc-managed-duplicate-write', + managedApprovalRequired: true, + }, + }, + ]; + + for (const testCase of testCases) { + const { session, runtime, signals, waitForSignal } = await createAgentSession(disposables); + const firstResultPromise = runtime.handlePermissionRequest(testCase.request); + await waitForSignal(signal => signal.kind === 'pending_confirmation'); + assert.ok(session.respondToPermissionRequest(testCase.request.toolCallId, true)); + const firstResult = await firstResultPromise; + + const duplicateResultPromise = runtime.handlePermissionRequest({ ...testCase.request }); + await timeout(0); + const pendingConfirmationCount = signals.filter(signal => signal.kind === 'pending_confirmation').length; + assert.ok(session.respondToPermissionRequest(testCase.request.toolCallId, false)); + + assert.deepStrictEqual({ + results: [firstResult, await duplicateResultPromise], + pendingConfirmationCount, + }, { + results: [{ kind: 'approve-once' }, { kind: 'denied-interactively-by-user' }], + pendingConfirmationCount: 2, + }, testCase.name); + disposables.clear(); + } + }); + test('Approve When Safe correlates a recommendation event that arrives after the permission callback', async () => { const { session, runtime, mockSession, signals } = await createAgentSession(disposables, { configValues: { [SessionConfigKey.AutoApprove]: 'assisted' }, @@ -5409,6 +5491,44 @@ suite('CopilotAgentSession', () => { await resultPromise; }); + test('client tool completion does not approve a pending managed permission', async () => { + const { session, runtime, mockSession, signals, waitForSignal } = await createAgentSession(disposables, { + clientSnapshot: snapshot, + activeClientToolSet: activeClientToolSetWith('test-client'), + }); + mockSession.fire('tool.execution_start', { + toolCallId: 'tc-managed-client', + toolName: 'my_tool', + arguments: { file: 'test.ts' }, + } as SessionEventPayload<'tool.execution_start'>['data']); + const permissionPromise = runtime.handlePermissionRequest({ + kind: 'custom-tool', + toolCallId: 'tc-managed-client', + toolName: 'my_tool', + managedApprovalRequired: true, + }); + await waitForSignal(signal => signal.kind === 'pending_confirmation'); + + session.handleClientToolCallComplete('tc-managed-client', { + success: true, + pastTenseMessage: 'did it', + content: [{ type: ToolResultContentType.Text, text: 'result text' }], + }); + let permissionResult: Awaited | undefined; + void permissionPromise.then(result => permissionResult = result); + await timeout(0); + + assert.deepStrictEqual({ + permissionResult, + pendingConfirmations: signals.filter(signal => signal.kind === 'pending_confirmation').length, + }, { + permissionResult: undefined, + pendingConfirmations: 1, + }); + assert.ok(session.respondToPermissionRequest('tc-managed-client', false)); + assert.deepStrictEqual(await permissionPromise, { kind: 'denied-interactively-by-user' }); + }); + test('handleClientToolCallComplete with content containing embedded resources', async () => { const { session, runtime } = await createAgentSession(disposables, { clientSnapshot: snapshot }); @@ -5577,6 +5697,43 @@ suite('CopilotAgentSession', () => { }, }); }); + + test('buffered client tool completion does not approve a managed permission', async () => { + const activeClientToolSet = new ActiveClientToolSet(); + activeClientToolSet.set('test-client', snapshot.tools); + const { session, runtime, mockSession, signals, waitForSignal } = await createAgentSession(disposables, { clientSnapshot: snapshot, activeClientToolSet }); + mockSession.fire('tool.execution_start', { + toolCallId: 'tc-managed-buffered', + toolName: 'my_tool', + arguments: {}, + } as SessionEventPayload<'tool.execution_start'>['data']); + session.handleClientToolCallComplete('tc-managed-buffered', { + success: true, + pastTenseMessage: 'did it', + content: [{ type: ToolResultContentType.Text, text: 'buffered result' }], + }); + + const permissionPromise = runtime.handlePermissionRequest({ + kind: 'custom-tool', + toolCallId: 'tc-managed-buffered', + toolName: 'my_tool', + managedApprovalRequired: true, + }); + await waitForSignal(signal => signal.kind === 'pending_confirmation'); + let permissionResult: Awaited | undefined; + void permissionPromise.then(result => permissionResult = result); + await timeout(0); + + assert.deepStrictEqual({ + permissionResult, + pendingConfirmations: signals.filter(signal => signal.kind === 'pending_confirmation').length, + }, { + permissionResult: undefined, + pendingConfirmations: 1, + }); + assert.ok(session.respondToPermissionRequest('tc-managed-buffered', false)); + assert.deepStrictEqual(await permissionPromise, { kind: 'denied-interactively-by-user' }); + }); }); // ---- Server tools -------------------------------------------------------