From 79d34e6e519a89e97d70cc6714337f58accd3ed2 Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Thu, 18 Jun 2026 21:57:38 -0700 Subject: [PATCH 1/8] AH: align autopilot and assisted approvals with CLI (#321865) * AH: align autopilot and assisted approvals with CLI * address feedback * remove assisted permissions, add better setting, better handoff between plan and autopilot * Address PR review comments on approvals config - Narrow IAgentSessionDefaultConfiguration mode/approvals types - Use neutral "new sessions" wording in elevated permission warnings - Validate normalizeAutoApproveValue against ChatPermissionLevel enum - Confirm tolerated non-default auto-approve values before applying Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/common/agentHostSchema.ts | 42 ++++- .../agentHost/common/sessionConfigKeys.ts | 24 ++- .../agentHost/node/copilot/copilotAgent.ts | 41 +++-- .../node/copilot/copilotAgentSession.ts | 121 ++++++------- .../agentHost/node/sessionPermissions.ts | 6 +- .../test/common/agentHostSchema.test.ts | 49 +++++- .../test/node/agentSideEffects.test.ts | 15 +- .../test/node/copilotAgentSession.test.ts | 69 ++++---- .../agentHostPermissionPickerDelegate.ts | 21 ++- .../browser/agentHostSessionConfigPicker.ts | 76 ++------ .../browser/baseAgentHostSessionsProvider.ts | 64 +++++-- .../agentHostPermissionPickerDelegate.test.ts | 27 ++- .../localAgentHostSessionsProvider.test.ts | 53 ++++-- .../browser/mobilePermissionPicker.ts | 48 ++--- .../browser/permissionPicker.ts | 138 ++++++++------- .../agentHost/agentHostChatInputPicker.ts | 62 ++++++- .../agentHost/agentHostSessionHandler.ts | 2 +- ...ntHostUntitledProvisionalSessionService.ts | 31 +++- .../chat/browser/chat.shared.contribution.ts | 31 +++- .../input/permissionPickerActionItem.ts | 166 ++++++++++-------- .../chat/common/chatPermissionWarnings.ts | 92 ++++++++-- .../contrib/chat/common/constants.ts | 16 ++ .../agentHostChatInputPicker.test.ts | 45 +++++ .../common/chatPermissionWarnings.test.ts | 47 +++++ 24 files changed, 830 insertions(+), 456 deletions(-) create mode 100644 src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts create mode 100644 src/vs/workbench/contrib/chat/test/common/chatPermissionWarnings.test.ts diff --git a/src/vs/platform/agentHost/common/agentHostSchema.ts b/src/vs/platform/agentHost/common/agentHostSchema.ts index 15998a2ed6fed..0d92c3c88f92e 100644 --- a/src/vs/platform/agentHost/common/agentHostSchema.ts +++ b/src/vs/platform/agentHost/common/agentHostSchema.ts @@ -259,9 +259,9 @@ function safeStringify(value: unknown): string { // ---- Platform-owned schema ------------------------------------------------- -export type AutoApproveLevel = 'default' | 'autoApprove' | 'autopilot'; +export type AutoApproveLevel = 'default' | 'autoApprove'; -export type SessionMode = 'interactive' | 'plan'; +export type SessionMode = 'interactive' | 'plan' | 'autopilot'; export interface IPermissionsValue { readonly allow: readonly string[]; @@ -307,16 +307,14 @@ export const platformSessionSchema = createSchema({ type: 'string', title: localize('agentHost.sessionConfig.autoApprove', "Approvals"), description: localize('agentHost.sessionConfig.autoApproveDescription', "Tool approval behavior for this session"), - enum: ['default', 'autoApprove', 'autopilot'], + enum: ['default', 'autoApprove'], enumLabels: [ localize('agentHost.sessionConfig.autoApprove.default', "Default Approvals"), localize('agentHost.sessionConfig.autoApprove.bypass', "Bypass Approvals"), - localize('agentHost.sessionConfig.autoApprove.autopilot', "Autopilot (Preview)"), ], enumDescriptions: [ localize('agentHost.sessionConfig.autoApprove.defaultDescription', "Copilot uses your configured settings"), localize('agentHost.sessionConfig.autoApprove.bypassDescription', "All tool calls are auto-approved"), - localize('agentHost.sessionConfig.autoApprove.autopilotDescription', "Autonomously iterates from start to finish"), ], default: 'default', sessionMutable: true, @@ -326,20 +324,52 @@ export const platformSessionSchema = createSchema({ type: 'string', title: localize('agentHost.sessionConfig.mode', "Agent Mode"), description: localize('agentHost.sessionConfig.modeDescription', "How the agent should approach this turn"), - enum: ['interactive', 'plan'], + enum: ['interactive', 'plan', 'autopilot'], enumLabels: [ localize('agentHost.sessionConfig.mode.interactive', "Interactive"), localize('agentHost.sessionConfig.mode.plan', "Plan"), + localize('agentHost.sessionConfig.mode.autopilot', "Autopilot"), ], enumDescriptions: [ localize('agentHost.sessionConfig.mode.interactiveDescription', "Step-by-step collaboration"), localize('agentHost.sessionConfig.mode.planDescription', "Plan first, execute when ready"), + localize('agentHost.sessionConfig.mode.autopilotDescription', "Autonomously iterates from start to finish"), ], default: 'interactive', sessionMutable: true, }), }); +/** + * Rewrites a legacy `autoApprove='autopilot'` config value — used before + * Autopilot moved from the `autoApprove` axis onto the orthogonal `mode` + * axis — into the current two-axis shape: + * + * - `autoApprove='autopilot'` + `mode='plan'` → `mode='plan'`, `autoApprove='default'` + * (legacy `plan` took precedence over autopilot when resolving the SDK mode). + * - `autoApprove='autopilot'` + any other mode → `mode='autopilot'`, `autoApprove='default'`. + * + * Returns a shallow copy with the migration applied, or the original + * reference unchanged when no legacy value is present. Safe to call on + * `undefined`. + * + * Without this, a session persisted (or a "remembered" picker value seeded) + * with `autoApprove='autopilot'` would fail the new schema's enum validation + * and silently fall back to `default`, downgrading the session from + * autonomous Autopilot to manual per-tool confirmation. + */ +export function migrateLegacyAutopilotConfig | undefined>(config: T): T { + if (!config || config[SessionConfigKey.AutoApprove] !== 'autopilot') { + return config; + } + const migrated: Record = { ...config }; + if (migrated[SessionConfigKey.Mode] !== 'plan') { + migrated[SessionConfigKey.Mode] = 'autopilot' satisfies SessionMode; + } + migrated[SessionConfigKey.AutoApprove] = 'default' satisfies AutoApproveLevel; + return migrated as T; +} + /** * Root (agent host) config properties owned by the platform itself. * diff --git a/src/vs/platform/agentHost/common/sessionConfigKeys.ts b/src/vs/platform/agentHost/common/sessionConfigKeys.ts index f241039f37772..cea7ce0316752 100644 --- a/src/vs/platform/agentHost/common/sessionConfigKeys.ts +++ b/src/vs/platform/agentHost/common/sessionConfigKeys.ts @@ -24,15 +24,27 @@ export const enum SessionConfigKey { Isolation = 'isolation', /** `'branch'` — base branch to work from. */ Branch = 'branch', - /** `'mode'` — agent execution mode (interactive / plan). */ + /** `'mode'` — agent execution mode (interactive / plan / autopilot). */ Mode = 'mode', } /** - * The set of enum values the unified permission picker understands for the - * {@link SessionConfigKey.AutoApprove} property. + * The set of enum values the unified permission picker *tolerates* for the + * {@link SessionConfigKey.AutoApprove} property when deciding whether a + * session's schema is "well-known" (and therefore handled by the dedicated + * permission picker rather than the generic per-property fallback). * - * `default` is the required baseline level; `autoApprove` and `autopilot` - * are optional (an agent may choose to advertise a subset). + * `default` is the required baseline level; `autoApprove` is the offered + * elevated level. `assisted` and `autopilot` are retained here purely for + * backward/forward compatibility so a session whose schema was resolved by an + * older or newer agent host (advertising those values) still renders the + * dedicated picker rather than disappearing. The picker itself only ever + * *offers* `default` / `autoApprove` (see the delegate's `availableLevels`). */ -export const KNOWN_AUTO_APPROVE_VALUES: ReadonlySet = new Set(['default', 'autoApprove', 'autopilot']); +export const KNOWN_AUTO_APPROVE_VALUES: ReadonlySet = new Set(['default', 'assisted', 'autoApprove', 'autopilot']); + +/** + * The set of enum values understood for the {@link SessionConfigKey.Mode} + * property: the agent execution mode axis. + */ +export const KNOWN_MODE_VALUES: ReadonlySet = new Set(['interactive', 'plan', 'autopilot']); diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index d33f640f4bcb8..df5101c9fa676 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -30,7 +30,7 @@ import { ILogService, LogLevel } from '../../../log/common/log.js'; import { IAgentHostCheckpointService } from '../../common/agentHostCheckpointService.js'; import { createAgentModelPricingMeta } from '../../common/agentModelPricing.js'; import { AgentHostConfigKey, agentHostCustomizationConfigSchema, toContainerCustomization } from '../../common/agentHostCustomizationConfig.js'; -import { AgentHostSessionSyncEnabledConfigKey, AutoApproveLevel, ISchemaProperty, SessionMode, createSchema, platformRootSchema, platformSessionSchema, schemaProperty } from '../../common/agentHostSchema.js'; +import { AgentHostSessionSyncEnabledConfigKey, AutoApproveLevel, ISchemaProperty, SessionMode, createSchema, migrateLegacyAutopilotConfig, platformRootSchema, platformSessionSchema, schemaProperty } from '../../common/agentHostSchema.js'; import { IAgentPluginManager, ISyncedCustomization } from '../../common/agentPluginManager.js'; import { AgentSession, AgentSignal, GITHUB_COPILOT_PROTECTED_RESOURCE, IAgent, IAgentCreateChatOptions, IAgentCreateSessionConfig, IAgentCreateSessionResult, IAgentDescriptor, IAgentMaterializeSessionEvent, IAgentModelInfo, IAgentResolveSessionConfigParams, IAgentSessionConfigCompletionsParams, IAgentSessionMetadata, IAgentSessionProjectInfo, IMcpNotification } from '../../common/agentService.js'; import { getEffectiveAgents } from '../../common/customAgents.js'; @@ -1240,7 +1240,7 @@ export class CopilotAgent extends Disposable implements IAgent { ...(branchProperty ? { [SessionConfigKey.Branch]: branchProperty } : {}), }); - const values = sessionSchema.validateOrDefault(params.config, { + const values = sessionSchema.validateOrDefault(migrateLegacyAutopilotConfig(params.config), { [SessionConfigKey.Isolation]: isolationValue, [SessionConfigKey.AutoApprove]: 'default' satisfies AutoApproveLevel, [SessionConfigKey.Mode]: 'interactive' satisfies SessionMode, @@ -1380,17 +1380,18 @@ export class CopilotAgent extends Disposable implements IAgent { } /** - * Translates the AHP-side `(mode, autoApprove)` pair to the Copilot - * SDK's three-mode space (`interactive` / `plan` / `autopilot`): + * Translates the AHP-side `mode` to the Copilot SDK's three-mode space + * (`interactive` / `plan` / `autopilot`). With Autopilot living on the + * `mode` axis the mapping is now direct: * - * - `mode='plan'` → SDK `plan` (auto-approval is irrelevant; the - * agent host's existing session-state auto-approval logic handles - * `plan.md` writes). - * - `mode='interactive'` + `autoApprove='autopilot'` → SDK `autopilot` - * (the SDK auto-approves all tool calls). - * - `mode='interactive'` + any other autoApprove → SDK `interactive` - * (the agent host's own auto-approval logic continues to gate tool - * calls based on `autoApprove`). + * - `mode='plan'` → SDK `plan`. + * - `mode='autopilot'` → SDK `autopilot` (autonomous, continue-until-done). + * - `mode='interactive'` → SDK `interactive`. + * + * Tool auto-approval is governed independently by the orthogonal + * `autoApprove` axis (Default / Bypass), enforced by the agent + * host's own permission handler — which the SDK still invokes even under + * autopilot mode. * * Returns `undefined` when no mode is configured for the session, so * the SDK's current mode is left untouched. @@ -1398,14 +1399,16 @@ export class CopilotAgent extends Disposable implements IAgent { private _resolveSdkMode(session: URI): CopilotSdkMode | undefined { const sessionKey = session.toString(); const mode = this._configurationService.getEffectiveValue(sessionKey, platformSessionSchema, SessionConfigKey.Mode); - if (mode === 'plan') { - return 'plan'; - } - if (mode === 'interactive') { - const autoApprove = this._configurationService.getEffectiveValue(sessionKey, platformSessionSchema, SessionConfigKey.AutoApprove); - return autoApprove === 'autopilot' ? 'autopilot' : 'interactive'; + switch (mode) { + case 'plan': + return 'plan'; + case 'autopilot': + return 'autopilot'; + case 'interactive': + return 'interactive'; + default: + return undefined; } - return undefined; } setPendingMessages(session: URI, steeringMessage: PendingMessage | undefined, _queuedMessages: readonly PendingMessage[]): void { diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index e81e71e4a462a..ff9a84d5c2dca 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -56,12 +56,10 @@ import { buildPendingEditContentUri } from './pendingEditContentStore.js'; import { McpServerStatus, type McpServerState } from '../../common/state/protocol/channels-session/state.js'; /** - * The full set of agent modes the Copilot SDK accepts. Wider than the - * {@link SessionMode} the AHP exposes — the SDK has a first-class - * `'autopilot'` mode while AHP models that as - * `mode='interactive', autoApprove='autopilot'`. The Copilot agent - * translates between the two views in {@link CopilotAgentSession.send} - * and the `session.mode_changed` listener. + * The full set of agent modes the Copilot SDK accepts. AHP now exposes the + * same three modes (`interactive` / `plan` / `autopilot`) on its `mode` axis, + * so the Copilot agent maps between the two views directly in + * {@link CopilotAgentSession.send} and the `session.mode_changed` listener. */ export type CopilotSdkMode = 'interactive' | 'plan' | 'autopilot'; type CopilotSdkAttachment = Required['attachments'][number]; @@ -1153,6 +1151,15 @@ export class CopilotAgentSession extends Disposable { } } + /** + * `true` when the session's effective `mode` is `autopilot` — the + * autonomous, continue-until-done mode in which no user is available to + * answer questions or fill in elicitation forms. + */ + private _isAutopilotMode(): boolean { + return this._configurationService.getEffectiveValue(this.sessionUri.toString(), platformSessionSchema, SessionConfigKey.Mode) === 'autopilot'; + } + async sendSteering(steeringMessage: PendingMessage): Promise { if (this._steeringMessagesInFlight.has(steeringMessage.id) || this._pendingSteeringFlips.has(steeringMessage.id)) { return; @@ -1652,7 +1659,7 @@ export class CopilotAgentSession extends Disposable { request: UserInputRequest, _invocation: { sessionId: string }, ): Promise { - const isAutopilot = this._configurationService.getEffectiveValue(this.sessionUri.toString(), platformSessionSchema, SessionConfigKey.AutoApprove) === 'autopilot'; + const isAutopilot = this._isAutopilotMode(); if (isAutopilot) { return { answer: 'The user is not available to answer your question. Choose a pragmatic option best aligned with the context of the request.', @@ -1740,7 +1747,7 @@ export class CopilotAgentSession extends Disposable { * be misleading to the MCP server. */ private async _handleElicitationRequest(context: ElicitationContext): Promise { - const isAutopilot = this._configurationService.getEffectiveValue(this.sessionUri.toString(), platformSessionSchema, SessionConfigKey.AutoApprove) === 'autopilot'; + const isAutopilot = this._isAutopilotMode(); if (isAutopilot) { return { action: 'cancel' }; } @@ -1899,6 +1906,14 @@ export class CopilotAgentSession extends Disposable { return { approved: false }; } + // Reflect the chosen implementation path on the AHP `mode` axis right + // away so the mode picker updates as soon as the user approves the + // plan (e.g. Plan → Autopilot when they pick "Implement with + // Autopilot"). The SDK also fires `session.mode_changed`, but that is + // async; writing here makes the UI update deterministic. The patch is + // idempotent, so the later event is a no-op. + this._syncAhpModeFromExitPlanAction(selectedAction); + const isAutopilot = selectedAction === 'autopilot' || selectedAction === 'autopilot_fleet'; return { approved: true, @@ -1907,6 +1922,26 @@ export class CopilotAgentSession extends Disposable { }; } + /** + * Translates an approved `exit_plan_mode` action into the AHP `mode` axis + * and writes it so the mode picker reflects the choice immediately: + * + * - `autopilot` / `autopilot_fleet` → `mode='autopilot'`. + * - `interactive` → `mode='interactive'`. + * - `exit_only` (approve plan without executing) leaves the mode untouched. + */ + private _syncAhpModeFromExitPlanAction(selectedAction: string): void { + switch (selectedAction) { + case 'autopilot': + case 'autopilot_fleet': + this._syncAhpConfigFromSdkMode('autopilot'); + break; + case 'interactive': + this._syncAhpConfigFromSdkMode('interactive'); + break; + } + } + private async _handlePreToolUse(input: PreToolUseHookInput): Promise { try { if (isEditTool(input.toolName, getToolCommand(input))) { @@ -2411,10 +2446,8 @@ export class CopilotAgentSession extends Disposable { // Sync the AHP session config when the SDK's `currentMode` changes // (e.g. after the model approves a plan, or after we set the mode - // before sending). The SDK has three modes (`interactive` / `plan` / - // `autopilot`); AHP only models `interactive` / `plan` and treats - // autopilot as `mode='interactive', autoApprove='autopilot'`, so we - // translate before writing. + // before sending). The SDK and AHP share the same three modes + // (`interactive` / `plan` / `autopilot`), so we map directly. this._register(wrapper.onSessionModeChanged(e => { this._logService.info(`[Copilot:${sessionId}] session.mode_changed: ${e.data.previousMode} -> ${e.data.newMode}`); const newMode = e.data.newMode; @@ -2549,13 +2582,15 @@ export class CopilotAgentSession extends Disposable { /** * Translates the SDK's three-mode space (`interactive` / `plan` / - * `autopilot`) to AHP's two-axis model: + * `autopilot`) to AHP's `mode` axis directly: * * - SDK `plan` → AHP `mode='plan'`. * - SDK `interactive` → AHP `mode='interactive'`. - * - SDK `autopilot` → AHP `mode='interactive', autoApprove='autopilot'`. - * Autopilot is exposed in AHP as the highest auto-approval level on - * the orthogonal `autoApprove` axis, not as a mode value. + * - SDK `autopilot` → AHP `mode='autopilot'`. + * + * Autopilot lives on the `mode` axis; the orthogonal `autoApprove` axis + * (Default / Bypass) is left untouched so the user's chosen + * approval level is preserved across SDK mode transitions. * * Patches that already match the current AHP values are still * dispatched (the reducer is a no-op in that case) but written values @@ -2569,8 +2604,7 @@ export class CopilotAgentSession extends Disposable { patch[SessionConfigKey.Mode] = 'plan'; break; case 'autopilot': - patch[SessionConfigKey.Mode] = 'interactive'; - patch[SessionConfigKey.AutoApprove] = 'autopilot'; + patch[SessionConfigKey.Mode] = 'autopilot'; break; case 'interactive': patch[SessionConfigKey.Mode] = 'interactive'; @@ -2592,16 +2626,6 @@ export class CopilotAgentSession extends Disposable { const questionId = generateUuid(); this._logService.info(`[Copilot:${this.sessionId}] exitPlanMode.request: rpcId=${requestId}, actions=[${data.actions.join(',')}], recommended=${data.recommendedAction}`); - // When the session's effective auto-approval level is `autopilot`, - // approve the plan automatically without surfacing a question to - // the user. Mirrors the "autopilot fast-path" in the Copilot CLI's - // own plan-mode handler. - const autoApprove = this._configurationService.getEffectiveValue(this.sessionUri.toString(), platformSessionSchema, SessionConfigKey.AutoApprove); - if (autoApprove === 'autopilot') { - const response = autoApproveExitPlanMode(data); - this._logService.info(`[Copilot:${this.sessionId}] exitPlanMode.request auto-accepted (autoApprove=autopilot): selectedAction=${response.selectedAction ?? '(none)'}`); - return response; - } // Resolve the plan file path so we can embed a markdown link. let planPath: string | null = null; @@ -2929,47 +2953,6 @@ export class CopilotAgentSession extends Disposable { } } -/** - * Builds the {@link IExitPlanModeResponse} used when the session is in - * autopilot and we approve the plan without user interaction. - * - * Selection priority mirrors the Copilot CLI's own autopilot handler. - * - * 1. If the SDK's `recommendedAction` is offered, take it. - * 2. Otherwise fall back to `autopilot` → `autopilot_fleet` → `interactive` - * → `exit_only`. - * 3. As a last resort, approve without picking a `selectedAction` (the SDK - * keeps `currentMode='interactive'` in that case). - * - * `autoApproveEdits: true` is set whenever the chosen action is one of the - * autopilot variants, mirroring the CLI behavior. - */ -function autoApproveExitPlanMode(data: ExitPlanModeRequest): IExitPlanModeResponse { - const choices = data.actions ?? []; - const isAutopilotAction = (action: string) => action === 'autopilot' || action === 'autopilot_fleet'; - - if (data.recommendedAction && choices.includes(data.recommendedAction)) { - const selectedAction = data.recommendedAction; - return { - approved: true, - selectedAction, - ...(isAutopilotAction(selectedAction) ? { autoApproveEdits: true } : {}), - }; - } - - for (const action of ['autopilot', 'autopilot_fleet', 'interactive', 'exit_only']) { - if (choices.includes(action)) { - return { - approved: true, - selectedAction: action, - ...(isAutopilotAction(action) ? { autoApproveEdits: true } : {}), - }; - } - } - - return { approved: true, autoApproveEdits: true }; -} - /** * Counts added/removed lines in a unified diff string. Ignores the `+++` and * `---` header rows and any non-hunk context. diff --git a/src/vs/platform/agentHost/node/sessionPermissions.ts b/src/vs/platform/agentHost/node/sessionPermissions.ts index 3059348818cb7..3172525d58bef 100644 --- a/src/vs/platform/agentHost/node/sessionPermissions.ts +++ b/src/vs/platform/agentHost/node/sessionPermissions.ts @@ -112,7 +112,7 @@ export class SessionPermissionManager extends Disposable { * when user confirmation is required. * * Checks are evaluated in order: - * 1. Session-level bypass (`autoApprove` / `autopilot` config) + * 1. Session-level bypass (`autoApprove` config) * 2. Per-tool session permissions (`permissions.allow`) * 3. Read path rules (within working directory) * 4. Write path rules (within working directory + glob patterns) @@ -168,8 +168,8 @@ export class SessionPermissionManager extends Disposable { } isSessionAutoApproveEnabled(sessionKey: ProtocolURI): boolean { - const autoApproveLevel = this._configService.getEffectiveValue(sessionKey, platformSessionSchema, SessionConfigKey.AutoApprove); - return autoApproveLevel === 'autoApprove' || autoApproveLevel === 'autopilot'; + // `autoApprove` (Bypass Approvals) auto-approves every tool call. + return this._configService.getEffectiveValue(sessionKey, platformSessionSchema, SessionConfigKey.AutoApprove) === 'autoApprove'; } // ---- Action construction (analogous to getPreConfirmActions) ------------- diff --git a/src/vs/platform/agentHost/test/common/agentHostSchema.test.ts b/src/vs/platform/agentHost/test/common/agentHostSchema.test.ts index 84c6526c78020..ecd90015f697e 100644 --- a/src/vs/platform/agentHost/test/common/agentHostSchema.test.ts +++ b/src/vs/platform/agentHost/test/common/agentHostSchema.test.ts @@ -5,7 +5,7 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; -import { createSchema, platformSessionSchema, schemaProperty, type AutoApproveLevel, type IPermissionsValue, type SessionMode } from '../../common/agentHostSchema.js'; +import { createSchema, migrateLegacyAutopilotConfig, platformSessionSchema, schemaProperty, type AutoApproveLevel, type IPermissionsValue, type SessionMode } from '../../common/agentHostSchema.js'; import { SessionConfigKey } from '../../common/sessionConfigKeys.js'; import { JsonRpcErrorCodes, ProtocolError } from '../../common/state/sessionProtocol.js'; @@ -283,11 +283,13 @@ suite('agentHostSchema', () => { suite('platformSessionSchema', () => { - test('validates the three autoApprove levels', () => { - const levels: AutoApproveLevel[] = ['default', 'autoApprove', 'autopilot']; + test('validates the autoApprove levels', () => { + const levels: AutoApproveLevel[] = ['default', 'autoApprove']; for (const level of levels) { assert.strictEqual(platformSessionSchema.validate(SessionConfigKey.AutoApprove, level), true, level); } + assert.strictEqual(platformSessionSchema.validate(SessionConfigKey.AutoApprove, 'assisted'), false); + assert.strictEqual(platformSessionSchema.validate(SessionConfigKey.AutoApprove, 'autopilot'), false); assert.strictEqual(platformSessionSchema.validate(SessionConfigKey.AutoApprove, 'bogus'), false); }); @@ -299,15 +301,48 @@ suite('agentHostSchema', () => { }); test('validates the agent modes', () => { - const modes: SessionMode[] = ['interactive', 'plan']; + const modes: SessionMode[] = ['interactive', 'plan', 'autopilot']; for (const mode of modes) { assert.strictEqual(platformSessionSchema.validate(SessionConfigKey.Mode, mode), true, mode); } - // `autopilot` is intentionally NOT in the AHP mode enum \u2014 it's - // modeled on the orthogonal `autoApprove` axis instead. - assert.strictEqual(platformSessionSchema.validate(SessionConfigKey.Mode, 'autopilot'), false); assert.strictEqual(platformSessionSchema.validate(SessionConfigKey.Mode, 'shell'), false); assert.strictEqual(platformSessionSchema.validate(SessionConfigKey.Mode, 42), false); }); }); + + // ---- legacy autopilot migration ---------------------------------------- + + suite('migrateLegacyAutopilotConfig', () => { + + test('maps legacy autoApprove=autopilot to mode=autopilot + autoApprove=default', () => { + const result = migrateLegacyAutopilotConfig({ [SessionConfigKey.AutoApprove]: 'autopilot' }); + assert.deepStrictEqual(result, { mode: 'autopilot', autoApprove: 'default' }); + }); + + test('preserves plan mode (legacy plan took precedence over autopilot)', () => { + const result = migrateLegacyAutopilotConfig({ [SessionConfigKey.Mode]: 'plan', [SessionConfigKey.AutoApprove]: 'autopilot' }); + assert.deepStrictEqual(result, { mode: 'plan', autoApprove: 'default' }); + }); + + test('overwrites a stale interactive mode with autopilot', () => { + const result = migrateLegacyAutopilotConfig({ [SessionConfigKey.Mode]: 'interactive', [SessionConfigKey.AutoApprove]: 'autopilot' }); + assert.deepStrictEqual(result, { mode: 'autopilot', autoApprove: 'default' }); + }); + + test('passes through configs without the legacy value untouched', () => { + const input = { [SessionConfigKey.AutoApprove]: 'assisted', [SessionConfigKey.Mode]: 'interactive' }; + assert.strictEqual(migrateLegacyAutopilotConfig(input), input); + }); + + test('migrated config validates against the schema', () => { + const input: Record = { [SessionConfigKey.AutoApprove]: 'autopilot' }; + const result = migrateLegacyAutopilotConfig(input)!; + assert.strictEqual(platformSessionSchema.validate(SessionConfigKey.Mode, result[SessionConfigKey.Mode]), true); + assert.strictEqual(platformSessionSchema.validate(SessionConfigKey.AutoApprove, result[SessionConfigKey.AutoApprove]), true); + }); + + test('handles undefined', () => { + assert.strictEqual(migrateLegacyAutopilotConfig(undefined), undefined); + }); + }); }); diff --git a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts index 7dc773a306adf..8c5b02cf53507 100644 --- a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts @@ -1604,8 +1604,8 @@ suite('AgentSideEffects', () => { ]); }); - test('auto-approves shell commands when autoApprove is set to autopilot', () => { - setupSessionWithConfig('autopilot'); + test('auto-approves shell commands when autoApprove is set to bypass', () => { + setupSessionWithConfig('autoApprove'); startTurn('turn-1'); disposables.add(sideEffects.registerProgressListener(agent)); @@ -1613,7 +1613,7 @@ suite('AgentSideEffects', () => { kind: 'action', session: sessionUri, action: { type: ActionType.ChatToolCallStart, turnId: 'turn-1', - toolCallId: 'tc-ap-shell-1', toolName: 'shell', displayName: 'Shell', contributor: undefined, + toolCallId: 'tc-bypass-shell-1', toolName: 'shell', displayName: 'Shell', contributor: undefined, _meta: { toolKind: undefined, language: undefined }, }, }); @@ -1621,7 +1621,7 @@ suite('AgentSideEffects', () => { kind: 'action', session: sessionUri, action: { type: ActionType.ChatToolCallReady, turnId: 'turn-1', - toolCallId: 'tc-ap-shell-1', invocationMessage: 'Run rm -rf /', toolInput: undefined, + toolCallId: 'tc-bypass-shell-1', invocationMessage: 'Run rm -rf /', toolInput: undefined, confirmed: ToolCallConfirmationReason.NotNeeded, }, }); @@ -1630,16 +1630,17 @@ suite('AgentSideEffects', () => { kind: 'pending_confirmation', session: sessionUri, state: { status: ToolCallStatus.PendingConfirmation, - toolCallId: 'tc-ap-shell-1', toolName: '', displayName: '', + toolCallId: 'tc-bypass-shell-1', toolName: '', displayName: '', invocationMessage: 'Run rm -rf /', toolInput: 'rm -rf /', confirmationTitle: undefined, edits: undefined, }, permissionKind: 'shell', permissionPath: undefined, }); - // Dangerous command would normally be blocked, but session-level auto-approve overrides + // Dangerous command would normally be blocked, but session-level + // bypass auto-approve overrides. assert.deepStrictEqual(agent.respondToPermissionCalls, [ - { requestId: 'tc-ap-shell-1', approved: true }, + { requestId: 'tc-bypass-shell-1', approved: true }, ]); }); diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index 6a1a5f317a927..4cc74377a7d71 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -2580,7 +2580,7 @@ suite('CopilotAgentSession', () => { test('autopilot auto-answers a free-form question without firing a progress event', async () => { const { runtime, signals } = await createAgentSession(disposables, { - configValues: { [SessionConfigKey.AutoApprove]: 'autopilot' }, + configValues: { [SessionConfigKey.Mode]: 'autopilot' }, }); const result = await runtime.handleUserInputRequest( @@ -2596,11 +2596,11 @@ suite('CopilotAgentSession', () => { assert.strictEqual(signals.length, 0); }); - test('autopilot does not auto-answer when autoApprove is not "autopilot"', async () => { - // Sanity check: with autoApprove=default the question must + test('autopilot does not auto-answer when mode is not "autopilot"', async () => { + // Sanity check: with mode=interactive the question must // still be surfaced as a progress event (the existing behavior). const { runtime, signals } = await createAgentSession(disposables, { - configValues: { [SessionConfigKey.AutoApprove]: 'default' }, + configValues: { [SessionConfigKey.Mode]: 'interactive' }, }); runtime.handleUserInputRequest( @@ -2775,7 +2775,7 @@ suite('CopilotAgentSession', () => { test('autopilot auto-cancels without firing a progress event', async () => { const { runtime, signals } = await createAgentSession(disposables, { - configValues: { [SessionConfigKey.AutoApprove]: 'autopilot' }, + configValues: { [SessionConfigKey.Mode]: 'autopilot' }, }); const result = await runtime.handleElicitationRequest({ @@ -3496,8 +3496,8 @@ suite('CopilotAgentSession', () => { await responsePromise; }); - test('completing the input request with autopilot resolves with approved + autopilot + autoApproveEdits', async () => { - const { session, runtime, waitForSignal } = await createAgentSession(disposables); + test('completing the input request with autopilot resolves with approved + autopilot + autoApproveEdits and syncs mode=autopilot', async () => { + const { session, runtime, waitForSignal, sessionConfigUpdates } = await createAgentSession(disposables); const responsePromise = runtime.handleExitPlanModeRequest(planRequestParams({ actions: ['autopilot', 'interactive'], recommendedAction: 'autopilot' }), { sessionId: 'test-session-1' }); const signal = await waitForSignal(s => isAction(s, ActionType.ChatInputRequested)); @@ -3513,10 +3513,14 @@ suite('CopilotAgentSession', () => { }); assert.deepStrictEqual(await responsePromise, { approved: true, selectedAction: 'autopilot', autoApproveEdits: true }); + // Picking "Implement with Autopilot" flips the AHP mode immediately. + assert.deepStrictEqual(sessionConfigUpdates, [ + { session: 'copilot:/test-session-1', patch: { mode: 'autopilot' } }, + ]); }); - test('completing the input request with interactive resolves with approved + interactive (no autoApprove)', async () => { - const { session, runtime, waitForSignal } = await createAgentSession(disposables); + test('completing the input request with interactive resolves with approved + interactive (no autoApprove) and syncs mode=interactive', async () => { + const { session, runtime, waitForSignal, sessionConfigUpdates } = await createAgentSession(disposables); const responsePromise = runtime.handleExitPlanModeRequest(planRequestParams({ actions: ['autopilot', 'interactive'], recommendedAction: 'interactive' }), { sessionId: 'test-session-1' }); const signal = await waitForSignal(s => isAction(s, ActionType.ChatInputRequested)); @@ -3532,6 +3536,9 @@ suite('CopilotAgentSession', () => { }); assert.deepStrictEqual(await responsePromise, { approved: true, selectedAction: 'interactive' }); + assert.deepStrictEqual(sessionConfigUpdates, [ + { session: 'copilot:/test-session-1', patch: { mode: 'interactive' } }, + ]); }); test('declining the input request resolves with approved=false', async () => { @@ -3706,16 +3713,16 @@ suite('CopilotAgentSession', () => { ]); }); - test('session.mode_changed → autopilot translates to mode=interactive + autoApprove=autopilot', async () => { - // The SDK has a first-class `autopilot` mode but AHP exposes it - // as the `autopilot` value on the orthogonal `autoApprove` axis. - // The translation is contained in the Copilot agent. + test('session.mode_changed → autopilot maps directly to mode=autopilot', async () => { + // The SDK and AHP share the same three-mode space; autopilot now + // lives on the `mode` axis and the `autoApprove` axis is left + // untouched. The translation is contained in the Copilot agent. const { mockSession, sessionConfigUpdates } = await createAgentSession(disposables); mockSession.fire('session.mode_changed', { previousMode: 'plan', newMode: 'autopilot' } as SessionEventPayload<'session.mode_changed'>['data']); assert.deepStrictEqual(sessionConfigUpdates, [ - { session: 'copilot:/test-session-1', patch: { mode: 'interactive', autoApprove: 'autopilot' } }, + { session: 'copilot:/test-session-1', patch: { mode: 'autopilot' } }, ]); }); @@ -3727,37 +3734,23 @@ suite('CopilotAgentSession', () => { assert.strictEqual(sessionConfigUpdates.length, 0); }); - // ---- autopilot fast-path ------------------------------------------- + // ---- no automatic plan → implementation handoff ------------------- - test('handleExitPlanModeRequest auto-accepts when autoApprove=autopilot (recommended action)', async () => { - const { runtime, signals } = await createAgentSession(disposables, { - configValues: { [SessionConfigKey.AutoApprove]: 'autopilot' }, + test('handleExitPlanModeRequest always surfaces the plan-review UI, even in autopilot mode', async () => { + const { session, runtime, waitForSignal } = await createAgentSession(disposables, { + configValues: { [SessionConfigKey.Mode]: 'autopilot' }, }); - const response = await runtime.handleExitPlanModeRequest(planRequestParams({ + const responsePromise = runtime.handleExitPlanModeRequest(planRequestParams({ actions: ['autopilot', 'interactive', 'exit_only'], recommendedAction: 'autopilot', }), { sessionId: 'test-session-1' }); - assert.deepStrictEqual(response, { approved: true, selectedAction: 'autopilot', autoApproveEdits: true }); - // User-input request should NOT be surfaced to the client. - assert.strictEqual(signals.filter(s => isAction(s, ActionType.ChatInputRequested)).length, 0); - }); - - test('handleExitPlanModeRequest auto-accepts with priority order when no recommended action available', async () => { - const { runtime } = await createAgentSession(disposables, { - configValues: { [SessionConfigKey.AutoApprove]: 'autopilot' }, - }); - - // SDK proposes a recommended action that's NOT in the offered set — - // fall back to the priority order (autopilot > autopilot_fleet > - // interactive > exit_only). - const response = await runtime.handleExitPlanModeRequest(planRequestParams({ - actions: ['interactive', 'exit_only'], - recommendedAction: 'autopilot_fleet', - }), { sessionId: 'test-session-1' }); - - assert.deepStrictEqual(response, { approved: true, selectedAction: 'interactive' }); + // There is no automatic handoff from plan into implementation: the + // user must explicitly choose an action regardless of mode. + const signal = await waitForSignal(s => isAction(s, ActionType.ChatInputRequested)); + session.respondToUserInputRequest(getInputRequest(signal).id, ChatInputResponseKind.Decline); + await responsePromise; }); test('handleExitPlanModeRequest does NOT auto-accept when autoApprove=default', async () => { diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostPermissionPickerDelegate.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostPermissionPickerDelegate.ts index 57229ba968a42..b768bf8a5252d 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostPermissionPickerDelegate.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostPermissionPickerDelegate.ts @@ -8,7 +8,7 @@ import { derived, IObservable, IReader, observableSignal } from '../../../../../ import { KNOWN_AUTO_APPROVE_VALUES, SessionConfigKey } from '../../../../../platform/agentHost/common/sessionConfigKeys.js'; import { narrowClaudePermissionMode } from '../../../../../platform/agentHost/common/claudeSessionConfigKeys.js'; import { SessionConfigPropertySchema } from '../../../../../platform/agentHost/common/state/protocol/commands.js'; -import { ChatPermissionLevel, isChatPermissionLevel } from '../../../../../workbench/contrib/chat/common/constants.js'; +import { ChatConfiguration, ChatPermissionLevel, isChatPermissionLevel } from '../../../../../workbench/contrib/chat/common/constants.js'; import { IPermissionPickerDelegate } from '../../copilotChatSessions/browser/permissionPicker.js'; import { IAgentHostSessionsProvider, isAgentHostProvider } from '../../../../common/agentHostSessionsProvider.js'; import { ISessionsProvider } from '../../../../services/sessions/common/sessionsProvider.js'; @@ -64,6 +64,18 @@ export class AgentHostPermissionPickerDelegate extends Disposable implements IPe readonly currentPermissionLevel: IObservable; readonly isApplicable: IObservable; + /** + * Agent-host sessions expose Autopilot on the orthogonal `mode` axis, so + * the permissions picker offers `Default` / `Bypass` here. + */ + readonly availableLevels: readonly ChatPermissionLevel[] = [ + ChatPermissionLevel.Default, + ChatPermissionLevel.AutoApprove, + ]; + + /** Agent-host sessions seed their default approval level from this setting. */ + readonly defaultSettingKey = ChatConfiguration.AgentSessionDefaultConfiguration; + constructor( private readonly _session: IObservable, @ISessionsProvidersService private readonly _sessionsProvidersService: ISessionsProvidersService, @@ -112,6 +124,13 @@ export class AgentHostPermissionPickerDelegate extends Disposable implements IPe return ChatPermissionLevel.Default; } const value = provider.getSessionConfig(session.sessionId)?.values[SessionConfigKey.AutoApprove]; + // Defensive: a legacy `autopilot` value on the autoApprove axis (from + // before Autopilot moved onto the mode axis) is no longer a valid + // approval level — surface it as Default rather than a level the picker + // doesn't offer. + if (value === ChatPermissionLevel.Autopilot) { + return ChatPermissionLevel.Default; + } return isChatPermissionLevel(value) ? value : ChatPermissionLevel.Default; } diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts index d3c0e0f73ae5e..3ebc5f8942354 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts @@ -12,10 +12,8 @@ import { IActionWidgetService } from '../../../../../platform/actionWidget/brows import { BaseActionViewItem } from '../../../../../base/browser/ui/actionbar/actionViewItems.js'; import { Delayer } from '../../../../../base/common/async.js'; import { Codicon } from '../../../../../base/common/codicons.js'; -import { MarkdownString } from '../../../../../base/common/htmlContent.js'; import { Disposable, DisposableMap, DisposableStore, IDisposable } from '../../../../../base/common/lifecycle.js'; import { autorun, constObservable, IObservable } from '../../../../../base/common/observable.js'; -import Severity from '../../../../../base/common/severity.js'; import { ThemeIcon } from '../../../../../base/common/themables.js'; import { localize, localize2 } from '../../../../../nls.js'; import { IActionViewItemService, type IActionViewItemFactory } from '../../../../../platform/actions/browser/actionViewItemService.js'; @@ -26,8 +24,10 @@ import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.j import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { IHoverService } from '../../../../../platform/hover/browser/hover.js'; import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js'; +import { IStorageService } from '../../../../../platform/storage/common/storage.js'; import type { SessionConfigPropertySchema, SessionConfigValueItem } from '../../../../../platform/agentHost/common/state/protocol/commands.js'; -import { ChatConfiguration } from '../../../../../workbench/contrib/chat/common/constants.js'; +import { ChatConfiguration, isChatPermissionLevel } from '../../../../../workbench/contrib/chat/common/constants.js'; +import { maybeConfirmElevatedPermissionLevel } from '../../../../../workbench/contrib/chat/common/chatPermissionWarnings.js'; import { ChatContextKeyExprs, ChatContextKeys } from '../../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../../workbench/common/contributions.js'; import { type IChatInputPickerOptions } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputPickerActionItem.js'; @@ -150,19 +150,6 @@ function renderPickerTrigger(slot: HTMLElement, disabled: boolean, disposables: } // Track whether auto-approve warnings have been shown this VS Code session -const shownAutoApproveWarnings = new Set(); - -function hasShownAutoApproveWarning(value: string): boolean { - if (shownAutoApproveWarnings.has(value)) { - return true; - } - // Confirming Autopilot implies the user accepted the Bypass risks too - if (value === 'autoApprove' && shownAutoApproveWarnings.has('autopilot')) { - return true; - } - return false; -} - /** * Marks bypass/autopilot as disabled if enterprise policy restricts * auto-approval. Returns the items and policy state. @@ -180,54 +167,18 @@ function applyAutoApproveFiltering( } /** - * Shows a confirmation dialog for elevated auto-approve levels. - * Returns true if confirmed or if the warning was already shown this session. + * Shows a confirmation dialog for elevated auto-approve levels (Bypass + * or legacy Autopilot). Delegates to the shared + * {@link maybeConfirmElevatedPermissionLevel} so the copy, icons, and + * "Don't show again" persistence stay consistent across every permission + * picker. Returns `true` when confirmed (or not elevated), `false` when the + * user cancels. */ -async function confirmAutoApproveLevel(value: string, dialogService: IDialogService): Promise { - if (hasShownAutoApproveWarning(value)) { +async function confirmAutoApproveLevel(value: string, dialogService: IDialogService, storageService: IStorageService): Promise { + if (!isChatPermissionLevel(value)) { return true; } - - const isAutopilot = value === 'autopilot'; - const result = await dialogService.prompt({ - type: Severity.Warning, - message: isAutopilot - ? localize('agentHostAutoApprove.autopilot.warning.title', "Enable Autopilot?") - : localize('agentHostAutoApprove.bypass.warning.title', "Enable Bypass Approvals?"), - buttons: [ - { - label: localize('agentHostAutoApprove.warning.confirm', "Enable"), - run: () => true, - }, - { - label: localize('agentHostAutoApprove.warning.cancel', "Cancel"), - run: () => false, - }, - ], - custom: { - icon: isAutopilot ? Codicon.rocket : Codicon.warning, - markdownDetails: [{ - markdown: new MarkdownString( - localize( - 'agentHostAutoApprove.warning.detailWithDefaultSetting', - "{0}\n\nTo make this the starting permission level for new chat sessions, change the [{1}](command:workbench.action.openSettings?%5B%22{1}%22%5D) setting.", - isAutopilot - ? localize('agentHostAutoApprove.autopilot.warning.detail', "Autopilot will auto-approve all tool calls and continue working autonomously until the task is complete. This includes terminal commands, file edits, and external tool calls. The agent will make decisions on your behalf without asking for confirmation.\n\nYou can stop the agent at any time by clicking the stop button. This applies to the current session only.") - : localize('agentHostAutoApprove.bypass.warning.detail', "Bypass Approvals will auto-approve all tool calls without asking for confirmation. This includes file edits, terminal commands, and external tool calls."), - ChatConfiguration.DefaultPermissionLevel, - ), - { isTrusted: { enabledCommands: ['workbench.action.openSettings'] } }, - ), - }], - }, - }); - - if (result.result !== true) { - return false; - } - - shownAutoApproveWarnings.add(value); - return true; + return maybeConfirmElevatedPermissionLevel(value, dialogService, storageService, { defaultSettingKey: ChatConfiguration.AgentSessionDefaultConfiguration }); } /** @@ -257,6 +208,7 @@ export class AgentHostSessionConfigPicker extends Disposable { @ISessionsProvidersService protected readonly _sessionsProvidersService: ISessionsProvidersService, @ITelemetryService protected readonly _telemetryService: ITelemetryService, @IWorkbenchLayoutService protected readonly _layoutService: IWorkbenchLayoutService, + @IStorageService protected readonly _storageService: IStorageService, ) { super(); @@ -475,7 +427,7 @@ export class AgentHostSessionConfigPicker extends Disposable { }); if (isAutoApproveProperty && (item.value === 'autoApprove' || item.value === 'autopilot')) { - const confirmed = await confirmAutoApproveLevel(item.value, this._dialogService); + const confirmed = await confirmAutoApproveLevel(item.value, this._dialogService, this._storageService); if (!confirmed) { return; } diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 2374ef47432a9..32729fb6f977c 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -22,7 +22,8 @@ import { AgentSession, IAgentConnection, IAgentSessionMetadata } from '../../../ import { buildSessionChangesetUri } from '../../../../../platform/agentHost/common/changesetUri.js'; import { buildAnnotationsUri } from '../../../../../platform/agentHost/common/annotationsUri.js'; import { getEffectiveAgents } from '../../../../../platform/agentHost/common/customAgents.js'; -import { KNOWN_AUTO_APPROVE_VALUES, SessionConfigKey } from '../../../../../platform/agentHost/common/sessionConfigKeys.js'; +import { KNOWN_MODE_VALUES, SessionConfigKey } from '../../../../../platform/agentHost/common/sessionConfigKeys.js'; +import { migrateLegacyAutopilotConfig } from '../../../../../platform/agentHost/common/agentHostSchema.js'; import type { IAgentSubscription } from '../../../../../platform/agentHost/common/state/agentSubscription.js'; import { ResolveSessionConfigResult } from '../../../../../platform/agentHost/common/state/protocol/commands.js'; import { AgentCustomization, AgentSelection, ChangesSummary, type ChangesetFile, Customization, CustomizationType, ModelSelection, SessionStatus as ProtocolSessionStatus, RootConfigState, RootState, SessionActiveClient, SessionState, SessionSummary, type Changeset } from '../../../../../platform/agentHost/common/state/protocol/state.js'; @@ -37,7 +38,7 @@ import { IAgentHostActiveClientService } from '../../../../../workbench/contrib/ import { IChatWidgetService } from '../../../../../workbench/contrib/chat/browser/chat.js'; import { IChatSendRequestOptions, IChatService } from '../../../../../workbench/contrib/chat/common/chatService/chatService.js'; import { IChatSessionFileChange, IChatSessionFileChange2, IChatSessionsService } from '../../../../../workbench/contrib/chat/common/chatSessionsService.js'; -import { ChatAgentLocation, ChatConfiguration, ChatModeKind, ChatPermissionLevel } from '../../../../../workbench/contrib/chat/common/constants.js'; +import { ChatAgentLocation, ChatConfiguration, ChatModeKind, ChatPermissionLevel, isChatPermissionLevel, type IAgentSessionDefaultConfiguration } from '../../../../../workbench/contrib/chat/common/constants.js'; import { ILanguageModelChatMetadataAndIdentifier, ILanguageModelsService } from '../../../../../workbench/contrib/chat/common/languageModels.js'; import { buildMutableConfigSchema, IAgentHostMcpServer, IAgentHostSessionsProvider, resolvedConfigsEqual } from '../../../../common/agentHostSessionsProvider.js'; import { agentHostSessionWorkspaceKey } from '../../../../common/agentHostSessionWorkspace.js'; @@ -59,11 +60,18 @@ function isSafeSessionConfigKey(property: string): boolean { } function normalizeAutoApproveValue(value: unknown, policyRestricted: boolean): ChatPermissionLevel | undefined { - if (typeof value !== 'string' || !KNOWN_AUTO_APPROVE_VALUES.has(value)) { + // `KNOWN_AUTO_APPROVE_VALUES` is intentionally tolerant of forward/legacy + // compatibility values (e.g. `assisted`) that are not real + // `ChatPermissionLevel`s. Validate against the enum here so this function + // never returns a value outside its declared contract. + if (!isChatPermissionLevel(value)) { return undefined; } - const normalized = value as ChatPermissionLevel; - if (policyRestricted && (normalized === ChatPermissionLevel.AutoApprove || normalized === ChatPermissionLevel.Autopilot)) { + const normalized = value; + // Bypass and (legacy) Autopilot auto-approve at least some + // tool calls, so clamp them to Default when enterprise policy disables + // global auto-approval. + if (policyRestricted && normalized !== ChatPermissionLevel.Default) { return ChatPermissionLevel.Default; } return normalized; @@ -74,7 +82,7 @@ function isAutoApprovePolicyRestricted(configurationService: IConfigurationServi } function normalizeSessionConfigValue(property: string, value: unknown, policyRestricted: boolean): unknown { - if (property === SessionConfigKey.AutoApprove && policyRestricted && (value === ChatPermissionLevel.AutoApprove || value === ChatPermissionLevel.Autopilot)) { + if (property === SessionConfigKey.AutoApprove && policyRestricted && value !== ChatPermissionLevel.Default) { return ChatPermissionLevel.Default; } return value; @@ -1743,38 +1751,58 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement * Initial session-config values applied to a brand-new agent-host session * before its schema is resolved. Values are seeded from the profile-scoped * remembered session-config map (plus legacy isolation fallback) and then - * normalized against policy/feature constraints. For `autoApprove`, - * `chat.permissions.default` takes precedence over remembered values. + * normalized against policy/feature constraints. + * + * The agent-host defaults are controlled by the single + * `chat.agentSessions.defaultConfiguration` object setting (with `mode` and + * `approvals` properties), which takes precedence over remembered values. + * The local-only `chat.permissions.default` setting is intentionally NOT + * consulted here. * * If enterprise policy disables global auto-approval - * (`chat.tools.global.autoApprove` policy value `false`), the seed is - * clamped to `default` so the agent host never starts in an elevated + * (`chat.tools.global.autoApprove` policy value `false`), the approval seed + * is clamped to `default` so the agent host never starts in an elevated * permission level the user is not allowed to pick. */ protected _initialNewSessionConfig(): Record | undefined { const config = Object.create(null) as Record; const policyRestricted = isAutoApprovePolicyRestricted(this._baseConfigurationService); - // Seed session config values from the last user picks. + // Seed session config values from the last user picks, then migrate any + // legacy `autoApprove='autopilot'` remembered value into the new + // `mode='autopilot'` shape *first*, so the configured agent-host + // defaults applied below cleanly take precedence over it (rather than + // the migration overwriting a configured mode afterwards). const rememberedValues = this._storageService.getObject>(STORAGE_KEY_REMEMBERED_SESSION_CONFIG_VALUES, StorageScope.PROFILE, {}); for (const [property, value] of Object.entries(rememberedValues)) { if (typeof value === 'string' && isSafeSessionConfigKey(property)) { config[property] = value; } } + const remembered = migrateLegacyAutopilotConfig(config); + + // Configured agent-host defaults (a single object setting controlling + // both axes) win over remembered values. + const configuredDefaults = this._baseConfigurationService.getValue(ChatConfiguration.AgentSessionDefaultConfiguration); - const configured = this._baseConfigurationService.getValue(ChatConfiguration.DefaultPermissionLevel); - const normalizedConfiguredAutoApprove = normalizeAutoApproveValue(configured, policyRestricted); - const normalizedRememberedAutoApprove = normalizeAutoApproveValue(config[SessionConfigKey.AutoApprove], policyRestricted); + // Approval axis. + const normalizedConfiguredAutoApprove = normalizeAutoApproveValue(configuredDefaults?.approvals, policyRestricted); + const normalizedRememberedAutoApprove = normalizeAutoApproveValue(remembered[SessionConfigKey.AutoApprove], policyRestricted); if (normalizedConfiguredAutoApprove) { - config[SessionConfigKey.AutoApprove] = normalizedConfiguredAutoApprove; + remembered[SessionConfigKey.AutoApprove] = normalizedConfiguredAutoApprove; } else if (normalizedRememberedAutoApprove) { - config[SessionConfigKey.AutoApprove] = normalizedRememberedAutoApprove; + remembered[SessionConfigKey.AutoApprove] = normalizedRememberedAutoApprove; } else { - delete config[SessionConfigKey.AutoApprove]; + delete remembered[SessionConfigKey.AutoApprove]; + } + + // Mode axis. + const configuredMode = configuredDefaults?.mode; + if (typeof configuredMode === 'string' && KNOWN_MODE_VALUES.has(configuredMode)) { + remembered[SessionConfigKey.Mode] = configuredMode; } - return Object.keys(config).length > 0 ? config : undefined; + return Object.keys(remembered).length > 0 ? remembered : undefined; } // -- Dynamic session config ---------------------------------------------- diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostPermissionPickerDelegate.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostPermissionPickerDelegate.test.ts index 7bfc956c295a5..bc3910b220c84 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostPermissionPickerDelegate.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostPermissionPickerDelegate.test.ts @@ -31,7 +31,7 @@ function makeWellKnownConfig(value: string | undefined): ResolveSessionConfigRes title: 'Auto Approve', description: '', type: 'string', - enum: ['default', 'autoApprove', 'autopilot'], + enum: ['default', 'autoApprove'], sessionMutable: true, }, }, @@ -122,15 +122,20 @@ suite('AgentHostPermissionPickerDelegate', () => { assert.strictEqual(delegate.currentPermissionLevel.get(), ChatPermissionLevel.AutoApprove); - provider.config = makeWellKnownConfig('autopilot'); - provider.fireChange(); - assert.strictEqual(delegate.currentPermissionLevel.get(), ChatPermissionLevel.Autopilot); - provider.config = makeWellKnownConfig('default'); provider.fireChange(); assert.strictEqual(delegate.currentPermissionLevel.get(), ChatPermissionLevel.Default); }); + test('maps a legacy autoApprove=autopilot value to Default (Autopilot moved onto the mode axis)', () => { + const { delegate } = setup(store, makeActiveSession(), 'autopilot'); + + // `autopilot` is no longer a valid approval level — the picker does not + // offer it, so the chip must surface Default rather than a level it + // cannot render. + assert.strictEqual(delegate.currentPermissionLevel.get(), ChatPermissionLevel.Default); + }); + test('falls back to Default when the stored value is unrecognized', () => { const { delegate } = setup(store, makeActiveSession(), 'something-else'); @@ -141,11 +146,11 @@ suite('AgentHostPermissionPickerDelegate', () => { const { delegate, provider } = setup(store, makeActiveSession(), 'default'); delegate.setPermissionLevel(ChatPermissionLevel.AutoApprove); - delegate.setPermissionLevel(ChatPermissionLevel.Autopilot); + delegate.setPermissionLevel(ChatPermissionLevel.Default); assert.deepStrictEqual(provider.setCalls, [ [SESSION_ID, 'autoApprove', 'autoApprove'], - [SESSION_ID, 'autoApprove', 'autopilot'], + [SESSION_ID, 'autoApprove', 'default'], ]); }); @@ -186,15 +191,19 @@ suite('isWellKnownAutoApproveSchema', () => { title: 'Auto Approve', description: 'desc', type: 'string', - enum: ['default', 'autoApprove', 'autopilot'], + enum: ['default', 'autoApprove'], ...overrides, } as SessionConfigPropertySchema; } - test('matches the canonical three-value enum', () => { + test('matches the canonical two-value enum', () => { assert.strictEqual(isWellKnownAutoApproveSchema(schema()), true); }); + test('still accepts a legacy enum that contains "autopilot" for backward compatibility', () => { + assert.strictEqual(isWellKnownAutoApproveSchema(schema({ enum: ['default', 'autoApprove', 'autopilot'] })), true); + }); + test('matches a subset that still contains "default"', () => { assert.strictEqual(isWellKnownAutoApproveSchema(schema({ enum: ['default', 'autoApprove'] })), true); assert.strictEqual(isWellKnownAutoApproveSchema(schema({ enum: ['default'] })), true); diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index 95f858110499c..917c11f82c26a 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -1316,11 +1316,11 @@ suite('LocalAgentHostSessionsProvider', () => { assert.strictEqual(provider.getSessionConfig(session.sessionId), undefined); }); - test('createNewSession seeds autoApprove from chat.permissions.default and forwards it to resolveSessionConfig', async () => { + test('createNewSession seeds autoApprove from chat.agentSessions.defaultConfiguration and forwards it to resolveSessionConfig', async () => { const config = new TestConfigurationService(); - await config.setUserConfiguration('chat.permissions.default', 'autoApprove'); + await config.setUserConfiguration('chat.agentSessions.defaultConfiguration', { approvals: 'autoApprove' }); agentHost.resolveSessionConfigResult = { - schema: { type: 'object', properties: { autoApprove: { type: 'string', enum: ['default', 'autoApprove', 'autopilot'], title: 'Auto-approve' } } }, + schema: { type: 'object', properties: { autoApprove: { type: 'string', enum: ['default', 'autoApprove'], title: 'Auto-approve' } } }, values: { autoApprove: 'autoApprove' }, }; const provider = createProvider(disposables, agentHost, undefined, { configurationService: config }); @@ -1336,9 +1336,25 @@ suite('LocalAgentHostSessionsProvider', () => { }); }); + test('createNewSession seeds mode from chat.agentSessions.defaultConfiguration and forwards it to resolveSessionConfig', async () => { + const config = new TestConfigurationService(); + await config.setUserConfiguration('chat.agentSessions.defaultConfiguration', { mode: 'autopilot' }); + const provider = createProvider(disposables, agentHost, undefined, { configurationService: config }); + const session = provider.createNewSession(URI.parse('file:///home/user/project'), provider.sessionTypes[0].id); + await waitForSessionConfig(provider, session.sessionId, c => c?.values.mode === 'autopilot'); + + assert.deepStrictEqual({ + seededImmediately: provider.getSessionConfig(session.sessionId)?.values.mode, + forwardedToAgentHost: agentHost.resolveSessionConfigRequests.at(-1)?.config?.mode, + }, { + seededImmediately: 'autopilot', + forwardedToAgentHost: 'autopilot', + }); + }); + test('createNewSession forwards seeded config to eager createSession', async () => { const config = new TestConfigurationService(); - await config.setUserConfiguration('chat.permissions.default', 'autoApprove'); + await config.setUserConfiguration('chat.agentSessions.defaultConfiguration', { approvals: 'autoApprove' }); const provider = createProvider(disposables, agentHost, undefined, { configurationService: config }); provider.createNewSession(URI.parse('file:///home/user/project'), provider.sessionTypes[0].id); await timeout(0); @@ -1346,7 +1362,7 @@ suite('LocalAgentHostSessionsProvider', () => { assert.deepStrictEqual(agentHost.createSessionConfigs[0]?.config, { autoApprove: 'autoApprove' }); }); - test('createNewSession does not seed autoApprove when chat.permissions.default is the default value', () => { + test('createNewSession does not seed autoApprove when chat.agentSessions.defaultConfiguration approvals is the default value', () => { const provider = createProvider(disposables, agentHost); const session = provider.createNewSession(URI.parse('file:///home/user/project'), provider.sessionTypes[0].id); @@ -1361,7 +1377,7 @@ suite('LocalAgentHostSessionsProvider', () => { test('createNewSession clamps seeded autoApprove to default when policy disables global auto-approve', async () => { const config = createPolicyRestrictedConfigurationService(); - await config.setUserConfiguration('chat.permissions.default', 'autopilot'); + await config.setUserConfiguration('chat.agentSessions.defaultConfiguration', { approvals: 'autoApprove' }); const provider = createProvider(disposables, agentHost, undefined, { configurationService: config }); const session = provider.createNewSession(URI.parse('file:///home/user/project'), provider.sessionTypes[0].id); @@ -1421,21 +1437,21 @@ suite('LocalAgentHostSessionsProvider', () => { }); }); - test('createNewSession gives chat.permissions.default precedence over remembered autoApprove while normalizing by policy', async () => { + test('createNewSession gives chat.agentSessions.defaultConfiguration precedence over remembered autoApprove while normalizing by policy', async () => { const storageService = disposables.add(new InMemoryStorageService()); storageService.store(STORAGE_KEY_REMEMBERED_SESSION_CONFIG_VALUES, JSON.stringify({ - [SessionConfigKey.AutoApprove]: 'autopilot', + [SessionConfigKey.AutoApprove]: 'autoApprove', }), StorageScope.PROFILE, StorageTarget.MACHINE); // Case 1: policy restricts auto-approve — setting 'autoApprove' is clamped to 'default' const policyRestrictedConfig = createPolicyRestrictedConfigurationService(); - await policyRestrictedConfig.setUserConfiguration('chat.permissions.default', 'autoApprove'); + await policyRestrictedConfig.setUserConfiguration('chat.agentSessions.defaultConfiguration', { approvals: 'autoApprove' }); const policyRestrictedProvider = createProvider(disposables, agentHost, undefined, { configurationService: policyRestrictedConfig, storageService }); policyRestrictedProvider.createNewSession(URI.parse('file:///home/user/project'), policyRestrictedProvider.sessionTypes[0].id); - // Case 2: configured 'default' wins over remembered 'autopilot' + // Case 2: configured 'default' wins over remembered 'autoApprove' const configuredDefaultConfig = new TestConfigurationService(); - await configuredDefaultConfig.setUserConfiguration('chat.permissions.default', 'default'); + await configuredDefaultConfig.setUserConfiguration('chat.agentSessions.defaultConfiguration', { approvals: 'default' }); const configuredDefaultProvider = createProvider(disposables, agentHost, undefined, { configurationService: configuredDefaultConfig, storageService }); configuredDefaultProvider.createNewSession(URI.parse('file:///home/user/project'), configuredDefaultProvider.sessionTypes[0].id); @@ -1450,6 +1466,21 @@ suite('LocalAgentHostSessionsProvider', () => { }); }); + test('createNewSession migrates a remembered legacy autoApprove=autopilot to mode=autopilot', async () => { + const storageService = disposables.add(new InMemoryStorageService()); + storageService.store(STORAGE_KEY_REMEMBERED_SESSION_CONFIG_VALUES, JSON.stringify({ + [SessionConfigKey.AutoApprove]: 'autopilot', + }), StorageScope.PROFILE, StorageTarget.MACHINE); + const provider = createProvider(disposables, agentHost, undefined, { storageService }); + provider.createNewSession(URI.parse('file:///home/user/project'), provider.sessionTypes[0].id); + await timeout(0); + + assert.deepStrictEqual(agentHost.resolveSessionConfigRequests.at(-1)?.config, { + mode: 'autopilot', + autoApprove: 'default', + }); + }); + test('getSessionByResource resolves current new session without listing it', () => { const provider = createProvider(disposables, agentHost); const workspaceUri = URI.parse('file:///home/user/my-project'); diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/mobilePermissionPicker.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/mobilePermissionPicker.ts index e3da053d8cb84..45b2b2771429c 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/mobilePermissionPicker.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/mobilePermissionPicker.ts @@ -14,16 +14,17 @@ import { IStorageService } from '../../../../../platform/storage/common/storage. import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js'; import { IWorkbenchLayoutService } from '../../../../../workbench/services/layout/browser/layoutService.js'; import { ChatConfiguration, ChatPermissionLevel } from '../../../../../workbench/contrib/chat/common/constants.js'; -import { IPermissionPickerDelegate, PermissionPicker } from './permissionPicker.js'; +import { DEFAULT_PERMISSION_LEVELS, getPermissionLevelMeta, IPermissionPickerDelegate, PermissionPicker } from './permissionPicker.js'; import { isPhoneLayout } from '../../../../browser/parts/mobile/mobileLayout.js'; import { IMobilePickerSheetItem, showMobilePickerSheet } from '../../../../browser/parts/mobile/mobilePickerSheet.js'; const LEARN_MORE_ID = 'learn-more'; /** - * Phone variant of {@link PermissionPicker} that surfaces the - * Default/Bypass/Autopilot choice as a {@link showMobilePickerSheet} - * bottom sheet rather than the desktop action-widget popup. + * Phone variant of {@link PermissionPicker} that surfaces the available + * approval levels (provided by the delegate, defaulting to + * Default/Bypass/Autopilot) as a {@link showMobilePickerSheet} bottom sheet + * rather than the desktop action-widget popup. * * Falls back to the inherited dropdown when the viewport is not phone * (e.g. user resized past the breakpoint after the picker rendered). @@ -54,31 +55,20 @@ export class MobilePermissionPicker extends PermissionPicker { const policyRestricted = this.configurationService.inspect(ChatConfiguration.GlobalAutoApprove).policyValue === false; - const items: IMobilePickerSheetItem[] = [ - { - id: ChatPermissionLevel.Default, - label: localize('permissions.default', "Default Approvals"), - description: localize('permissions.default.subtext', "Copilot uses your configured settings"), - icon: Codicon.shield, - checked: this._currentLevel === ChatPermissionLevel.Default, - }, - { - id: ChatPermissionLevel.AutoApprove, - label: localize('permissions.autoApprove', "Bypass Approvals"), - description: localize('permissions.autoApprove.subtext', "All tool calls are auto-approved"), - icon: Codicon.warning, - checked: this._currentLevel === ChatPermissionLevel.AutoApprove, - disabled: policyRestricted, - }, - { - id: ChatPermissionLevel.Autopilot, - label: localize('permissions.autopilot', "Autopilot (Preview)"), - description: localize('permissions.autopilot.subtext', "Autonomously iterates from start to finish"), - icon: Codicon.rocket, - checked: this._currentLevel === ChatPermissionLevel.Autopilot, - disabled: policyRestricted, - }, - ]; + const levels = this._delegate.availableLevels ?? DEFAULT_PERMISSION_LEVELS; + const items: IMobilePickerSheetItem[] = levels.map(level => { + const meta = getPermissionLevelMeta(level); + return { + id: level, + label: meta.label, + description: meta.detail, + icon: meta.icon, + checked: this._currentLevel === level, + // Default is never policy-restricted; elevated levels are + // disabled when enterprise policy turns off auto-approval. + ...(level !== ChatPermissionLevel.Default && policyRestricted ? { disabled: true } : {}), + } satisfies IMobilePickerSheetItem; + }); items.push({ id: LEARN_MORE_ID, label: localize('permissions.learnMore', "Learn more about permissions"), diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/permissionPicker.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/permissionPicker.ts index ad788589ac35e..cd21caf92a09e 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/permissionPicker.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/permissionPicker.ts @@ -52,6 +52,21 @@ export interface IPermissionPickerDelegate { */ readonly isApplicable?: IObservable; + /** + * The ordered set of permission levels the picker should offer. When + * omitted, the picker offers the default Copilot set + * (`Default` / `Bypass` / `Autopilot`). Agent-host sessions override this + * to offer `Default` / `Bypass`. + */ + readonly availableLevels?: readonly ChatPermissionLevel[]; + + /** + * The setting id the elevated-level warning dialog links to as "make this + * the default". Defaults to `chat.permissions.default`; agent-host sessions + * pass `chat.agentSessions.defaultConfiguration`. + */ + readonly defaultSettingKey?: string; + /** * Called after the user selects a level (and any required confirmation * dialog has been accepted). @@ -59,6 +74,45 @@ export interface IPermissionPickerDelegate { setPermissionLevel(level: ChatPermissionLevel): void; } +export interface IPermissionLevelMeta { + readonly label: string; + readonly detail: string; + readonly icon: ThemeIcon; + readonly hover?: string; +} + +/** Default level set offered when a delegate does not specify {@link IPermissionPickerDelegate.availableLevels}. */ +export const DEFAULT_PERMISSION_LEVELS: readonly ChatPermissionLevel[] = [ + ChatPermissionLevel.Default, + ChatPermissionLevel.AutoApprove, + ChatPermissionLevel.Autopilot, +]; + +export function getPermissionLevelMeta(level: ChatPermissionLevel): IPermissionLevelMeta { + switch (level) { + case ChatPermissionLevel.AutoApprove: + return { + label: localize('permissions.autoApprove', "Bypass Approvals"), + detail: localize('permissions.autoApprove.subtext', "All tool calls are auto-approved"), + icon: Codicon.warning, + }; + case ChatPermissionLevel.Autopilot: + return { + label: localize('permissions.autopilot', "Autopilot (Preview)"), + detail: localize('permissions.autopilot.subtext', "Autonomously iterates from start to finish"), + icon: Codicon.rocket, + hover: localize('permissions.autopilot.description', "Auto-approve all tool calls and continue until the task is done. Autopilot may increase costs."), + }; + case ChatPermissionLevel.Default: + default: + return { + label: localize('permissions.default', "Default Approvals"), + detail: localize('permissions.default.subtext', "Copilot uses your configured settings"), + icon: Codicon.shield, + }; + } +} + interface IPermissionItem { readonly level?: ChatPermissionLevel; readonly label: string; @@ -158,50 +212,27 @@ export class PermissionPicker extends Disposable { const policyRestricted = this.configurationService.inspect(ChatConfiguration.GlobalAutoApprove).policyValue === false; - const items: IActionListItem[] = [ - { + const levels = this._delegate.availableLevels ?? DEFAULT_PERMISSION_LEVELS; + const items: IActionListItem[] = levels.map(level => { + const meta = getPermissionLevelMeta(level); + // Default is never policy-restricted; elevated levels are disabled + // when enterprise policy turns off global auto-approval. + const disabled = level !== ChatPermissionLevel.Default && policyRestricted; + return { kind: ActionListItemKind.Action, - group: { kind: ActionListItemKind.Header, title: '', icon: Codicon.shield }, + group: { kind: ActionListItemKind.Header, title: '', icon: meta.icon }, item: { - level: ChatPermissionLevel.Default, - label: localize('permissions.default', "Default Approvals"), - icon: Codicon.shield, - checked: this._currentLevel === ChatPermissionLevel.Default, + level, + label: meta.label, + icon: meta.icon, + checked: this._currentLevel === level, }, - label: localize('permissions.default', "Default Approvals"), - detail: localize('permissions.default.subtext', "Copilot uses your configured settings"), - disabled: false, - }, - { - kind: ActionListItemKind.Action, - group: { kind: ActionListItemKind.Header, title: '', icon: Codicon.warning }, - item: { - level: ChatPermissionLevel.AutoApprove, - label: localize('permissions.autoApprove', "Bypass Approvals"), - icon: Codicon.warning, - checked: this._currentLevel === ChatPermissionLevel.AutoApprove, - }, - label: localize('permissions.autoApprove', "Bypass Approvals"), - detail: localize('permissions.autoApprove.subtext', "All tool calls are auto-approved"), - disabled: policyRestricted, - }, - { - kind: ActionListItemKind.Action, - group: { kind: ActionListItemKind.Header, title: '', icon: Codicon.rocket }, - item: { - level: ChatPermissionLevel.Autopilot, - label: localize('permissions.autopilot', "Autopilot (Preview)"), - icon: Codicon.rocket, - checked: this._currentLevel === ChatPermissionLevel.Autopilot, - }, - label: localize('permissions.autopilot', "Autopilot (Preview)"), - detail: localize('permissions.autopilot.subtext', "Autonomously iterates from start to finish"), - hover: { - content: localize('permissions.autopilot.description', "Auto-approve all tool calls and continue until the task is done. Autopilot may increase costs."), - }, - disabled: policyRestricted, - }, - ]; + label: meta.label, + detail: meta.detail, + ...(meta.hover ? { hover: { content: meta.hover } } : {}), + disabled, + } satisfies IActionListItem; + }); items.push({ kind: ActionListItemKind.Separator, @@ -251,7 +282,7 @@ export class PermissionPicker extends Disposable { } protected async _selectLevel(level: ChatPermissionLevel): Promise { - if (!await maybeConfirmElevatedPermissionLevel(level, this.dialogService, this.storageService)) { + if (!await maybeConfirmElevatedPermissionLevel(level, this.dialogService, this.storageService, { defaultSettingKey: this._delegate.defaultSettingKey })) { reportNewChatPickerClosed(this.telemetryService, { id: 'NewChatPermissionPicker', name: 'NewChatPermissionPicker', @@ -285,28 +316,13 @@ export class PermissionPicker extends Disposable { } dom.clearNode(trigger); - let icon: ThemeIcon; - let label: string; - switch (this._currentLevel) { - case ChatPermissionLevel.Autopilot: - icon = Codicon.rocket; - label = localize('permissions.autopilot.label', "Autopilot (Preview)"); - break; - case ChatPermissionLevel.AutoApprove: - icon = Codicon.warning; - label = localize('permissions.autoApprove.label', "Bypass Approvals"); - break; - default: - icon = Codicon.shield; - label = localize('permissions.default.label', "Default Approvals"); - break; - } + const meta = getPermissionLevelMeta(this._currentLevel); - dom.append(trigger, renderIcon(icon)); + dom.append(trigger, renderIcon(meta.icon)); const labelSpan = dom.append(trigger, dom.$('span.sessions-chat-dropdown-label')); - labelSpan.textContent = label; + labelSpan.textContent = meta.label; - trigger.ariaLabel = localize('permissionPicker.triggerAriaLabel', "Pick Permission Level, {0}", label); + trigger.ariaLabel = localize('permissionPicker.triggerAriaLabel', "Pick Permission Level, {0}", meta.label); trigger.classList.toggle('warning', this._currentLevel === ChatPermissionLevel.Autopilot); trigger.classList.toggle('info', this._currentLevel === ChatPermissionLevel.AutoApprove); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts index 99dd728444d8f..dd11c1b5e2f00 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts @@ -27,11 +27,14 @@ import { ActionListItemKind, IActionListDelegate, IActionListItem } from '../../ import { IActionWidgetService } from '../../../../../../platform/actionWidget/browser/actionWidget.js'; import { IHoverService } from '../../../../../../platform/hover/browser/hover.js'; import { IOpenerService } from '../../../../../../platform/opener/common/opener.js'; +import { IDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; +import { IStorageService } from '../../../../../../platform/storage/common/storage.js'; import type { IAction } from '../../../../../../base/common/actions.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js'; import type { IChatWidget } from '../../chat.js'; -import { ChatConfiguration } from '../../../common/constants.js'; +import { ChatConfiguration, ChatPermissionLevel, isChatPermissionLevel } from '../../../common/constants.js'; +import { maybeConfirmElevatedPermissionLevel } from '../../../common/chatPermissionWarnings.js'; import { isUntitledChatSession } from '../../../common/model/chatUri.js'; import { IAgentHostSessionWorkingDirectoryResolver } from './agentHostSessionWorkingDirectoryResolver.js'; import { IAgentHostNewSessionFolderService } from './agentHostNewSessionFolderService.js'; @@ -84,7 +87,9 @@ function isAutoApprovePolicyRestricted(configurationService: IConfigurationServi } function normalizeConfigValue(property: string, value: string, policyRestricted: boolean): string { - if (property === SessionConfigKey.AutoApprove && policyRestricted && (value === 'autoApprove' || value === 'autopilot')) { + // Assisted, Bypass, and (legacy) Autopilot all auto-approve at least some + // tool calls, so clamp anything but Default when policy disables auto-approve. + if (property === SessionConfigKey.AutoApprove && policyRestricted && value !== 'default') { return 'default'; } return value; @@ -187,6 +192,26 @@ export function isClaimedByDedicatedPicker(property: string, schema: SessionConf return WELL_KNOWN_PICKER_PROPERTIES.has(property); } +/** + * Resolves which config value a chat-input chip should display, given the + * server's session-state value and the workbench overlay value. + * + * Precedence depends on the session lifecycle: + * - Untitled (pre-send): the workbench overlay is authoritative — it reflects + * synchronous chip edits before the provisional backend echoes them, so it + * wins over server state. + * - Running (titled): the *server* is authoritative. The overlay is only + * refreshed on manual chip edits, so server-driven changes (e.g. Plan → + * Autopilot when the user approves a plan) must win, otherwise a stale + * overlay value would shadow them. + */ +export function resolveConfigChipValue(isUntitled: boolean, serverValue: unknown, overlayValue: unknown, schemaDefault: unknown): unknown { + const preferred = isUntitled + ? (overlayValue ?? serverValue) + : (serverValue ?? overlayValue); + return preferred ?? schemaDefault; +} + /** * One workbench chat-input chip bound to a single agent-host session-config * property. Used both for dedicated well-known property chips @@ -215,6 +240,8 @@ export class AgentHostChatInputPicker extends Disposable { @IAgentHostUntitledProvisionalSessionService private readonly _provisional: IAgentHostUntitledProvisionalSessionService, @IConfigurationService private readonly _configurationService: IConfigurationService, @IAgentHostNewSessionFolderService private readonly _newSessionFolderService: IAgentHostNewSessionFolderService, + @IDialogService private readonly _dialogService: IDialogService, + @IStorageService private readonly _storageService: IStorageService, ) { super(); @@ -419,9 +446,9 @@ export class AgentHostChatInputPicker extends Disposable { if (!schema) { return undefined; } - const value = overlay?.values?.[this._property] - ?? state.config?.values?.[this._property] - ?? schema.default; + const serverValue = state.config?.values?.[this._property]; + const overlayValue = overlay?.values?.[this._property]; + const value = resolveConfigChipValue(isUntitledChatSession(sessionResource), serverValue, overlayValue, schema.default); return { backendSession: this._subRef.value.backendSession, schema, value }; } @@ -478,7 +505,7 @@ export class AgentHostChatInputPicker extends Disposable { void this._openerService.open(URI.parse(PERMISSION_MODE_LEARN_MORE_URL)); return; } - void this._setValue(ctx.backendSession, item.value); + void this._confirmAndSetValue(ctx.backendSession, item.value); }, onFilter: ctx.schema.enumDynamic ? query => this._filterDelayer.trigger(async () => { @@ -567,6 +594,29 @@ export class AgentHostChatInputPicker extends Disposable { return overlay?.values ?? this._initialResolved?.result.values; } + /** + * Surfaces the shared elevated-level warning for a Bypass / (legacy) + * Autopilot approval pick before applying it. Non-elevated levels and + * non-approval properties apply directly. Tolerated forward/legacy + * auto-approve values that are not recognized {@link ChatPermissionLevel}s + * (e.g. `assisted`) are still treated as elevated and fall back to the + * Bypass Approvals warning so they can never be selected silently. + */ + private async _confirmAndSetValue(backendSession: URI, value: string): Promise { + if (this._property === SessionConfigKey.AutoApprove) { + const levelToConfirm = isChatPermissionLevel(value) + ? value + : (value !== ChatPermissionLevel.Default ? ChatPermissionLevel.AutoApprove : undefined); + if (levelToConfirm) { + const confirmed = await maybeConfirmElevatedPermissionLevel(levelToConfirm, this._dialogService, this._storageService, { defaultSettingKey: ChatConfiguration.AgentSessionDefaultConfiguration }); + if (!confirmed) { + return; + } + } + } + await this._setValue(backendSession, value); + } + private async _setValue(backendSession: URI, value: string): Promise { const sessionResource = this._widget.viewModel?.sessionResource; if (!sessionResource) { diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts index 1a77d971553d9..428c24f95610c 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts @@ -1780,8 +1780,8 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC invocation.didExecuteTool(undefined); const confirmInvocation = toolCallStateToInvocation(tc, subAgentInvocationId, opts.backendSession, this._config.connectionAuthority); opts.sink([confirmInvocation]); - this._awaitToolConfirmation(confirmInvocation, toolCallId, opts.backendSession, opts.turnId, opts.cancellationToken, tc.options, opts.chatChannel); invocation = confirmInvocation; + this._awaitToolConfirmation(confirmInvocation, toolCallId, opts.backendSession, opts.turnId, opts.cancellationToken, tc.options, opts.chatChannel); } else if (status === ToolCallStatus.Running || status === ToolCallStatus.PendingResultConfirmation) { invocation.invocationMessage = stringOrMarkdownToString(tc.invocationMessage, this._config.connectionAuthority); this._reviveTerminalIfNeeded(invocation, tc, opts.backendSession); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.ts index 1027ee766cc08..a92e31d58fc4d 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.ts @@ -55,7 +55,8 @@ import { ResourceMap, ResourceSet } from '../../../../../../base/common/map.js'; import { equals } from '../../../../../../base/common/objects.js'; import { URI } from '../../../../../../base/common/uri.js'; import { IAgentHostService } from '../../../../../../platform/agentHost/common/agentService.js'; -import { KNOWN_AUTO_APPROVE_VALUES, SessionConfigKey } from '../../../../../../platform/agentHost/common/sessionConfigKeys.js'; +import { KNOWN_AUTO_APPROVE_VALUES, KNOWN_MODE_VALUES, SessionConfigKey } from '../../../../../../platform/agentHost/common/sessionConfigKeys.js'; +import { migrateLegacyAutopilotConfig } from '../../../../../../platform/agentHost/common/agentHostSchema.js'; import { ActionType } from '../../../../../../platform/agentHost/common/state/protocol/actions.js'; import type { ResolveSessionConfigResult } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; @@ -63,7 +64,7 @@ import { InstantiationType, registerSingleton } from '../../../../../../platform import { createDecorator } from '../../../../../../platform/instantiation/common/instantiation.js'; import { ILogService } from '../../../../../../platform/log/common/log.js'; import { IWorkbenchEnvironmentService } from '../../../../../services/environment/common/environmentService.js'; -import { ChatConfiguration } from '../../../common/constants.js'; +import { ChatConfiguration, type IAgentSessionDefaultConfiguration } from '../../../common/constants.js'; import { IChatService } from '../../../common/chatService/chatService.js'; import { IAgentHostNewSessionFolderService } from './agentHostNewSessionFolderService.js'; @@ -602,8 +603,11 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple * drops the workbench defaults. * * - `isolation`: workbench has no isolation picker, so always `'folder'`. - * - `autoApprove`: seeded from `chat.permissions.default`, clamped to - * `'default'` when the `chat.tools.global.autoApprove` policy is off. + * - `mode` / `autoApprove`: seeded from the single + * `chat.agentSessions.defaultConfiguration` object setting (`mode` and + * `approvals` properties). The approval seed is clamped to `'default'` + * when the `chat.tools.global.autoApprove` policy is off. The local-only + * `chat.permissions.default` setting is NOT used. * * Skipped entirely in the Agents window, where the sessions provider * supplies config via `request.agentHostSessionConfig` instead. @@ -613,13 +617,24 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple return undefined; } const config: Record = { [SessionConfigKey.Isolation]: 'folder' }; - const configured = this._configurationService.getValue(ChatConfiguration.DefaultPermissionLevel); + + const configuredDefaults = this._configurationService.getValue(ChatConfiguration.AgentSessionDefaultConfiguration); const policyValue = this._configurationService.inspect(ChatConfiguration.GlobalAutoApprove).policyValue; - if (typeof configured === 'string' && KNOWN_AUTO_APPROVE_VALUES.has(configured)) { + + const configuredApprovals = configuredDefaults?.approvals; + if (typeof configuredApprovals === 'string' && KNOWN_AUTO_APPROVE_VALUES.has(configuredApprovals)) { const policyRestricted = policyValue === false; - config[SessionConfigKey.AutoApprove] = policyRestricted ? 'default' : configured; + // Bypass and (legacy) Autopilot auto-approve at least some tool + // calls, so clamp anything but Default under policy. + config[SessionConfigKey.AutoApprove] = policyRestricted && configuredApprovals !== 'default' ? 'default' : configuredApprovals; } - return config; + + const configuredMode = configuredDefaults?.mode; + if (typeof configuredMode === 'string' && KNOWN_MODE_VALUES.has(configuredMode)) { + config[SessionConfigKey.Mode] = configuredMode; + } + + return migrateLegacyAutopilotConfig(config); } } diff --git a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts index 0b193aa517e01..963845986e859 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts @@ -488,9 +488,38 @@ configurationRegistry.registerConfiguration({ nls.localize('chat.permissions.default.autoApprove.description', "Start new chat sessions in Bypass Approvals mode."), nls.localize('chat.permissions.default.autopilot.description', "Start new chat sessions in Autopilot mode."), ], - description: nls.localize('chat.permissions.default.settingDescription', "Controls the default permissions picker mode for new chat sessions. You can still change the permission mode per session, and each session remembers the permission mode that was used. If enterprise policy disables auto approval, new sessions use Default Approvals."), + description: nls.localize('chat.permissions.default.settingDescription', "Controls the default permissions picker mode for new local chat sessions. You can still change the permission mode per session, and each session remembers the permission mode that was used. If enterprise policy disables auto approval, new sessions use Default Approvals."), default: ChatPermissionLevel.Default, }, + [ChatConfiguration.AgentSessionDefaultConfiguration]: { + type: 'object', + additionalProperties: false, + properties: { + mode: { + type: 'string', + enum: ['interactive', 'plan', 'autopilot'], + enumDescriptions: [ + nls.localize('chat.agentSessions.defaultConfiguration.mode.interactive', "Interactive — step-by-step collaboration."), + nls.localize('chat.agentSessions.defaultConfiguration.mode.plan', "Plan — plan first, execute when ready."), + nls.localize('chat.agentSessions.defaultConfiguration.mode.autopilot', "Autopilot — autonomously iterate from start to finish."), + ], + default: 'interactive', + description: nls.localize('chat.agentSessions.defaultConfiguration.mode.description', "The starting mode for new agent sessions."), + }, + approvals: { + type: 'string', + enum: [ChatPermissionLevel.Default, ChatPermissionLevel.AutoApprove], + enumDescriptions: [ + nls.localize('chat.agentSessions.defaultConfiguration.approvals.default', "Default Approvals — Copilot uses your configured settings."), + nls.localize('chat.agentSessions.defaultConfiguration.approvals.autoApprove', "Bypass Approvals — all tool calls are auto-approved."), + ], + default: ChatPermissionLevel.Default, + description: nls.localize('chat.agentSessions.defaultConfiguration.approvals.description', "The starting approval behavior for new agent sessions. If enterprise policy disables auto approval, new sessions use Default Approvals."), + }, + }, + default: { mode: 'interactive', approvals: ChatPermissionLevel.Default }, + markdownDescription: nls.localize('chat.agentSessions.defaultConfiguration.settingDescription', "Controls the default configuration (mode and approval behavior) for new agent sessions (such as Copilot CLI). You can still change the mode and approval level per session, and each session remembers what was used."), + }, [ChatConfiguration.GlobalAutoApprove]: { default: false, markdownDescription: globalAutoApproveDescription.value, diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts b/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts index ce332564e5d82..8db3c18022676 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts @@ -39,6 +39,19 @@ export interface IExtensionPermissionState { export interface IPermissionPickerDelegate { readonly currentPermissionLevel: IObservable; readonly setPermissionLevel: (level: ChatPermissionLevel) => void; + /** + * The ordered set of permission levels the picker should offer. When + * omitted, the built-in Default/Bypass/Autopilot set is used. Agent-host + * sessions override this to Default/Bypass (Autopilot lives on the + * orthogonal mode axis there). + */ + readonly availableLevels?: readonly ChatPermissionLevel[]; + /** + * The setting id the elevated-level warning dialog links to as "make this + * the default". Defaults to `chat.permissions.default`; agent-host sessions + * pass `chat.agentSessions.defaultConfiguration`. + */ + readonly defaultSettingKey?: string; /** * When defined and returns a non-empty state, the picker shows the extension-contributed * items in place of the built-in {@link ChatPermissionLevel} items. @@ -47,6 +60,60 @@ export interface IPermissionPickerDelegate { readonly setExtensionPermission?: (groupId: string, item: IChatSessionProviderOptionItem) => void; } +/** Default level set offered when a delegate does not specify {@link IPermissionPickerDelegate.availableLevels}. */ +const DEFAULT_PERMISSION_LEVELS: readonly ChatPermissionLevel[] = [ + ChatPermissionLevel.Default, + ChatPermissionLevel.AutoApprove, + ChatPermissionLevel.Autopilot, +]; + +interface IPermissionLevelMeta { + readonly id: string; + readonly label: string; + readonly shortLabel: string; + readonly detail: string; + readonly icon: ThemeIcon; + readonly description: string; + /** Elevated levels are disabled when enterprise policy turns off auto-approval and need a warning dialog. */ + readonly elevated: boolean; +} + +function getPermissionLevelMeta(level: ChatPermissionLevel): IPermissionLevelMeta { + switch (level) { + case ChatPermissionLevel.AutoApprove: + return { + id: 'chat.permissions.autoApprove', + label: localize('permissions.autoApprove', "Bypass Approvals"), + shortLabel: localize('permissions.autoApprove.label', "Bypass Approvals"), + detail: localize('permissions.autoApprove.subtext', "All tool calls are auto-approved"), + icon: ThemeIcon.fromId(Codicon.warning.id), + description: localize('permissions.autoApprove.description', "Auto-approve all tool calls and retry on errors"), + elevated: true, + }; + case ChatPermissionLevel.Autopilot: + return { + id: 'chat.permissions.autopilot', + label: localize('permissions.autopilot', "Autopilot (Preview)"), + shortLabel: localize('permissions.autopilot.label', "Autopilot (Preview)"), + detail: localize('permissions.autopilot.subtext', "Autonomously iterates from start to finish"), + icon: ThemeIcon.fromId(Codicon.rocket.id), + description: localize('permissions.autopilot.description', "Auto-approve all tool calls and continue until the task is done. Autopilot may increase costs."), + elevated: true, + }; + case ChatPermissionLevel.Default: + default: + return { + id: 'chat.permissions.default', + label: localize('permissions.default', "Default Approvals"), + shortLabel: localize('permissions.default.label', "Default Approvals"), + detail: localize('permissions.default.subtext', "Copilot uses your configured settings"), + icon: ThemeIcon.fromId(Codicon.shield.id), + description: localize('permissions.default.description', "Use configured approval settings"), + elevated: false, + }; + } +} + /** Sanitize a free-form id segment so it is safe to embed in a stable action identifier. */ function sanitizeIdSegment(value: string): string { return value.replace(/[^a-zA-Z0-9_-]/g, '_'); @@ -104,74 +171,36 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { } const currentLevel = delegate.currentPermissionLevel.get(); const policyRestricted = isAutoApprovePolicyRestricted(); - const actions: IActionWidgetDropdownAction[] = [ - { + const levels = delegate.availableLevels ?? DEFAULT_PERMISSION_LEVELS; + const actions: IActionWidgetDropdownAction[] = levels.map(level => { + const meta = getPermissionLevelMeta(level); + const disabledByPolicy = meta.elevated && policyRestricted; + return { ...action, - id: 'chat.permissions.default', - label: localize('permissions.default', "Default Approvals"), - detail: localize('permissions.default.subtext', "Copilot uses your configured settings"), - icon: ThemeIcon.fromId(Codicon.shield.id), - checked: currentLevel === ChatPermissionLevel.Default, - tooltip: '', + id: meta.id, + label: meta.label, + detail: meta.detail, + icon: meta.icon, + checked: currentLevel === level, + enabled: !disabledByPolicy, + tooltip: disabledByPolicy ? localize('permissions.policyDisabled', "Disabled by enterprise policy") : '', hover: { - content: localize('permissions.default.description', "Use configured approval settings"), + content: disabledByPolicy + ? localize('permissions.policyDescription', "Disabled by enterprise policy") + : meta.description, }, run: async () => { - delegate.setPermissionLevel(ChatPermissionLevel.Default); - if (this.element) { - this.renderLabel(this.element); - } - }, - } satisfies IActionWidgetDropdownAction, - { - ...action, - id: 'chat.permissions.autoApprove', - label: localize('permissions.autoApprove', "Bypass Approvals"), - detail: localize('permissions.autoApprove.subtext', "All tool calls are auto-approved"), - icon: ThemeIcon.fromId(Codicon.warning.id), - checked: currentLevel === ChatPermissionLevel.AutoApprove, - enabled: !policyRestricted, - tooltip: policyRestricted ? localize('permissions.autoApprove.policyDisabled', "Disabled by enterprise policy") : '', - hover: { - content: policyRestricted - ? localize('permissions.autoApprove.policyDescription', "Disabled by enterprise policy") - : localize('permissions.autoApprove.description', "Auto-approve all tool calls and retry on errors"), - }, - run: async () => { - if (!await maybeConfirmElevatedPermissionLevel(ChatPermissionLevel.AutoApprove, this.dialogService, storageService)) { + // Elevated levels show a one-time confirmation warning. + if (meta.elevated && !await maybeConfirmElevatedPermissionLevel(level, this.dialogService, storageService, { defaultSettingKey: delegate.defaultSettingKey })) { return; } - delegate.setPermissionLevel(ChatPermissionLevel.AutoApprove); + delegate.setPermissionLevel(level); if (this.element) { this.renderLabel(this.element); } }, - } satisfies IActionWidgetDropdownAction, - ]; - actions.push({ - ...action, - id: 'chat.permissions.autopilot', - label: localize('permissions.autopilot', "Autopilot (Preview)"), - detail: localize('permissions.autopilot.subtext', "Autonomously iterates from start to finish"), - icon: ThemeIcon.fromId(Codicon.rocket.id), - checked: currentLevel === ChatPermissionLevel.Autopilot, - enabled: !policyRestricted, - tooltip: policyRestricted ? localize('permissions.autopilot.policyDisabled', "Disabled by enterprise policy") : '', - hover: { - content: policyRestricted - ? localize('permissions.autopilot.policyDescription', "Disabled by enterprise policy") - : localize('permissions.autopilot.description', "Auto-approve all tool calls and continue until the task is done. Autopilot may increase costs."), - }, - run: async () => { - if (!await maybeConfirmElevatedPermissionLevel(ChatPermissionLevel.Autopilot, this.dialogService, storageService)) { - return; - } - delegate.setPermissionLevel(ChatPermissionLevel.Autopilot); - if (this.element) { - this.renderLabel(this.element); - } - }, - } satisfies IActionWidgetDropdownAction); + } satisfies IActionWidgetDropdownAction; + }); return actions; } }; @@ -213,23 +242,10 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { label = selected.name; tooltip = selected.description ?? selected.name; } else { - switch (level) { - case ChatPermissionLevel.Autopilot: - icon = Codicon.rocket; - label = localize('permissions.autopilot.label', "Autopilot (Preview)"); - tooltip = localize('permissions.autopilot.description', "Auto-approve all tool calls and continue until the task is done. Autopilot may increase costs."); - break; - case ChatPermissionLevel.AutoApprove: - icon = Codicon.warning; - label = localize('permissions.autoApprove.label', "Bypass Approvals"); - tooltip = localize('permissions.autoApprove.description', "Auto-approve all tool calls and retry on errors"); - break; - default: - icon = Codicon.shield; - label = localize('permissions.default.label', "Default Approvals"); - tooltip = localize('permissions.default.description', "Use configured approval settings"); - break; - } + const meta = getPermissionLevelMeta(level); + icon = meta.icon; + label = meta.shortLabel; + tooltip = meta.description; } const labelElements = []; diff --git a/src/vs/workbench/contrib/chat/common/chatPermissionWarnings.ts b/src/vs/workbench/contrib/chat/common/chatPermissionWarnings.ts index e252b13de38c3..4070d0095fc5c 100644 --- a/src/vs/workbench/contrib/chat/common/chatPermissionWarnings.ts +++ b/src/vs/workbench/contrib/chat/common/chatPermissionWarnings.ts @@ -6,6 +6,7 @@ import { Codicon } from '../../../../base/common/codicons.js'; import { MarkdownString } from '../../../../base/common/htmlContent.js'; import Severity from '../../../../base/common/severity.js'; +import { ThemeIcon } from '../../../../base/common/themables.js'; import { localize } from '../../../../nls.js'; import { IDialogService } from '../../../../platform/dialogs/common/dialogs.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js'; @@ -43,17 +44,68 @@ export function resetShownWarnings(): void { shownWarnings.clear(); } +/** + * Relative "auto-approval reach" of each elevated level, used to suppress + * lower-level warnings once a stronger one has been accepted. Bypass Approvals + * and Autopilot both auto-approve *every* tool call, so confirming either in + * this session (or persistently) implies acceptance of the other — the user is + * not warned again when stepping between them (e.g. Autopilot → Bypass). + */ +const ELEVATION_RANK: ReadonlyMap = new Map([ + [ChatPermissionLevel.AutoApprove, 1], + [ChatPermissionLevel.Autopilot, 1], +]); + export function hasShownElevatedWarning(level: ChatPermissionLevel, storageService: IStorageService): boolean { - if (shownWarnings.has(level)) { - return true; + const rank = ELEVATION_RANK.get(level); + if (rank === undefined) { + return false; } - const key = dontShowAgainKey(level); - if (key && storageService.getBoolean(key, StorageScope.PROFILE, false)) { - return true; + // Accepting any elevated level whose reach is >= this one (in this session + // or persistently) suppresses the warning. + for (const [candidate, candidateRank] of ELEVATION_RANK) { + if (candidateRank < rank) { + continue; + } + if (shownWarnings.has(candidate)) { + return true; + } + const key = dontShowAgainKey(candidate); + if (key && storageService.getBoolean(key, StorageScope.PROFILE, false)) { + return true; + } } return false; } +interface IElevatedWarningCopy { + readonly title: string; + readonly confirm: string; + readonly icon: ThemeIcon; + readonly detail: string; +} + +function getElevatedWarningCopy(level: ChatPermissionLevel, defaultSettingKey: string): IElevatedWarningCopy | undefined { + switch (level) { + case ChatPermissionLevel.Autopilot: + return { + title: localize('permissions.autopilot.warning.title', "Enable Autopilot?"), + confirm: localize('permissions.autopilot.warning.confirm', "Enable"), + icon: Codicon.rocket, + detail: localize('permissions.autopilot.warning.detail', "Autopilot will auto-approve all tool calls and continue working autonomously until the task is complete. This includes terminal commands, file edits, and external tool calls. The agent will make decisions on your behalf without asking for confirmation.\n\nYou can stop the agent at any time by clicking the stop button. This applies to the current session only.\n\nTo make this the starting permission level for new sessions, change the [{0}](command:workbench.action.openSettings?%5B%22{0}%22%5D) setting.", defaultSettingKey), + }; + case ChatPermissionLevel.AutoApprove: + return { + title: localize('permissions.autoApprove.warning.title', "Enable Bypass Approvals?"), + confirm: localize('permissions.autoApprove.warning.confirm', "Enable"), + icon: Codicon.warning, + detail: localize('permissions.autoApprove.warning.detail', "Bypass Approvals will auto-approve all tool calls without asking for confirmation. This includes file edits, terminal commands, and external tool calls.\n\nTo make this the starting permission level for new sessions, change the [{0}](command:workbench.action.openSettings?%5B%22{0}%22%5D) setting.", defaultSettingKey), + }; + default: + return undefined; + } +} + /** * If `level` is an elevated permission level (Bypass Approvals or Autopilot) * that hasn't already been confirmed in this session or persistently @@ -68,34 +120,38 @@ export function hasShownElevatedWarning(level: ChatPermissionLevel, storageServi * When the user ticks "Don't show again", the choice is persisted in * `StorageScope.PROFILE`, which is shared across the workbench and Agents * windows so a dismissal in one applies to the other. + * + * `options.defaultSettingKey` controls which "make this the default" setting + * the dialog links to — local chat uses `chat.permissions.default` (the + * default), while agent-host sessions pass + * `chat.agentSessions.defaultConfiguration`. */ export async function maybeConfirmElevatedPermissionLevel( level: ChatPermissionLevel, dialogService: IDialogService, storageService: IStorageService, + options?: { readonly defaultSettingKey?: string }, ): Promise { const key = dontShowAgainKey(level); if (!key || hasShownElevatedWarning(level, storageService)) { return true; } - const isAutopilot = level === ChatPermissionLevel.Autopilot; + const copy = getElevatedWarningCopy(level, options?.defaultSettingKey ?? ChatConfiguration.DefaultPermissionLevel); + if (!copy) { + return true; + } + const result = await dialogService.prompt({ type: Severity.Warning, - message: isAutopilot - ? localize('permissions.autopilot.warning.title', "Enable Autopilot?") - : localize('permissions.autoApprove.warning.title', "Enable Bypass Approvals?"), + message: copy.title, buttons: [ { - label: isAutopilot - ? localize('permissions.autopilot.warning.confirm', "Enable") - : localize('permissions.autoApprove.warning.confirm', "Enable"), + label: copy.confirm, run: () => true, }, { - label: isAutopilot - ? localize('permissions.autopilot.warning.cancel', "Cancel") - : localize('permissions.autoApprove.warning.cancel', "Cancel"), + label: localize('permissions.warning.cancel', "Cancel"), run: () => false, }, ], @@ -104,12 +160,10 @@ export async function maybeConfirmElevatedPermissionLevel( checked: false, }, custom: { - icon: isAutopilot ? Codicon.rocket : Codicon.warning, + icon: copy.icon, markdownDetails: [{ markdown: new MarkdownString( - isAutopilot - ? localize('permissions.autopilot.warning.detail', "Autopilot will auto-approve all tool calls and continue working autonomously until the task is complete. This includes terminal commands, file edits, and external tool calls. The agent will make decisions on your behalf without asking for confirmation.\n\nYou can stop the agent at any time by clicking the stop button. This applies to the current session only.\n\nTo make this the starting permission level for new chat sessions, change the [{0}](command:workbench.action.openSettings?%5B%22{0}%22%5D) setting.", ChatConfiguration.DefaultPermissionLevel) - : localize('permissions.autoApprove.warning.detail', "Bypass Approvals will auto-approve all tool calls without asking for confirmation. This includes file edits, terminal commands, and external tool calls.\n\nTo make this the starting permission level for new chat sessions, change the [{0}](command:workbench.action.openSettings?%5B%22{0}%22%5D) setting.", ChatConfiguration.DefaultPermissionLevel), + copy.detail, { isTrusted: { enabledCommands: ['workbench.action.openSettings'] } }, ), }], diff --git a/src/vs/workbench/contrib/chat/common/constants.ts b/src/vs/workbench/contrib/chat/common/constants.ts index 6f4d9c515bcbb..3b77d543f7fa7 100644 --- a/src/vs/workbench/contrib/chat/common/constants.ts +++ b/src/vs/workbench/contrib/chat/common/constants.ts @@ -80,6 +80,7 @@ export enum ChatConfiguration { AutopilotAdvancedEnabled = 'chat.autopilot.advanced.enabled', PlanReviewInlineEditorEnabled = 'chat.planReview.inlineEditor.enabled', DefaultPermissionLevel = 'chat.permissions.default', + AgentSessionDefaultConfiguration = 'chat.agentSessions.defaultConfiguration', ImageCarouselEnabled = 'imageCarousel.chat.enabled', ArtifactsEnabled = 'chat.artifacts.enabled', ArtifactsRulesByMimeType = 'chat.artifacts.rules.byMimeType', @@ -130,6 +131,21 @@ export function isChatPermissionLevel(level: unknown | undefined): level is Chat return chatPermissionLevels.has(level as string); } +/** + * Shape of the {@link ChatConfiguration.AgentSessionDefaultConfiguration} + * object setting. Controls the starting `mode` and `approvals` for new + * agent-host sessions (such as Copilot CLI). Both properties are optional — + * a missing property falls back to the per-axis default. + */ +export type AgentSessionMode = 'interactive' | 'plan' | 'autopilot'; + +export interface IAgentSessionDefaultConfiguration { + /** Starting agent mode: `interactive` / `plan` / `autopilot`. */ + readonly mode?: AgentSessionMode; + /** Starting approval level: `default` / `autoApprove`. */ + readonly approvals?: ChatPermissionLevel.Default | ChatPermissionLevel.AutoApprove; +} + /** * Returns true if the permission level enables auto-approval of all tool calls. * Both {@link ChatPermissionLevel.AutoApprove} and {@link ChatPermissionLevel.Autopilot} enable auto-approval. diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts new file mode 100644 index 0000000000000..d135430b1272b --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts @@ -0,0 +1,45 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { resolveConfigChipValue } from '../../../browser/agentSessions/agentHost/agentHostChatInputPicker.js'; + +suite('AgentHostChatInputPicker - resolveConfigChipValue', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + suite('running (titled) session', () => { + + test('server value wins over a stale overlay (server-driven mode change is reflected)', () => { + // Server flips Plan → Autopilot (e.g. user approved a plan); the + // overlay still holds the old manually-picked value. + assert.strictEqual(resolveConfigChipValue(false, 'autopilot', 'plan', 'interactive'), 'autopilot'); + }); + + test('falls back to overlay when the server has no value', () => { + assert.strictEqual(resolveConfigChipValue(false, undefined, 'plan', 'interactive'), 'plan'); + }); + + test('falls back to schema default when neither has a value', () => { + assert.strictEqual(resolveConfigChipValue(false, undefined, undefined, 'interactive'), 'interactive'); + }); + }); + + suite('untitled (pre-send) session', () => { + + test('overlay wins so a synchronous chip edit is reflected before the backend echoes', () => { + assert.strictEqual(resolveConfigChipValue(true, 'interactive', 'plan', 'interactive'), 'plan'); + }); + + test('falls back to server value when the overlay has none', () => { + assert.strictEqual(resolveConfigChipValue(true, 'autopilot', undefined, 'interactive'), 'autopilot'); + }); + + test('falls back to schema default when neither has a value', () => { + assert.strictEqual(resolveConfigChipValue(true, undefined, undefined, 'interactive'), 'interactive'); + }); + }); +}); diff --git a/src/vs/workbench/contrib/chat/test/common/chatPermissionWarnings.test.ts b/src/vs/workbench/contrib/chat/test/common/chatPermissionWarnings.test.ts new file mode 100644 index 0000000000000..7017092dd129b --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/common/chatPermissionWarnings.test.ts @@ -0,0 +1,47 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { InMemoryStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; +import { AUTO_APPROVE_DONT_SHOW_AGAIN_KEY, AUTOPILOT_DONT_SHOW_AGAIN_KEY } from '../../common/chatPermissionStorageKeys.js'; +import { hasShownElevatedWarning, resetShownWarnings } from '../../common/chatPermissionWarnings.js'; +import { ChatPermissionLevel } from '../../common/constants.js'; + +suite('chatPermissionWarnings', () => { + + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + function storage(dismissedKey?: string): InMemoryStorageService { + const s = store.add(new InMemoryStorageService()); + if (dismissedKey) { + s.store(dismissedKey, true, StorageScope.PROFILE, StorageTarget.USER); + } + return s; + } + + setup(() => resetShownWarnings()); + teardown(() => resetShownWarnings()); + + test('non-elevated Default is never considered "warned"', () => { + assert.strictEqual(hasShownElevatedWarning(ChatPermissionLevel.Default, storage()), false); + }); + + test('an unconfirmed elevated level has not been warned', () => { + const s = storage(); + assert.strictEqual(hasShownElevatedWarning(ChatPermissionLevel.AutoApprove, s), false); + assert.strictEqual(hasShownElevatedWarning(ChatPermissionLevel.Autopilot, s), false); + }); + + test('confirming Autopilot suppresses the equal-reach Bypass warning', () => { + const s = storage(AUTOPILOT_DONT_SHOW_AGAIN_KEY); + assert.strictEqual(hasShownElevatedWarning(ChatPermissionLevel.AutoApprove, s), true); + }); + + test('confirming Bypass suppresses the equal-reach Autopilot warning', () => { + const s = storage(AUTO_APPROVE_DONT_SHOW_AGAIN_KEY); + assert.strictEqual(hasShownElevatedWarning(ChatPermissionLevel.Autopilot, s), true); + }); +}); From 4bbe52aa780876fafb994bdfba0d562aa9e65c12 Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Thu, 18 Jun 2026 22:47:24 -0700 Subject: [PATCH 2/8] fix pasted attachments rendering (#322040) --- .../contrib/chat/browser/widget/chatListRenderer.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index 2f5c5f3935f05..31dd5d3dd3281 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -56,7 +56,7 @@ import { ChatPlanReviewData } from '../../common/model/chatProgressTypes/chatPla import { ChatQuestionCarouselData } from '../../common/model/chatProgressTypes/chatQuestionCarouselData.js'; import { localChatSessionType } from '../../common/chatSessionsService.js'; import { getChatSessionType } from '../../common/model/chatUri.js'; -import { getExplicitFileOrImageAttachmentSummary, IChatRequestVariableEntry, isExplicitFileOrImageVariableEntry } from '../../common/attachments/chatVariableEntries.js'; +import { getExplicitFileOrImageAttachmentSummary, IChatRequestVariableEntry, isExplicitFileOrImageVariableEntry, isPasteVariableEntry } from '../../common/attachments/chatVariableEntries.js'; import { IChatChangesSummaryPart, IChatCodeCitations, IChatErrorDetailsPart, IChatReferences, IChatRendererContent, IChatRequestViewModel, IChatResponseViewModel, IChatViewModel, IChatWorkingProgress, isRequestVM, isResponseVM, IChatPendingDividerViewModel, isPendingDividerVM } from '../../common/model/chatViewModel.js'; import { getNWords } from '../../common/model/chatWordCounter.js'; import { ChatAgentLocation, ChatConfiguration, ChatModeKind, CollapsedToolsDisplayMode, ThinkingDisplayMode } from '../../common/constants.js'; @@ -1445,8 +1445,8 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer variable.kind === 'image'); - const explicitFileOrDirectoryVariables = explicitFileOrImageVariables.filter(variable => variable.kind === 'file' || variable.kind === 'directory'); - const otherVariables = element.variables.filter(variable => !isExplicitFileOrImageVariableEntry(variable)); + const explicitFileOrDirectoryVariables = element.variables.filter(variable => variable.kind === 'file' || variable.kind === 'directory' || isPasteVariableEntry(variable)); + const otherVariables = element.variables.filter(variable => !isExplicitFileOrImageVariableEntry(variable) && !isPasteVariableEntry(variable)); if (!element.confirmation) { const markdown = isChatFollowup(element.message) ? element.message.message : From 0db88a667d78694d2971363b58b2ae194fe4fab1 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 19 Jun 2026 15:56:49 +1000 Subject: [PATCH 3/8] Implement CoalescingAgentHostSessionListConnection to optimize session list retrieval (#322030) * Implement CoalescingAgentHostSessionListConnection to optimize session list retrieval * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Use type imports for URI in agentHostChatContribution.ts * Fix destructuring in AgentHostChatContribution test to only capture the first promise result --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../agentHost/agentHostChatContribution.ts | 53 +++++- .../agentHostSessionListController.ts | 32 +++- .../agentHostChatContribution.test.ts | 170 +++++++++++++++++- 3 files changed, 246 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts index c5607b0544cda..02a0f33e64980 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts @@ -7,8 +7,9 @@ import { Codicon } from '../../../../../../base/common/codicons.js'; import { Event } from '../../../../../../base/common/event.js'; import { Disposable, DisposableMap, DisposableStore, toDisposable } from '../../../../../../base/common/lifecycle.js'; import { ThemeIcon } from '../../../../../../base/common/themables.js'; +import { type URI } from '../../../../../../base/common/uri.js'; import { localize } from '../../../../../../nls.js'; -import { AgentHostEnabledSettingId, claudePreferAgentHostSettingId, IAgentHostService, shouldSurfaceLocalAgentHostProvider, type AgentProvider } from '../../../../../../platform/agentHost/common/agentService.js'; +import { AgentHostEnabledSettingId, claudePreferAgentHostSettingId, IAgentHostService, shouldSurfaceLocalAgentHostProvider, type AgentProvider, type IAgentSessionMetadata } from '../../../../../../platform/agentHost/common/agentService.js'; import { type ProtectedResourceMetadata } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; import { type AgentInfo, type RootState } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; @@ -29,7 +30,7 @@ import { authenticateProtectedResources, AgentHostAuthTokenCache, resolveAuthent import { AgentHostLanguageModelProvider, agentHostProviderSupportsAutoModel } from './agentHostLanguageModelProvider.js'; import { AgentHostSessionHandler } from './agentHostSessionHandler.js'; import { IAgentHostActiveClientService } from './agentHostActiveClientService.js'; -import { AgentHostSessionListController } from './agentHostSessionListController.js'; +import { AgentHostSessionListController, IAgentHostSessionListConnection } from './agentHostSessionListController.js'; import { AICustomizationSources } from '../../../common/aiCustomizationWorkspaceService.js'; const LOCAL_AGENT_HOST_SESSION_TYPE_PREFIX = 'agent-host-'; @@ -78,6 +79,50 @@ function getLocalAgentHostProviderForSessionType(sessionType: string): AgentProv return sessionType.slice(LOCAL_AGENT_HOST_SESSION_TYPE_PREFIX.length) || undefined; } +/** + * Shared session-list connection used by all local agent-host list controllers. + * + * The agent host exposes a single provider-agnostic `listSessions()` RPC, while + * the workbench registers one {@link AgentHostSessionListController} per agent + * provider. Those controllers can refresh at the same time during startup, + * reconnect, or workspace changes. This wrapper keeps the controller coupled + * only to the minimal list-session surface and joins concurrent refreshes onto + * one in-flight `listSessions()` request so the agent host does not repeat the + * same session enumeration work for every provider. + */ +export class CoalescingAgentHostSessionListConnection implements IAgentHostSessionListConnection { + + private _listSessionsInFlight: Promise | undefined; + + constructor( + private readonly _delegate: IAgentHostService, + ) { } + + get onDidNotification(): IAgentHostSessionListConnection['onDidNotification'] { + return this._delegate.onDidNotification; + } + + disposeSession(session: URI): Promise { + return this._delegate.disposeSession(session); + } + + listSessions(): Promise { + if (this._listSessionsInFlight) { + return this._listSessionsInFlight; + } + + const request = this._delegate.listSessions(); + this._listSessionsInFlight = request; + const clear = () => { + if (this._listSessionsInFlight === request) { + this._listSessionsInFlight = undefined; + } + }; + request.then(clear, clear); + return request; + } +} + export { AgentHostSessionHandler } from './agentHostSessionHandler.js'; export { AgentHostSessionListController } from './agentHostSessionListController.js'; @@ -97,6 +142,7 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr private readonly _modelProviders = new Map(); /** List controllers keyed by agent provider, for cache resets on reconnect. */ private readonly _listControllers = new Map(); + private readonly _sessionListConnection: CoalescingAgentHostSessionListConnection; /** Dedupes redundant `authenticate` RPCs when the resolved token hasn't changed. */ private readonly _authTokenCache = new AgentHostAuthTokenCache(); @@ -122,6 +168,7 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr this._isSessionsWindow = environmentService.isSessionsWindow; this._enableSmokeTestDriver = !!environmentService.enableSmokeTestDriver; + this._sessionListConnection = new CoalescingAgentHostSessionListConnection(this._agentHostService); if (!this._configurationService.getValue(AgentHostEnabledSettingId)) { return; @@ -257,7 +304,7 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr })); // Session list controller - const listController = store.add(this._instantiationService.createInstance(AgentHostSessionListController, sessionType, agent.provider, this._agentHostService, undefined, 'local')); + const listController = store.add(this._instantiationService.createInstance(AgentHostSessionListController, sessionType, agent.provider, this._sessionListConnection, undefined, 'local')); this._listControllers.set(agent.provider, listController); store.add({ dispose: () => this._listControllers.delete(agent.provider) }); store.add(this._chatSessionsService.registerChatSessionItemController(sessionType, listController)); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts index b53890a13e3ac..a6bd68095d63f 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts @@ -5,13 +5,14 @@ import { CancellationToken } from '../../../../../../base/common/cancellation.js'; import { Codicon } from '../../../../../../base/common/codicons.js'; -import { Emitter } from '../../../../../../base/common/event.js'; +import { Emitter, Event } from '../../../../../../base/common/event.js'; import { Disposable } from '../../../../../../base/common/lifecycle.js'; import { extUriBiasedIgnorePathCase } from '../../../../../../base/common/resources.js'; import { URI } from '../../../../../../base/common/uri.js'; import { generateUuid } from '../../../../../../base/common/uuid.js'; -import { AgentSession, type IAgentConnection } from '../../../../../../platform/agentHost/common/agentService.js'; +import { AgentSession, type IAgentSessionMetadata } from '../../../../../../platform/agentHost/common/agentService.js'; import type { ChangesSummary } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import type { INotification } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; import { SessionStatus, type SessionSummary } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js'; import { ChatSessionStatus, IChatNewSessionRequest, IChatSessionItem, IChatSessionItemController, IChatSessionItemsDelta } from '../../../common/chatSessionsService.js'; @@ -31,6 +32,15 @@ function mapSessionStatus(status: SessionStatus | undefined): ChatSessionStatus return ChatSessionStatus.Completed; } +/** + * Minimal agent-host connection surface needed by the session list controller. + */ +export interface IAgentHostSessionListConnection { + readonly onDidNotification: Event; + listSessions(): Promise; + disposeSession(session: URI): Promise; +} + /** * Provides session list items for the chat sessions sidebar by querying * active sessions from an agent host connection. Listens to protocol @@ -60,6 +70,7 @@ export class AgentHostSessionListController extends Disposable implements IChatS * replacement), so this flag naturally resets on reconnect. */ private _cacheValid = false; + private _refreshInFlight: Promise | undefined; /** * Incremented whenever the in-memory list is mutated outside of * {@link refresh}. Used to detect races where a `root/sessionAdded`, @@ -73,7 +84,7 @@ export class AgentHostSessionListController extends Disposable implements IChatS constructor( private readonly _sessionType: string, private readonly _provider: string, - private readonly _connection: IAgentConnection, + private readonly _connection: IAgentHostSessionListConnection, private readonly _description: string | undefined, _connectionAuthority: string, @IAgentHostUntitledProvisionalSessionService private readonly _provisional: IAgentHostUntitledProvisionalSessionService, @@ -217,6 +228,19 @@ export class AgentHostSessionListController extends Disposable implements IChatS } async refresh(token: CancellationToken): Promise { + if (this._refreshInFlight) { + return this._refreshInFlight; + } + + this._refreshInFlight = this._doRefresh(token); + try { + await this._refreshInFlight; + } finally { + this._refreshInFlight = undefined; + } + } + + private async _doRefresh(token: CancellationToken): Promise { if (this._cacheValid) { // Cache is kept in sync by notify/sessionAdded, // notify/sessionRemoved, and notify/sessionSummaryChanged. No @@ -252,7 +276,7 @@ export class AgentHostSessionListController extends Disposable implements IChatS // instead of overwriting the just-updated `_items` / // `_cachedSummaries`. if (startGeneration !== this._mutationGeneration) { - return this.refresh(token); + return this._doRefresh(token); } const filtered = sessions.filter(s => AgentSession.provider(s.session) === this._provider diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts index f2953642d38b1..0a5172de4006d 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts @@ -40,7 +40,7 @@ import { IOpenerService } from '../../../../../../platform/opener/common/opener. import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { IOutputService } from '../../../../../services/output/common/output.js'; import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js'; -import { AgentHostContribution, AgentHostSessionListController, AgentHostSessionHandler } from '../../../browser/agentSessions/agentHost/agentHostChatContribution.js'; +import { AgentHostContribution, AgentHostSessionListController, AgentHostSessionHandler, CoalescingAgentHostSessionListConnection } from '../../../browser/agentSessions/agentHost/agentHostChatContribution.js'; import { AgentHostLanguageModelProvider } from '../../../browser/agentSessions/agentHost/agentHostLanguageModelProvider.js'; import { IFileService } from '../../../../../../platform/files/common/files.js'; import { TestFileService } from '../../../../../test/common/workbenchTestServices.js'; @@ -1232,6 +1232,78 @@ suite('AgentHostChatContribution', () => { assert.strictEqual(listController.items.length, 1); }); + test('refresh shares an in-flight listSessions RPC', async () => { + const { listController, agentHostService } = createContribution(disposables); + + agentHostService.addSession({ session: AgentSession.uri('copilot', 'aaa'), startTime: 1000, modifiedTime: 2000, summary: 'My session' }); + + let listCalls = 0; + const originalListSessions = agentHostService.listSessions.bind(agentHostService); + const releaseListSessions = disposables.add(new Emitter()); + const released = Event.toPromise(releaseListSessions.event); + agentHostService.listSessions = async () => { + listCalls++; + await released; + return originalListSessions(); + }; + + const firstRefresh = listController.refresh(CancellationToken.None); + const secondRefresh = listController.refresh(CancellationToken.None); + await timeout(0); + assert.strictEqual(listCalls, 1); + + releaseListSessions.fire(); + await Promise.all([firstRefresh, secondRefresh]); + + assert.deepStrictEqual({ + listCalls, + labels: listController.items.map(item => item.label), + }, { + listCalls: 1, + labels: ['My session'], + }); + }); + + test('controllers sharing a connection coalesce their listSessions RPCs', async () => { + const { instantiationService, agentHostService } = createTestServices(disposables); + + agentHostService.addSession({ session: AgentSession.uri('copilot', 'aaa'), startTime: 1000, modifiedTime: 2000, summary: 'Copilot session' }); + + let listCalls = 0; + const originalListSessions = agentHostService.listSessions.bind(agentHostService); + const releaseListSessions = disposables.add(new Emitter()); + const released = Event.toPromise(releaseListSessions.event); + agentHostService.listSessions = async () => { + listCalls++; + await released; + return originalListSessions(); + }; + + // One shared connection, two controllers for different providers — the + // agent host should only be asked to enumerate sessions once. + const connection = new CoalescingAgentHostSessionListConnection(agentHostService); + const copilotController = disposables.add(instantiationService.createInstance(AgentHostSessionListController, 'agent-host-copilot', 'copilot', connection, undefined, 'local')); + const otherController = disposables.add(instantiationService.createInstance(AgentHostSessionListController, 'agent-host-other', 'other', connection, undefined, 'local')); + + const refreshCopilot = copilotController.refresh(CancellationToken.None); + const refreshOther = otherController.refresh(CancellationToken.None); + await timeout(0); + assert.strictEqual(listCalls, 1); + + releaseListSessions.fire(); + await Promise.all([refreshCopilot, refreshOther]); + + assert.deepStrictEqual({ + listCalls, + copilot: copilotController.items.map(item => item.label), + other: otherController.items.map(item => item.label), + }, { + listCalls: 1, + copilot: ['Copilot session'], + other: [], + }); + }); + test('refresh retries listSessions if the first call failed', async () => { const { listController, agentHostService } = createContribution(disposables); @@ -1336,10 +1408,19 @@ suite('AgentHostChatContribution', () => { // Open a workspace folder → only the in-workspace session should remain. folders = [{ uri: workspaceFolder, name: 'root', index: 0, toResource: () => workspaceFolder }]; + let listCalls = 0; + const originalListSessions = agentHostService.listSessions.bind(agentHostService); + agentHostService.listSessions = async () => { listCalls++; return originalListSessions(); }; onDidChangeWorkspaceFolders.fire({ added: [], removed: [], changed: [] }); await timeout(0); - assert.deepStrictEqual(listController.items.map(item => item.label), ['In workspace']); + assert.deepStrictEqual({ + listCalls, + labels: listController.items.map(item => item.label), + }, { + listCalls: 1, + labels: ['In workspace'], + }); }); test('sessionAdded notification filters out sessions outside the workspace', async () => { @@ -1560,6 +1641,91 @@ suite('AgentHostChatContribution', () => { }); }); + suite('CoalescingAgentHostSessionListConnection', () => { + + test('coalesces concurrent listSessions into a single delegate call', async () => { + const { agentHostService } = createTestServices(disposables); + + agentHostService.addSession({ session: AgentSession.uri('copilot', 'aaa'), startTime: 1000, modifiedTime: 2000, summary: 'My session' }); + + let listCalls = 0; + const originalListSessions = agentHostService.listSessions.bind(agentHostService); + const releaseListSessions = disposables.add(new Emitter()); + const released = Event.toPromise(releaseListSessions.event); + agentHostService.listSessions = async () => { + listCalls++; + await released; + return originalListSessions(); + }; + + const connection = new CoalescingAgentHostSessionListConnection(agentHostService); + const first = connection.listSessions(); + const second = connection.listSessions(); + await timeout(0); + assert.strictEqual(listCalls, 1); + + releaseListSessions.fire(); + const [a,] = await Promise.all([first, second]); + + assert.deepStrictEqual({ + listCalls, + summaries: a.map(s => s.summary), + }, { + listCalls: 1, + summaries: ['My session'], + }); + }); + + test('issues a fresh delegate call once the in-flight request settles', async () => { + const { agentHostService } = createTestServices(disposables); + + let listCalls = 0; + const originalListSessions = agentHostService.listSessions.bind(agentHostService); + agentHostService.listSessions = async () => { listCalls++; return originalListSessions(); }; + + const connection = new CoalescingAgentHostSessionListConnection(agentHostService); + await connection.listSessions(); + await connection.listSessions(); + + assert.strictEqual(listCalls, 2); + }); + + test('delivers a rejected listSessions to all callers and clears the in-flight slot', async () => { + const { agentHostService } = createTestServices(disposables); + + let listCalls = 0; + const originalListSessions = agentHostService.listSessions.bind(agentHostService); + agentHostService.listSessions = async () => { + listCalls++; + if (listCalls === 1) { + throw new Error('boom'); + } + return originalListSessions(); + }; + + const connection = new CoalescingAgentHostSessionListConnection(agentHostService); + const first = connection.listSessions(); + const second = connection.listSessions(); + const firstError = await first.then(() => undefined, err => err.message); + const secondError = await second.then(() => undefined, err => err.message); + + // The failed request must not be cached; a subsequent call re-fetches. + const retried = await connection.listSessions(); + + assert.deepStrictEqual({ + firstError, + secondError, + listCalls, + retried, + }, { + firstError: 'boom', + secondError: 'boom', + listCalls: 2, + retried: [], + }); + }); + }); + // ---- Session ID resolution in _invokeAgent -------------------------- suite('session ID resolution', () => { From e3badb65a887983b9d1b60078564f980e9cabc82 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 18 Jun 2026 23:00:52 -0700 Subject: [PATCH 4/8] Model hover redesign based on feedback (#322003) --- .../vscode-node/languageModelAccess.ts | 1 + .../endpoint/common/endpointProvider.ts | 1 + .../platform/endpoint/node/chatEndpoint.ts | 2 + .../platform/networking/common/networking.ts | 1 + .../common/extensionsApiProposals.ts | 2 +- .../api/common/extHostLanguageModels.ts | 2 + .../chatSessionPickerActionItem.ts | 4 +- .../browser/widget/input/chatModelPicker.ts | 135 +++++++---------- .../chat/browser/widget/media/chat.css | 136 ++++++++++++------ .../contrib/chat/common/languageModels.ts | 1 + .../vscode.proposed.languageModelPricing.d.ts | 12 ++ 11 files changed, 173 insertions(+), 124 deletions(-) diff --git a/extensions/copilot/src/extension/conversation/vscode-node/languageModelAccess.ts b/extensions/copilot/src/extension/conversation/vscode-node/languageModelAccess.ts index 32229b434080b..4017dc52bc67b 100644 --- a/extensions/copilot/src/extension/conversation/vscode-node/languageModelAccess.ts +++ b/extensions/copilot/src/extension/conversation/vscode-node/languageModelAccess.ts @@ -380,6 +380,7 @@ export class LanguageModelAccess extends Disposable implements IExtensionContrib longContextCacheWriteCost: endpoint instanceof AutoChatEndpoint ? undefined : endpoint.tokenPricing?.longContext?.cacheWriteTokenPrice, multiplierNumeric: endpoint instanceof AutoChatEndpoint ? undefined : endpoint.multiplier, priceCategory: endpoint instanceof AutoChatEndpoint ? undefined : endpoint.priceCategory, + category: endpoint instanceof AutoChatEndpoint ? undefined : endpoint.modelPickerCategory, detail: modelDetail, statusIcon: endpoint.degradationReason ? new vscode.ThemeIcon('warning') : undefined, version: endpoint.version, diff --git a/extensions/copilot/src/platform/endpoint/common/endpointProvider.ts b/extensions/copilot/src/platform/endpoint/common/endpointProvider.ts index 959f3bd4ea7ef..647b162de87ac 100644 --- a/extensions/copilot/src/platform/endpoint/common/endpointProvider.ts +++ b/extensions/copilot/src/platform/endpoint/common/endpointProvider.ts @@ -117,6 +117,7 @@ export interface IModelAPIResponse { info_messages?: { code: string; message: string }[]; billing?: IModelBilling; model_picker_price_category?: string; + model_picker_category?: string; capabilities: IChatModelCapabilities | ICompletionModelCapabilities | IEmbeddingModelCapabilities; supported_endpoints?: ModelSupportedEndpoint[]; custom_model?: CustomModel; diff --git a/extensions/copilot/src/platform/endpoint/node/chatEndpoint.ts b/extensions/copilot/src/platform/endpoint/node/chatEndpoint.ts index 5f446dbebdce2..4f6f23037f694 100644 --- a/extensions/copilot/src/platform/endpoint/node/chatEndpoint.ts +++ b/extensions/copilot/src/platform/endpoint/node/chatEndpoint.ts @@ -138,6 +138,7 @@ export class ChatEndpoint implements IChatEndpoint { public readonly restrictedToSkus?: string[] | undefined; public readonly tokenPricing?: IChatEndpointTokenPricing | undefined; public readonly priceCategory?: string | undefined; + public readonly modelPickerCategory?: string | undefined; public readonly customModel?: CustomModel | undefined; public readonly maxPromptImages?: number | undefined; @@ -175,6 +176,7 @@ export class ChatEndpoint implements IChatEndpoint { longContext: normalized.longContext ? { inputPrice: normalized.longContext.inputPrice, outputPrice: normalized.longContext.outputPrice, cacheReadTokenPrice: normalized.longContext.cachePrice, cacheWriteTokenPrice: normalized.longContext.cacheWritePrice, contextMax: normalized.longContext.contextMax } : undefined, } : undefined; this.priceCategory = modelMetadata.model_picker_price_category; + this.modelPickerCategory = modelMetadata.model_picker_category; this.isFallback = modelMetadata.is_chat_fallback; this.supportsToolCalls = !!modelMetadata.capabilities.supports.tool_calls; this.supportsVision = !!modelMetadata.capabilities.supports.vision; diff --git a/extensions/copilot/src/platform/networking/common/networking.ts b/extensions/copilot/src/platform/networking/common/networking.ts index 6f13fd66e8845..d85cd20361496 100644 --- a/extensions/copilot/src/platform/networking/common/networking.ts +++ b/extensions/copilot/src/platform/networking/common/networking.ts @@ -347,6 +347,7 @@ export interface IChatEndpoint extends IEndpoint { */ readonly tokenPricing?: IChatEndpointTokenPricing; readonly priceCategory?: string; + readonly modelPickerCategory?: string; readonly isFallback: boolean; readonly customModel?: CustomModel; readonly isExtensionContributed?: boolean; diff --git a/src/vs/platform/extensions/common/extensionsApiProposals.ts b/src/vs/platform/extensions/common/extensionsApiProposals.ts index d682eb9bce4c9..ffee8e7b664fd 100644 --- a/src/vs/platform/extensions/common/extensionsApiProposals.ts +++ b/src/vs/platform/extensions/common/extensionsApiProposals.ts @@ -532,5 +532,5 @@ const _allApiProposals = { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.workspaceTrust.d.ts', } }; -export const allApiProposals = Object.freeze<{ [proposalName: string]: Readonly<{ proposal: string; version?: number }> }>(_allApiProposals); +export const allApiProposals = Object.freeze<{ [proposalName: string]: Readonly<{ proposal: string }> }>(_allApiProposals); export type ApiProposalName = keyof typeof _allApiProposals; diff --git a/src/vs/workbench/api/common/extHostLanguageModels.ts b/src/vs/workbench/api/common/extHostLanguageModels.ts index 871abb627c4e5..016b6bd3f747f 100644 --- a/src/vs/workbench/api/common/extHostLanguageModels.ts +++ b/src/vs/workbench/api/common/extHostLanguageModels.ts @@ -232,6 +232,7 @@ export class ExtHostLanguageModels implements ExtHostLanguageModelsShape { longContextCacheCost: m.longContextCacheCost, longContextCacheWriteCost: m.longContextCacheWriteCost, priceCategory: m.priceCategory, + category: m.category, maxInputTokens: m.maxInputTokens, maxOutputTokens: m.maxOutputTokens, auth, @@ -438,6 +439,7 @@ export class ExtHostLanguageModels implements ExtHostLanguageModelsShape { longContextCacheCost: model.metadata.longContextCacheCost, longContextCacheWriteCost: model.metadata.longContextCacheWriteCost, priceCategory: model.metadata.priceCategory, + category: model.metadata.category, capabilities: { supportsImageToText: model.metadata.capabilities?.vision ?? false, supportsToolCalling: !!model.metadata.capabilities?.toolCalling, diff --git a/src/vs/workbench/contrib/chat/browser/chatSessions/chatSessionPickerActionItem.ts b/src/vs/workbench/contrib/chat/browser/chatSessions/chatSessionPickerActionItem.ts index d4faf96464154..b76ec6816c993 100644 --- a/src/vs/workbench/contrib/chat/browser/chatSessions/chatSessionPickerActionItem.ts +++ b/src/vs/workbench/contrib/chat/browser/chatSessions/chatSessionPickerActionItem.ts @@ -22,7 +22,6 @@ import { localize } from '../../../../../nls.js'; import { URI } from '../../../../../base/common/uri.js'; import { IChatInputPickerOptions } from '../widget/input/chatInputPickerActionItem.js'; import { autorun } from '../../../../../base/common/observable.js'; -import { IOpenerService } from '../../../../../platform/opener/common/opener.js'; import { IChatEntitlementService } from '../../../../services/chat/common/chatEntitlementService.js'; import { IActionListItemHover } from '../../../../../platform/actionWidget/browser/actionList.js'; import { getModelHoverContent } from '../widget/input/chatModelPicker.js'; @@ -55,7 +54,6 @@ export class ChatSessionPickerActionItem extends ActionWidgetDropdownActionViewI @IKeybindingService keybindingService: IKeybindingService, @ICommandService protected readonly commandService: ICommandService, @ITelemetryService telemetryService: ITelemetryService, - @IOpenerService private readonly openerService: IOpenerService, @IChatEntitlementService private readonly chatEntitlementService: IChatEntitlementService, ) { const { group, item } = initialState; @@ -192,7 +190,7 @@ export class ChatSessionPickerActionItem extends ActionWidgetDropdownActionViewI isDefaultForLocation: {}, }, }; - const hover = getModelHoverContent(syntheticModel, this.openerService, isUBB); + const hover = getModelHoverContent(syntheticModel, isUBB); if (hover) { return { content: hover.element, disposable: hover.disposable }; } diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts b/src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts index 6a4b8170da7a1..5b99a03dc5b49 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts @@ -5,7 +5,6 @@ import * as dom from '../../../../../../base/browser/dom.js'; import { StandardKeyboardEvent } from '../../../../../../base/browser/keyboardEvent.js'; -import { renderMarkdown } from '../../../../../../base/browser/markdownRenderer.js'; import { renderIcon } from '../../../../../../base/browser/ui/iconLabel/iconLabels.js'; import { Button } from '../../../../../../base/browser/ui/button/button.js'; import { getBaseLayerHoverDelegate } from '../../../../../../base/browser/ui/hover/hoverDelegate2.js'; @@ -221,7 +220,7 @@ function createModelItem( onConfigure?: (model: ILanguageModelChatMetadataAndIdentifier, group: string) => void, ): IActionListItem { const hover = model && openerService - ? getModelHoverContent(model, openerService, isUBB, onConfigure ? (group) => onConfigure(model, group) : undefined) + ? getModelHoverContent(model, isUBB, onConfigure ? (group) => onConfigure(model, group) : undefined) : undefined; return { item: action, @@ -315,26 +314,22 @@ function isHighCostCategory(priceCategory: string | undefined): boolean { } /** - * Resolves the context-size column headers (e.g. "200K" / "1M") from the model's - * context-size configuration. The smallest window is treated as the default context - * and the largest as the long context. Falls back to the model's max input tokens. + * Returns a display label for the model category tag (e.g. "Versatile", "Powerful"). */ -function getContextSizeLabels(model: ILanguageModelChatMetadataAndIdentifier): { defaultLabel?: string; longLabel?: string } { - const properties = model.metadata.configurationSchema?.properties; - if (properties) { - for (const propSchema of Object.values(properties)) { - if (propSchema.group !== 'tokens' || !propSchema.enum || propSchema.enum.length < 2) { - continue; - } - const entries = propSchema.enum.map((value, index) => ({ - value: Number(value), - label: propSchema.enumItemLabels?.[index] ?? formatTokenCount(Number(value)), - })); - entries.sort((a, b) => a.value - b.value); - return { defaultLabel: entries[0].label, longLabel: entries[entries.length - 1].label }; - } +function getCategoryLabel(category: string | undefined): string | undefined { + switch (category) { + case undefined: + case '': + return undefined; + case 'lightweight': + return localize('chat.category.lightweight', "Lightweight"); + case 'versatile': + return localize('chat.category.versatile', "Versatile"); + case 'powerful': + return localize('chat.category.powerful', "Powerful"); + default: + return category.charAt(0).toUpperCase() + category.slice(1); } - return { defaultLabel: model.metadata.maxInputTokens ? formatTokenCount(model.metadata.maxInputTokens) : undefined }; } /** @@ -1540,109 +1535,88 @@ export class ModelPickerWidget extends Disposable { */ const SUPPORTED_CONFIG_GROUPS: readonly string[] = ['navigation', 'tokens']; -export function getModelHoverContent(model: ILanguageModelChatMetadataAndIdentifier, openerService: IOpenerService, isUBB?: boolean, onConfigure?: (group: string) => void): { element: HTMLElement; disposable: DisposableStore } | undefined { +export function getModelHoverContent(model: ILanguageModelChatMetadataAndIdentifier, isUBB?: boolean, onConfigure?: (group: string) => void): { element: HTMLElement; disposable: DisposableStore } | undefined { const isAuto = isAutoModel(model); const container = dom.$('.chat-model-hover'); const disposables = new DisposableStore(); - // --- Title row: model name + price category badge (top-right) --- + // --- Title row: model name + category tag + price category badge (top-right) --- const titleRow = dom.$('.chat-model-hover-title-row'); titleRow.appendChild(dom.$('.chat-model-hover-name', undefined, model.metadata.name)); + const tags = dom.$('.chat-model-hover-title-tags'); + const categoryLabel = !isAuto ? getCategoryLabel(model.metadata.category) : undefined; + if (categoryLabel) { + tags.appendChild(dom.$('span.chat-model-hover-category', undefined, categoryLabel)); + } const priceCategoryLabel = (!isAuto && isUBB) ? getPriceCategoryLabel(model.metadata.priceCategory) : undefined; if (priceCategoryLabel) { const badge = dom.$('span.chat-model-hover-price-badge', undefined, priceCategoryLabel); if (isHighCostCategory(model.metadata.priceCategory)) { badge.classList.add('high-cost'); } - titleRow.appendChild(badge); + tags.appendChild(badge); } - container.appendChild(titleRow); - - // --- Description (tooltip as markdown) --- - if (model.metadata.tooltip) { - const descriptionContainer = dom.$('.chat-model-hover-description'); - const md = new MarkdownString('', { isTrusted: true, supportThemeIcons: true }); - if (model.metadata.statusIcon) { - md.appendMarkdown(`$(${model.metadata.statusIcon.id}) `); - } - md.appendMarkdown(model.metadata.tooltip); - const rendered = renderMarkdown(md, { - actionHandler: (url: string) => { - openerService.open(URI.parse(url), { allowCommands: true }); - }, - }); - disposables.add(rendered); - descriptionContainer.appendChild(rendered.element); - container.appendChild(descriptionContainer); + if (tags.childElementCount > 0) { + titleRow.appendChild(tags); } + container.appendChild(titleRow); // --- Cost info (UBB only) --- let costTableRendered = false; if (!isAuto && isUBB) { const metrics: { label: string; def: number | undefined; long: number | undefined }[] = [ { label: localize('models.inputCostLabel', "Input"), def: model.metadata.inputCost, long: model.metadata.longContextInputCost }, + { label: localize('models.outputCostLabel', "Output"), def: model.metadata.outputCost, long: model.metadata.longContextOutputCost }, { label: localize('models.cacheCostLabel', "Cache Read"), def: model.metadata.cacheCost, long: model.metadata.longContextCacheCost }, { label: localize('models.cacheWriteCostLabel', "Cache Write"), def: model.metadata.cacheWriteCost, long: model.metadata.longContextCacheWriteCost }, - { label: localize('models.outputCostLabel', "Output"), def: model.metadata.outputCost, long: model.metadata.longContextOutputCost }, ].filter(m => m.def !== undefined || m.long !== undefined); if (metrics.length > 0) { - const contextLabels = getContextSizeLabels(model); // Show the long-context column whenever any metric has a long-context price. const hasLongContext = metrics.some(m => m.long !== undefined); - container.appendChild(dom.$('.chat-model-hover-separator')); - const table = dom.$('.chat-model-hover-cost-table'); if (hasLongContext) { container.classList.add('has-long-context'); table.classList.add('has-long-context'); } - const appendCell = (text: string, ...classNames: string[]): void => { - table.appendChild(dom.$(`.chat-model-hover-cost-cell${classNames.map(c => '.' + c).join('')}`, undefined, text)); - }; - - // Renders a " credits" value cell with the count visually emphasized. - const appendCostCell = (cost: number | undefined): void => { + // Each value cell paints a dotted leader behind a right-aligned, background-masked + // number so the dots run right up to the number (and between the two columns). + const appendValueCell = (row: HTMLElement, cost: number | undefined): void => { if (cost === undefined) { - table.appendChild(dom.$('.chat-model-hover-cost-cell.value')); + row.appendChild(dom.$('span.chat-model-hover-cost-value.empty')); return; } - const unit = cost === 1 - ? localize('models.creditUnitSingular', "credit") - : localize('models.creditUnitPlural', "credits"); - table.appendChild(dom.$('.chat-model-hover-cost-cell.value', undefined, - dom.$('span.chat-model-hover-cost-count', undefined, String(cost)), - dom.$('span.chat-model-hover-cost-unit', undefined, ` ${unit}`), + row.appendChild(dom.$('span.chat-model-hover-cost-value', undefined, + dom.$('span.chat-model-hover-cost-number', undefined, String(cost)), )); }; - // Header row: "Cost" + context-size value(s) — context columns only shown with long context - appendCell(localize('models.priceTitle', "Cost"), 'header', 'title'); + // Header row: "Credits Per 1M Tokens" heading + (when long context) Default / Long Context labels + const headerRow = dom.$('.chat-model-hover-cost-row.header'); + headerRow.appendChild(dom.$('span.chat-model-hover-cost-heading', undefined, localize('models.creditsPerMillionTokens', "Credits Per 1M Tokens"))); if (hasLongContext) { - appendCell(contextLabels.defaultLabel ?? '', 'header', 'value'); - appendCell(contextLabels.longLabel ?? '', 'header', 'value'); - - // Subheader row: "per 1M tokens" + context column labels - appendCell(localize('models.perMillionTokens', "per 1M tokens"), 'subheader', 'label'); - appendCell(localize('models.defaultContext', "Default Context"), 'subheader', 'value'); - appendCell(localize('models.longContext', "Long Context"), 'subheader', 'value'); + headerRow.appendChild(dom.$('span.chat-model-hover-cost-value.subheader', undefined, localize('models.defaultContext', "Default"))); + headerRow.appendChild(dom.$('span.chat-model-hover-cost-value.subheader', undefined, localize('models.longContext', "Long Context"))); } else { - // Single price: still surface the (max) context size in the header, - // and keep "per 1M tokens" beneath the "Cost" label. - appendCell(contextLabels.defaultLabel ?? '', 'header', 'value'); - appendCell(localize('models.perMillionTokens', "per 1M tokens"), 'subheader', 'label'); - appendCell(localize('models.defaultContext', "Default Context"), 'subheader', 'value'); + // Placeholder so the header occupies the full row and grid columns stay aligned. + headerRow.appendChild(dom.$('span.chat-model-hover-cost-value.subheader')); } + table.appendChild(headerRow); - // Cost rows + // Cost rows: label on the left, dotted leader, then right-aligned credit value(s) for (const metric of metrics) { - appendCell(metric.label, 'label'); - appendCostCell(metric.def); + const row = dom.$('.chat-model-hover-cost-row'); + const labelCell = dom.$('.chat-model-hover-cost-label'); + labelCell.appendChild(dom.$('span.chat-model-hover-cost-label-text', undefined, metric.label)); + labelCell.appendChild(dom.$('span.chat-model-hover-cost-leader')); + row.appendChild(labelCell); + appendValueCell(row, metric.def); if (hasLongContext) { - appendCostCell(metric.long); + appendValueCell(row, metric.long); } + table.appendChild(row); } container.appendChild(table); @@ -1677,18 +1651,19 @@ export function getModelHoverContent(model: ILanguageModelChatMetadataAndIdentif } } if (configButtons.length > 0) { - container.appendChild(dom.$('.chat-model-hover-separator')); const configRow = dom.$('.chat-model-hover-configurable'); + configRow.appendChild(dom.$('span.chat-model-hover-configurable-label', undefined, localize('models.configurable', "Configurable"))); + const buttonsContainer = dom.$('.chat-model-hover-configurable-buttons'); for (const { group, label } of configButtons) { - const button = disposables.add(new Button(configRow, { + const button = disposables.add(new Button(buttonsContainer, { ...defaultButtonStyles, secondary: true, - supportIcons: true, title: label, })); - button.label = `$(${Codicon.settingsGear.id}) ${label}`; + button.label = label; disposables.add(button.onDidClick(() => onConfigure?.(group))); } + configRow.appendChild(buttonsContainer); container.appendChild(configRow); } } diff --git a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css index 45a9cde76ffb6..f9a119c1e21ed 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css +++ b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css @@ -4256,8 +4256,8 @@ have to be updated for changes to the rules above, or to support more deeply nes .chat-model-hover { display: flex; flex-direction: column; - gap: 8px; - max-width: 250px; + gap: 24px; + /* max-width: 250px; */ } .chat-model-hover.has-long-context { @@ -4265,8 +4265,8 @@ have to be updated for changes to the rules above, or to support more deeply nes } .chat-model-hover-name { - font-weight: bold; - font-size: 13px; + font-weight: 500; + font-size: 14px; } .chat-model-hover-title-row { @@ -4291,60 +4291,108 @@ have to be updated for changes to the rules above, or to support more deeply nes border-color: var(--vscode-notificationsWarningIcon-foreground, var(--vscode-editorWarning-foreground)); } -.chat-model-hover-separator { - border-top: 1px solid var(--vscode-menu-separatorBackground, var(--vscode-editorWidget-border)); - margin: 2px 0; -} - -.chat-model-hover-description { - font-size: 12px; - line-height: 1.4; -} - -.monaco-workbench .chat-model-hover-description .codicon[class*='codicon-'] { - vertical-align: middle; - font-size: 12px; +.chat-model-hover-title-tags { + display: flex; + align-items: center; + gap: 8px; + flex: 0 0 auto; } -.chat-model-hover-description > div p { - margin: 0; +.chat-model-hover-category { + font-size: 11px; + /* color: var(--vscode-descriptionForeground); */ + white-space: nowrap; } .chat-model-hover-cost-table { display: grid; grid-template-columns: auto auto; - column-gap: 16px; - row-gap: 2px; + column-gap: 0; + row-gap: 12px; + align-items: center; font-size: 12px; - color: var(--vscode-descriptionForeground); } .chat-model-hover-cost-table.has-long-context { grid-template-columns: auto auto auto; } -.chat-model-hover-cost-cell.value { - text-align: right; +/* Rows are transparent: their cells participate directly in the table grid so columns align. */ +.chat-model-hover-cost-row { + display: contents; } -.chat-model-hover-cost-count { - font-weight: 500; +.chat-model-hover-cost-heading { color: var(--vscode-foreground); + white-space: nowrap; + padding-right: 12px; } -.chat-model-hover-cost-unit { - font-weight: normal; +/* Label cell: the text plus a dotted leader that fills the rest of the (heading-wide) first column. */ +.chat-model-hover-cost-label { + display: flex; + align-items: center; + gap: 6px; color: var(--vscode-descriptionForeground); + white-space: nowrap; } -.chat-model-hover-cost-cell.header { +.chat-model-hover-cost-label-text { + flex: 0 0 auto; +} + +.chat-model-hover-cost-leader { + flex: 1 1 auto; + min-width: 12px; + height: 0; + border-bottom: 1px dotted var(--vscode-descriptionForeground); + opacity: 0.5; +} + +/* Each value cell paints a dotted leader behind a right-aligned, background-masked number. */ +.chat-model-hover-cost-value { + position: relative; + text-align: right; + white-space: nowrap; + min-width: 0; +} + +.chat-model-hover-cost-value::before { + content: ''; + position: absolute; + left: 0; + right: 0; + top: 50%; + border-top: 1px dotted var(--vscode-descriptionForeground); + opacity: 0.5; +} + +.chat-model-hover-cost-value.subheader::before, +.chat-model-hover-cost-value.empty::before { + content: none; +} + +/* Blank gap (no dots) separating the Default and Long Context value columns. */ +.chat-model-hover-cost-table.has-long-context .chat-model-hover-cost-value + .chat-model-hover-cost-value { + padding-left: 18px; +} + +.chat-model-hover-cost-table.has-long-context .chat-model-hover-cost-value + .chat-model-hover-cost-value::before { + left: 18px; +} + +.chat-model-hover-cost-number { + position: relative; + padding-left: 6px; + font-weight: 700; color: var(--vscode-foreground); - font-weight: 600; + background-color: var(--vscode-editorHoverWidget-background, var(--vscode-editorWidget-background)); } -.chat-model-hover-cost-cell.subheader { - font-size: 11px; - padding-bottom: 4px; +.chat-model-hover-cost-value.subheader { + font-weight: normal; + color: var(--vscode-descriptionForeground); + padding-left: 6px; } .chat-model-hover-cost { @@ -4369,18 +4417,26 @@ have to be updated for changes to the rules above, or to support more deeply nes .chat-model-hover-configurable { display: flex; - flex-wrap: wrap; align-items: center; - gap: 6px; + gap: 8px; font-size: 12px; } -.chat-model-hover-configurable > .monaco-button { +.chat-model-hover-configurable-label { + flex: 0 0 auto; + color: var(--vscode-descriptionForeground); + white-space: nowrap; +} + +.chat-model-hover-configurable-buttons { + display: flex; + flex-wrap: wrap; + justify-content: flex-start; + gap: 6px; +} + +.chat-model-hover-configurable-buttons > .monaco-button { width: auto; padding: 2px 8px; font-size: 11px; } - -.monaco-workbench .chat-model-hover-configurable > .monaco-button .codicon[class*='codicon-'] { - font-size: var(--vscode-codiconFontSize-compact); -} diff --git a/src/vs/workbench/contrib/chat/common/languageModels.ts b/src/vs/workbench/contrib/chat/common/languageModels.ts index 5d7f09492e8d3..3f7136d838fd6 100644 --- a/src/vs/workbench/contrib/chat/common/languageModels.ts +++ b/src/vs/workbench/contrib/chat/common/languageModels.ts @@ -208,6 +208,7 @@ export interface ILanguageModelChatMetadata { readonly longContextCacheWriteCost?: number; readonly longContextOutputCost?: number; readonly priceCategory?: string; + readonly category?: string; readonly family: string; readonly maxInputTokens: number; readonly maxOutputTokens: number; diff --git a/src/vscode-dts/vscode.proposed.languageModelPricing.d.ts b/src/vscode-dts/vscode.proposed.languageModelPricing.d.ts index 57025acab04eb..2d09642c90c42 100644 --- a/src/vscode-dts/vscode.proposed.languageModelPricing.d.ts +++ b/src/vscode-dts/vscode.proposed.languageModelPricing.d.ts @@ -69,6 +69,12 @@ declare module 'vscode' { * Displayed in the model picker as a visual indicator of relative cost. */ readonly priceCategory?: string; + + /** + * Optional model category describing the model's tier (e.g. "lightweight", "versatile", "powerful"). + * Displayed in the model picker as a tag. Clients should be resilient to additional categories. + */ + readonly category?: string; } export interface LanguageModelChat { @@ -127,5 +133,11 @@ declare module 'vscode' { * Displayed in the model picker as a visual indicator of relative cost. */ readonly priceCategory?: string; + + /** + * Optional model category describing the model's tier (e.g. "lightweight", "versatile", "powerful"). + * Displayed in the model picker as a tag. Clients should be resilient to additional categories. + */ + readonly category?: string; } } From f7512c826725d05674127068c8f3b2f5628c5436 Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Thu, 18 Jun 2026 23:08:39 -0700 Subject: [PATCH 5/8] AH: improve task complete + make sure to not show goal (#320894) --- .../node/copilot/copilotAgentSession.ts | 4 +- .../node/copilot/copilotToolDisplay.ts | 35 ++++++++++ .../node/copilot/mapSessionEvents.ts | 15 +++- .../test/node/copilotAgentSession.test.ts | 2 +- .../test/node/copilotToolDisplay.test.ts | 28 +++++++- .../test/node/historyRecordFixtures.test.ts | 2 +- .../test/node/mapSessionEvents.test.ts | 70 +++++++++++++++++++ .../contrib/chat/browser/widget/chatWidget.ts | 7 +- 8 files changed, 155 insertions(+), 8 deletions(-) create mode 100644 src/vs/platform/agentHost/test/node/mapSessionEvents.test.ts diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index ff9a84d5c2dca..0f45446f659cc 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -47,7 +47,7 @@ import { isPromptInvokedCopilotSlashCommand, isRuntimeCopilotSlashCommand, parse import type { IUnsandboxedCommandConfirmationRequest, ShellManager } from './copilotShellTools.js'; import { buildSandboxConfigForSdk } from './sandboxConfigForSdk.js'; import type { IAgentServerToolHost } from '../../common/agentServerTools.js'; -import { getEditFilePaths, getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getSubagentMetadata, getTaskCompleteSummary, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, isShellTool, isTaskCompleteTool, synthesizeSkillToolCall, tryStringify, type ITypedPermissionRequest } from './copilotToolDisplay.js'; +import { getEditFilePaths, getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getSubagentMetadata, getTaskCompleteMarkdown, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, isShellTool, isTaskCompleteTool, synthesizeSkillToolCall, tryStringify, type ITypedPermissionRequest } from './copilotToolDisplay.js'; import { FileEditTracker } from '../shared/fileEditTracker.js'; import { stripProxyErrorMarker, tryBuildChatErrorMeta, tryBuildChatErrorMetaFromFields } from '../shared/forwardedChatError.js'; import { McpCustomizationController, type ISdkMcpServer } from '../shared/mcpCustomizationController.js'; @@ -2251,7 +2251,7 @@ export class CopilotAgentSession extends Disposable { if (isTaskCompleteTool(tracked.toolName)) { this._sendToolInvokedTelemetry(e.data.success, e.data.error?.code, tracked); - const summary = getTaskCompleteSummary(tracked.parameters, toolOutput); + const summary = getTaskCompleteMarkdown(tracked.parameters, toolOutput); if (summary) { this._emitAction({ type: ActionType.ChatResponsePart, diff --git a/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts b/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts index 18c6808ac256c..fc0b847bed2ab 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts @@ -398,6 +398,41 @@ export function getTaskCompleteSummary(parameters: Record | und return typeof summary === 'string' && summary.trim().length > 0 ? summary : undefined; } +/** + * Formats the Autopilot completion summary as the markdown response part + * content, including the localized prefix. + */ +export function getTaskCompleteMarkdown(parameters: Record | undefined, toolOutput: string | undefined): string | undefined { + const summary = getTaskCompleteSummary(parameters, toolOutput); + if (!summary) { + return undefined; + } + return '\n\n' + localize('toolMarkdown.taskComplete', "**Task completed:** {0}", summary); +} + +/** + * Returns true if the tool should render as a markdown response part instead + * of a tool-call entry. + */ +export function isMarkdownRenderedTool(toolName: string): boolean { + return isTaskCompleteTool(toolName); +} + +/** + * Returns markdown content for tools rendered as inline markdown response + * parts. + */ +export function getToolMarkdownContent(toolName: string, parameters: Record | undefined): string | undefined { + if (!isMarkdownRenderedTool(toolName)) { + return undefined; + } + const summary = getTaskCompleteSummary(parameters, undefined); + if (!summary) { + return undefined; + } + return getTaskCompleteMarkdown(parameters, undefined); +} + /** * Returns true if the tool executes shell commands. */ diff --git a/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts b/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts index 03ac95b90f831..caf218651ba9d 100644 --- a/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts +++ b/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts @@ -13,7 +13,7 @@ import { stripRedundantCdPrefix } from '../../common/commandLineHelpers.js'; import { IFileEditRecord, ISessionDatabase } from '../../common/sessionDataService.js'; import { MessageAttachmentKind, type MessageAttachment } from '../../common/state/protocol/state.js'; import { MessageKind, ResponsePartKind, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildSubagentSessionUri, type Message, type ResponsePart, type StringOrMarkdown, type ToolCallCompletedState, type ToolResultContent, type Turn } from '../../common/state/sessionState.js'; -import { getInvocationMessage, getPastTenseMessage, getShellLanguage, getSubagentMetadata, getTaskCompleteSummary, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, isTaskCompleteTool, synthesizeSkillToolCall } from './copilotToolDisplay.js'; +import { getInvocationMessage, getPastTenseMessage, getShellLanguage, getSubagentMetadata, getTaskCompleteMarkdown, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, isTaskCompleteTool, synthesizeSkillToolCall } from './copilotToolDisplay.js'; import { buildSessionDbUri } from '../shared/fileEditTracker.js'; import { getMediaMime } from '../../../../base/common/mime.js'; @@ -440,7 +440,7 @@ export async function mapSessionEvents( if (!builder) { continue; } - const summary = getTaskCompleteSummary(info.parameters, d.error?.message ?? d.result?.content); + const summary = getTaskCompleteMarkdown(info.parameters, d.error?.message ?? d.result?.content); if (summary) { builder.responseParts.push({ kind: ResponsePartKind.Markdown, @@ -514,6 +514,17 @@ export async function mapSessionEvents( if (!info) { continue; } + if (isTaskCompleteTool(info.toolName)) { + const summary = getTaskCompleteMarkdown(info.parameters, completion?.error?.message ?? completion?.result?.content); + if (summary) { + builder.responseParts.push({ + kind: ResponsePartKind.Markdown, + id: generateUuid(), + content: summary, + }); + } + continue; + } builder.responseParts.push(makeCompletedToolCallPart( completion ?? { toolCallId: request.toolCallId, success: true }, info, diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index 4cc74377a7d71..b1b3954ad2164 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -1823,7 +1823,7 @@ suite('CopilotAgentSession', () => { assert.deepStrictEqual(responsePart.part, { kind: ResponsePartKind.Markdown, id: responsePart.part.id, - content: 'Completed the requested work.', + content: '\n\n**Task completed:** Completed the requested work.', }); }); diff --git a/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts b/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts index d7a36fb862574..5da1df74d6474 100644 --- a/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts @@ -6,7 +6,7 @@ import assert from 'assert'; import { URI } from '../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; -import { getEditFilePath, getEditFilePaths, getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, synthesizeSkillToolCall, type ITypedPermissionRequest } from '../../node/copilot/copilotToolDisplay.js'; +import { getEditFilePath, getEditFilePaths, getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getToolDisplayName, getToolInputString, getToolKind, getToolMarkdownContent, isEditTool, isHiddenTool, isMarkdownRenderedTool, synthesizeSkillToolCall, type ITypedPermissionRequest } from '../../node/copilot/copilotToolDisplay.js'; suite('copilotToolDisplay — friendly tool names', () => { @@ -98,6 +98,32 @@ suite('copilotToolDisplay — edit tool classification', () => { }); }); +suite('copilotToolDisplay — markdown-rendered tools', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + test('task_complete renders as markdown, other tools do not', () => { + assert.strictEqual(isMarkdownRenderedTool('task_complete'), true); + assert.strictEqual(isMarkdownRenderedTool('bash'), false); + assert.strictEqual(isMarkdownRenderedTool('report_intent'), false); + }); + + test('getToolMarkdownContent returns the task_complete summary when present', () => { + assert.strictEqual(getToolMarkdownContent('task_complete', { summary: 'All tests pass.' }), '\n\n**Task completed:** All tests pass.'); + }); + + test('getToolMarkdownContent returns undefined for empty, missing, or non-string summaries', () => { + assert.strictEqual(getToolMarkdownContent('task_complete', { summary: '' }), undefined); + assert.strictEqual(getToolMarkdownContent('task_complete', {}), undefined); + assert.strictEqual(getToolMarkdownContent('task_complete', undefined), undefined); + assert.strictEqual(getToolMarkdownContent('task_complete', { summary: 42 }), undefined); + }); + + test('getToolMarkdownContent returns undefined for non-markdown tools', () => { + assert.strictEqual(getToolMarkdownContent('bash', { summary: 'ignored' }), undefined); + }); +}); + suite('getPermissionDisplay — cd-prefix stripping', () => { ensureNoDisposablesAreLeakedInTestSuite(); diff --git a/src/vs/platform/agentHost/test/node/historyRecordFixtures.test.ts b/src/vs/platform/agentHost/test/node/historyRecordFixtures.test.ts index 5f4e290a3a471..2245226ab7183 100644 --- a/src/vs/platform/agentHost/test/node/historyRecordFixtures.test.ts +++ b/src/vs/platform/agentHost/test/node/historyRecordFixtures.test.ts @@ -99,7 +99,7 @@ suite('mapSessionEventsToHistoryRecords', () => { state: 'complete', parts: [ { kind: ResponsePartKind.ToolCall, toolName: 'view' }, - { kind: ResponsePartKind.Markdown, content: 'Reviewed index.html.' }, + { kind: ResponsePartKind.Markdown, content: '\n\n**Task completed:** Reviewed index.html.' }, ], }]); }); diff --git a/src/vs/platform/agentHost/test/node/mapSessionEvents.test.ts b/src/vs/platform/agentHost/test/node/mapSessionEvents.test.ts new file mode 100644 index 0000000000000..c407afc800c2e --- /dev/null +++ b/src/vs/platform/agentHost/test/node/mapSessionEvents.test.ts @@ -0,0 +1,70 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { AgentSession } from '../../common/agentService.js'; +import { ResponsePartKind, type ResponsePart } from '../../common/state/sessionState.js'; +import { mapSessionEvents, type ISessionEvent } from '../../node/copilot/mapSessionEvents.js'; + +suite('mapSessionEvents — task_complete rendering', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + const session = AgentSession.uri('copilot', 'test-session'); + + function partKinds(parts: readonly ResponsePart[]): Array<{ kind: ResponsePartKind; content?: string }> { + return parts.map(p => p.kind === ResponsePartKind.Markdown ? { kind: p.kind, content: p.content } : { kind: p.kind }); + } + + test('task_complete with a summary renders as a markdown part, not a tool call', async () => { + const events: ISessionEvent[] = [ + { type: 'user.message', data: { messageId: 'm1', content: 'hi' } }, + { type: 'assistant.message', data: { messageId: 'm2', content: 'Working on it.', toolRequests: [{ toolCallId: 'tc-1', name: 'task_complete' }] } }, + { type: 'tool.execution_start', data: { toolCallId: 'tc-1', toolName: 'task_complete', arguments: { summary: 'Done. All good.' } } }, + { type: 'tool.execution_complete', data: { toolCallId: 'tc-1', success: true } }, + ]; + + const { turns } = await mapSessionEvents(session, undefined, events); + + assert.strictEqual(turns.length, 1); + assert.deepStrictEqual(partKinds(turns[0].responseParts), [ + { kind: ResponsePartKind.Markdown, content: 'Working on it.' }, + { kind: ResponsePartKind.Markdown, content: '\n\n**Task completed:** Done. All good.' }, + ]); + }); + + test('task_complete without a summary renders nothing', async () => { + const events: ISessionEvent[] = [ + { type: 'user.message', data: { messageId: 'm1', content: 'hi' } }, + { type: 'assistant.message', data: { messageId: 'm2', content: 'All set.', toolRequests: [{ toolCallId: 'tc-1', name: 'task_complete' }] } }, + { type: 'tool.execution_start', data: { toolCallId: 'tc-1', toolName: 'task_complete', arguments: {} } }, + { type: 'tool.execution_complete', data: { toolCallId: 'tc-1', success: true } }, + ]; + + const { turns } = await mapSessionEvents(session, undefined, events); + + assert.strictEqual(turns.length, 1); + assert.deepStrictEqual(partKinds(turns[0].responseParts), [ + { kind: ResponsePartKind.Markdown, content: 'All set.' }, + ]); + }); + + test('a regular tool still renders as a tool call', async () => { + const events: ISessionEvent[] = [ + { type: 'user.message', data: { messageId: 'm1', content: 'hi' } }, + { type: 'assistant.message', data: { messageId: 'm2', content: '', toolRequests: [{ toolCallId: 'tc-1', name: 'bash' }] } }, + { type: 'tool.execution_start', data: { toolCallId: 'tc-1', toolName: 'bash', arguments: { command: 'echo hi' } } }, + { type: 'tool.execution_complete', data: { toolCallId: 'tc-1', success: true, result: { content: 'hi\n' } } }, + ]; + + const { turns } = await mapSessionEvents(session, undefined, events); + + assert.strictEqual(turns.length, 1); + assert.deepStrictEqual(partKinds(turns[0].responseParts), [ + { kind: ResponsePartKind.ToolCall }, + ]); + }); +}); diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts index 1cfb1af9fba24..2bfb99638d15e 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts @@ -2426,9 +2426,14 @@ export class ChatWidget extends Disposable implements IChatWidget { return; } + // The advanced autopilot goal banner is only supported in the local chat + // harness. Agent-host backed sessions (Copilot CLI, Claude, Codex and the + // local/remote agent hosts) must never render it. + const sessionResource = this.viewModel?.model.sessionResource; + const isLocalHarness = !!sessionResource && getChatSessionType(sessionResource) === localChatSessionType; const permissionLevel = inputPart.currentModeInfo?.permissionLevel; const goalModeOn = this.configurationService.getValue(ChatConfiguration.AutopilotAdvancedEnabled) === true; - if (permissionLevel !== ChatPermissionLevel.Autopilot || !goalModeOn) { + if (!isLocalHarness || permissionLevel !== ChatPermissionLevel.Autopilot || !goalModeOn) { this._cancelGoalSummary(); inputPart.clearGoalBanner(); return; From c178db3841a8f5c7fe12edc6379c8298ff2e019d Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Fri, 19 Jun 2026 00:21:06 -0700 Subject: [PATCH 6/8] chat: fix tool call confirmation message rendering (#322039) - Render file widget links in confirmation messages so markdown links in tool invocation text display as clickable file pills instead of plain markdown. Fixes 'Editing [file]' collapsing to just 'Editing'. - Remove toolInput from read file confirmations to prevent the generic JSON input panel from being shown during permission requests. - Update read permission display to use the view-tool invocation message format (e.g. 'Reading [file.json]' instead of plain text) and correct the confirmation title localization. Fixes #321797 (Commit message generated by Copilot) --- .../node/copilot/copilotToolDisplay.ts | 7 +++-- .../test/node/copilotToolDisplay.test.ts | 26 +++++++++++++++++++ .../chatModifiedFilesConfirmationSubPart.ts | 4 +++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts b/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts index fc0b847bed2ab..3a4c2d713536c 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts @@ -459,7 +459,7 @@ function truncate(text: string, maxLength: number): string { */ function formatPathAsMarkdownLink(path: string): string { const uri = URI.file(path); - return `[${basename(uri)}](${uri})`; + return `[${escapeMarkdownLinkLabel(basename(uri))}](${uri})`; } function formatUrlAsMarkdownLink(url: string): string { @@ -1071,9 +1071,8 @@ export function getPermissionDisplay(request: ITypedPermissionRequest, workingDi } case 'read': return { - confirmationTitle: localize('copilot.permission.read.title', "Read file?"), - invocationMessage: intention ?? getInvocationMessage(CopilotToolName.View, getToolDisplayName(CopilotToolName.View), path ? { path } : undefined), - toolInput: tryStringify(path ? { path, intention } : request) ?? undefined, + confirmationTitle: localize('copilot.permission.read.title', "Allow reading file outside of workspace?"), + invocationMessage: getInvocationMessage(CopilotToolName.View, getToolDisplayName(CopilotToolName.View), path ? { path } : undefined), permissionKind: 'read', permissionPath: path, }; diff --git a/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts b/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts index 5da1df74d6474..08bcbf9317bf5 100644 --- a/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts @@ -190,6 +190,32 @@ suite('getPermissionDisplay — cd-prefix stripping', () => { const display = getPermissionDisplay(request, wd); assert.strictEqual(display.toolInput, 'dir'); }); + +}); + +suite('getPermissionDisplay — read permission display', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + test('uses the view-tool invocation message for read permissions', () => { + const display = getPermissionDisplay({ + kind: 'read', + path: '/Users/connor/Downloads/context7-copilot-debug-main.json', + intention: 'Read file: /Users/connor/Downloads/context7-copilot-debug-main.json', + } as ITypedPermissionRequest, URI.file('/repo/project')); + + assert.deepStrictEqual({ + invocationMessage: display.invocationMessage, + toolInput: display.toolInput, + permissionKind: display.permissionKind, + permissionPath: display.permissionPath, + }, { + invocationMessage: { markdown: 'Reading [context7-copilot-debug-main.json](file:///Users/connor/Downloads/context7-copilot-debug-main.json)' }, + toolInput: undefined, + permissionKind: 'read', + permissionPath: '/Users/connor/Downloads/context7-copilot-debug-main.json', + }); + }); }); suite('view tool — view_range display', () => { diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatModifiedFilesConfirmationSubPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatModifiedFilesConfirmationSubPart.ts index 4c3e91c09eb30..385c9fd9f780d 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatModifiedFilesConfirmationSubPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatModifiedFilesConfirmationSubPart.ts @@ -25,6 +25,8 @@ import { IChatCodeBlockInfo, IChatWidgetService } from '../../../chat.js'; import { IChatToolRiskAssessmentService } from '../../../tools/chatToolRiskAssessmentService.js'; import { IChatContentPartRenderContext } from '../chatContentParts.js'; import { ChatCustomConfirmationWidget, IChatConfirmationButton } from '../chatConfirmationWidget.js'; +import { renderFileWidgets } from '../chatInlineAnchorWidget.js'; +import { IChatMarkdownAnchorService } from '../chatMarkdownAnchorService.js'; import { CollapsibleListPool, IChatCollapsibleListItem } from '../chatReferencesContentPart.js'; import { IEditorService } from '../../../../../../services/editor/common/editorService.js'; import { AbstractToolConfirmationSubPart } from './abstractToolConfirmationSubPart.js'; @@ -43,6 +45,7 @@ export class ChatModifiedFilesConfirmationSubPart extends AbstractToolConfirmati @IChatWidgetService chatWidgetService: IChatWidgetService, @ILanguageModelToolsService languageModelToolsService: ILanguageModelToolsService, @IMarkdownRendererService private readonly markdownRendererService: IMarkdownRendererService, + @IChatMarkdownAnchorService private readonly chatMarkdownAnchorService: IChatMarkdownAnchorService, @IEditorService private readonly editorService: IEditorService, @ICommandService private readonly commandService: ICommandService, @IChatToolRiskAssessmentService riskAssessmentService: IChatToolRiskAssessmentService, @@ -106,6 +109,7 @@ export class ChatModifiedFilesConfirmationSubPart extends AbstractToolConfirmati if (message) { const renderedMessage = this._register(this.markdownRendererService.render(typeof message === 'string' ? new MarkdownString(message) : message)); + renderFileWidgets(renderedMessage.element, this.instantiationService, this.chatMarkdownAnchorService, this._store); container.append(renderedMessage.element); } From f948a1d8dafe85f72c62373968e2ba5049954c21 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Fri, 19 Jun 2026 12:11:22 +0200 Subject: [PATCH 7/8] sessions: fix New Session button showing Ctrl+L instead of Cmd+N (#322070) The Agents window had two Cmd+N keybindings: the canonical new-session command (workbench.action.sessions.newChat, used by the New button) and a legacy duplicate rule binding Cmd+N to the workbench chat panel command (workbench.action.chat.newChat) in sessionsViewActions.ts. After #321910 bumped the legacy rule's weight to SessionsContrib (250), it outranked the canonical command (202). Because both shared the same when clause (editorArea.negate), the resolver evicted the lower-weight binding from the command->keybinding lookup map, so the New button fell back to its secondary Ctrl+L label. Remove the redundant legacy rule and raise the canonical action's keybinding weight to SessionsContrib, leaving a single Cmd+N binding on the command the button actually runs. The editor-area fallthrough from #319609 is preserved. --- .../contrib/chat/browser/chat.contribution.ts | 2 +- .../sessions/browser/views/sessionsViewActions.ts | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/chat.contribution.ts b/src/vs/sessions/contrib/chat/browser/chat.contribution.ts index 5cd569bff3a82..f7dbc6647f831 100644 --- a/src/vs/sessions/contrib/chat/browser/chat.contribution.ts +++ b/src/vs/sessions/contrib/chat/browser/chat.contribution.ts @@ -50,7 +50,7 @@ class NewChatInSessionsWindowAction extends Action2 { category: CHAT_CATEGORY, f1: true, keybinding: { - weight: KeybindingWeight.WorkbenchContrib + 2, + weight: KeybindingWeight.SessionsContrib, // Don't shadow Ctrl/Cmd+N (and Ctrl/Cmd+L) when focus is in the // editor area so the standard editor commands (new untitled file, // expand line selection) handle the shortcut instead. diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts index db1b02a2d6d00..b960833777b06 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts @@ -32,20 +32,6 @@ import { hasActiveSessionFailedCIChecks } from '../../../changes/browser/checksA import { ISessionsPartService } from '../../../../services/sessions/browser/sessionsPartService.js'; import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; -// Constants - -const ACTION_ID_NEW_SESSION = 'workbench.action.chat.newChat'; -// Keybindings - -KeybindingsRegistry.registerKeybindingRule({ - id: ACTION_ID_NEW_SESSION, - weight: KeybindingWeight.SessionsContrib, - // Don't shadow Ctrl/Cmd+N when focus is in the editor area so the standard - // `workbench.action.files.newUntitledFile` command handles the shortcut. - when: EditorAreaFocusContext.negate(), - primary: KeyMod.CtrlCmd | KeyCode.KeyN, -}); - const CLOSE_SESSION_COMMAND_ID = 'sessionsViewPane.closeSession'; registerAction2(class CloseSessionAction extends Action2 { constructor() { From 7a241e8f8cbe63cb31a991079630fb19af8241bf Mon Sep 17 00:00:00 2001 From: Martin Aeschlimann Date: Fri, 19 Jun 2026 11:21:20 +0100 Subject: [PATCH 8/8] Agent host customizations are not restored (#322069) --- .../agentHost/node/agentHostStateManager.ts | 16 ++++++++++- .../platform/agentHost/node/agentService.ts | 22 ++++++++++++--- .../agentHost/test/node/agentService.test.ts | 28 +++++++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentHostStateManager.ts b/src/vs/platform/agentHost/node/agentHostStateManager.ts index 1ad4e57113745..105e6b96df247 100644 --- a/src/vs/platform/agentHost/node/agentHostStateManager.ts +++ b/src/vs/platform/agentHost/node/agentHostStateManager.ts @@ -12,7 +12,7 @@ import { TelemetryLevel } from '../../telemetry/common/telemetry.js'; import { ActionType, ActionEnvelope, ActionOrigin, INotification, IRootConfigChangedAction, SessionAction, ChatAction, RootAction, StateAction, TerminalAction, ChangesetAction, AnnotationsAction, ClientAnnotationsAction, isRootAction, isSessionAction, isChatAction, isChangesetAction, isAnnotationsAction } from '../common/state/sessionActions.js'; import type { IStateSnapshot } from '../common/state/sessionProtocol.js'; import { rootReducer, sessionReducer, chatReducer, changesetReducer, annotationsReducer } from '../common/state/sessionReducers.js'; -import { createRootState, createSessionState, createChatState, createDefaultChatSummary, chatSummaryFromState, buildDefaultChatUri, parseDefaultChatUri, isAhpChatChannel, isDefaultChatUri, mergeSessionWithDefaultChat, isAhpRootChannel, SessionLifecycle, withHostBuildInfo, type Changeset, type ChangesetState, type AnnotationsState, type ChatState, type ChatSummary, type ISessionWithDefaultChat, type RootState, type SessionConfigState, type SessionMeta, type SessionState, type SessionSummary, type Turn, type URI, ROOT_STATE_URI, ChangesetStatus, IHostBuildInfo, SessionStatus } from '../common/state/sessionState.js'; +import { createRootState, createSessionState, createChatState, createDefaultChatSummary, chatSummaryFromState, buildDefaultChatUri, parseDefaultChatUri, isAhpChatChannel, isDefaultChatUri, mergeSessionWithDefaultChat, isAhpRootChannel, SessionLifecycle, withHostBuildInfo, type Changeset, type ChangesetState, type AnnotationsState, type ChatState, type ChatSummary, type Customization, type ISessionWithDefaultChat, type RootState, type SessionConfigState, type SessionMeta, type SessionState, type SessionSummary, type Turn, type URI, ROOT_STATE_URI, ChangesetStatus, IHostBuildInfo, SessionStatus } from '../common/state/sessionState.js'; import { AgentHostTelemetryLevelConfigKey, IPermissionsValue, platformRootSchema, telemetryLevelToAgentHostConfigValue } from '../common/agentHostSchema.js'; import { SessionConfigKey } from '../common/sessionConfigKeys.js'; import { parseChangesetUri } from '../common/changesetUri.js'; @@ -666,6 +666,20 @@ export class AgentHostStateManager extends Disposable { state.config = config; } + /** + * Seeds or replaces the session's effective customizations directly on the + * authoritative in-memory state. Used by create/restore flows to ensure the + * first snapshot already contains customizations. + */ + setSessionCustomizations(session: URI, customizations: readonly Customization[] | undefined): void { + const state = this._sessionStates.get(session); + if (!state) { + this._logService.warn(`[AgentHostStateManager] setSessionCustomizations: unknown session ${session}`); + return; + } + state.customizations = customizations ? [...customizations] : undefined; + } + // ---- Changeset registry ------------------------------------------------- /** diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index ff235e574dbba..e6065a8e801aa 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -1553,13 +1553,27 @@ export class AgentService extends Disposable implements IAgentService { // sessions that were not created in the current process lifetime. // Overlay any values the user previously selected (persisted via // `SessionConfigChanged`) on top of the provider's resolved defaults. - const restoredConfig = await this._resolveCreatedSessionConfig(agent, { - workingDirectory: meta.workingDirectory, - config: persistedConfigValues, - }); + const [restoredConfig, restoredCustomizations] = await Promise.all([ + this._resolveCreatedSessionConfig(agent, { + workingDirectory: meta.workingDirectory, + config: persistedConfigValues, + }), + agent.getSessionCustomizations + ? agent.getSessionCustomizations(session).catch(err => { + this._logService.error('[AgentService] restoreSession: failed to resolve session customizations', err); + return undefined; + }) + : Promise.resolve(undefined), + ]); if (restoredConfig) { this._stateManager.setSessionConfig(sessionStr, restoredConfig); } + // Seed restored session customizations into state so the very first + // snapshot after selecting an existing session contains effective + // instructions/agents without waiting for a follow-up republish. + if (restoredCustomizations && restoredCustomizations.length > 0) { + this._stateManager.setSessionCustomizations(sessionStr, restoredCustomizations); + } this._logService.info(`[AgentService] Restored session ${sessionStr} with ${turns.length} turns`); diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 64d6ed82bb92a..e5f34def2d9be 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -1918,6 +1918,34 @@ suite('AgentService (node dispatcher)', () => { }); }); + test('hydrates session customizations when restoring an existing session', async () => { + service.registerProvider(copilotAgent); + const { session } = await copilotAgent.createSession(); + service.stateManager.deleteSession(session.toString()); + + copilotAgent.sessionMessages = [ + { type: 'message', session, role: 'user', messageId: 'msg-1', content: 'Hello', toolRequests: [] }, + { type: 'message', session, role: 'assistant', messageId: 'msg-2', content: 'Hi', toolRequests: [] }, + ]; + let getSessionCustomizationsCalls = 0; + copilotAgent.getSessionCustomizations = async () => { + getSessionCustomizationsCalls++; + return [ + { type: CustomizationType.Plugin, id: customizationId('file:///restore-skill'), uri: 'file:///restore-skill', name: 'Restore Skill', enabled: true }, + ]; + }; + + await service.restoreSession(session); + + const customizations = service.stateManager.getSessionState(session.toString())?.customizations; + assert.strictEqual(getSessionCustomizationsCalls, 1); + assert.strictEqual(customizations?.length, 1); + assert.strictEqual(customizations?.[0]?.type, CustomizationType.Plugin); + assert.strictEqual(customizations?.[0]?.name, 'Restore Skill'); + assert.strictEqual(customizations?.[0]?.id, customizationId('file:///restore-skill')); + assert.strictEqual(customizations?.[0]?.enabled, true); + }); + test('clears failed restore attempts so sessions can be retried', async () => { class FailingOnceRestoreAgent extends MockAgent { shouldFailRestore = true;