From b302fdfe33932085e3267e2651fa2629ba93ef45 Mon Sep 17 00:00:00 2001 From: Osvaldo Ortega Date: Thu, 10 Sep 2026 17:05:22 -0700 Subject: [PATCH 1/4] Agent Host changes for agents/diagnostic-logging-session-permissions --- .../browser/agentHostModePicker.fixture.ts | 15 +- .../agentHost/agentHostChatInputPicker.ts | 173 +++++--- .../agentHost/agentHostGenericConfigChips.ts | 50 ++- .../agentHost/agentHostSessionUri.ts | 9 + ...ntHostUntitledProvisionalSessionService.ts | 44 +- .../agentHostChatInputPicker.test.ts | 31 +- .../agentHostGenericConfigChips.test.ts | 379 +++++++++++++++++- ...tUntitledProvisionalSessionService.test.ts | 120 +++++- 8 files changed, 716 insertions(+), 105 deletions(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostModePicker.fixture.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostModePicker.fixture.ts index b186982daac033..597c592807ebd7 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostModePicker.fixture.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostModePicker.fixture.ts @@ -12,6 +12,7 @@ import { mock } from '../../../../../../base/test/common/mock.js'; import { ActionWidgetService, IActionWidgetService } from '../../../../../../platform/actionWidget/browser/actionWidget.js'; import { IAgentHostEnablementService } from '../../../../../../platform/agentHost/common/agentHostEnablementService.js'; import { getAgentHostCopilotSandboxSettingId, IAgentHostService } from '../../../../../../platform/agentHost/common/agentService.js'; +import { AMBIENT_AGENT_HOST_AUTHORITY, IAgentHostConnectionsService } from '../../../../../../platform/agentHost/common/agentHostConnectionsService.js'; import { IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; import { ComponentToState, StateComponents } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { ActionType } from '../../../../../../platform/agentHost/common/state/protocol/actions.js'; @@ -23,6 +24,7 @@ import { IContextViewService } from '../../../../../../platform/contextview/brow import { ContextViewService } from '../../../../../../platform/contextview/browser/contextViewService.js'; import { ILayoutService } from '../../../../../../platform/layout/browser/layoutService.js'; import { AgentHostChatInputPicker } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.js'; +import { toAgentHostBackendSessionUri } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionUri.js'; import { IAgentHostSessionWorkingDirectoryResolver } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionWorkingDirectoryResolver.js'; import { IAgentHostNewSessionFolderService } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostNewSessionFolderService.js'; import { IAgentHostUntitledProvisionalSessionService } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.js'; @@ -146,12 +148,13 @@ async function render(context: ComponentFixtureContext, mode: string, permission if (editor) { const state = new class extends mock() { override readonly config = config; + override readonly provider = 'copilotcli'; }(); const changed = disposableStore.add(new Emitter()); const subscriptions: { [K in StateComponents]?: IAgentSubscription } = { [StateComponents.Session]: { value: state, verifiedValue: state, onDidChange: changed.event, onWillApplyAction: Event.None, onDidApplyAction: Event.None }, }; - instantiationService.set(IAgentHostService, new class extends mock() { + const connection = new class extends mock() { override getSubscription(kind: T): IReference> { const subscription = subscriptions[kind]; if (!subscription) { @@ -165,7 +168,15 @@ async function render(context: ComponentFixtureContext, mode: string, permission changed.fire(state); } } - }()); + }(); + instantiationService.stub(IAgentHostConnectionsService, { + ambientConnection: connection, + onDidChangeSessionResolution: Event.None, + resolveSessionResource: sessionResource => { + const backendSession = toAgentHostBackendSessionUri(sessionResource); + return backendSession ? { connection, backendSession, connectionAuthority: AMBIENT_AGENT_HOST_AUTHORITY } : undefined; + }, + }); instantiationService.stub(IAgentHostSessionWorkingDirectoryResolver, { resolve: () => undefined }); instantiationService.stub(IAgentHostNewSessionFolderService, { getFolder: () => undefined, getDefaultFolder: () => undefined }); instantiationService.stub(IAgentHostUntitledProvisionalSessionService, { onDidChange: Event.None, get: () => undefined, getResolvedConfig: () => config, refreshResolvedConfig: async () => { } }); 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 dc2976282683bf..3d4b439b9e30c8 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts @@ -11,7 +11,7 @@ import { BaseActionViewItem } from '../../../../../../base/browser/ui/actionbar/ import { Delayer } from '../../../../../../base/common/async.js'; import { CancellationTokenSource } from '../../../../../../base/common/cancellation.js'; import { Codicon } from '../../../../../../base/common/codicons.js'; -import { onUnexpectedError } from '../../../../../../base/common/errors.js'; +import { CancellationError, onUnexpectedError } from '../../../../../../base/common/errors.js'; import { Disposable, DisposableStore, IDisposable, MutableDisposable, toDisposable } from '../../../../../../base/common/lifecycle.js'; import { autorun, observableSignal } from '../../../../../../base/common/observable.js'; import { isEqual } from '../../../../../../base/common/resources.js'; @@ -24,7 +24,8 @@ import { IActionWidgetService } from '../../../../../../platform/actionWidget/br import { getCodexApprovalsPickerListOptions } from '../../../../../../platform/agentHost/browser/codexApprovalsPicker.js'; import { createAgentHostSandboxToggle, equalsAgentHostSandboxTogglePresentation, getAgentHostSandboxToggleState } from '../../../../../../platform/agentHost/browser/agentHostSandboxToggle.js'; import { IAgentHostEnablementService } from '../../../../../../platform/agentHost/common/agentHostEnablementService.js'; -import { AgentHostCopilotSandboxSettingId, getAgentHostCopilotSandboxSettingId, IAgentHostService } from '../../../../../../platform/agentHost/common/agentService.js'; +import { AgentHostCopilotSandboxSettingId, getAgentHostCopilotSandboxSettingId, IAgentConnection } from '../../../../../../platform/agentHost/common/agentService.js'; +import { IAgentHostConnectionsService } from '../../../../../../platform/agentHost/common/agentHostConnectionsService.js'; import { AgentHostCustomTerminalToolEnabledSettingId } from '../../../../../../platform/agentHost/common/copilotCliConfig.js'; import { SessionConfigKey } from '../../../../../../platform/agentHost/common/sessionConfigKeys.js'; import { ClaudeSessionConfigKey } from '../../../../../../platform/agentHost/common/claudeSessionConfigKeys.js'; @@ -52,7 +53,7 @@ import { withChatInputPickerMotion } from '../../widget/input/chatInputPickerAct import { IAgentHostSessionWorkingDirectoryResolver } from './agentHostSessionWorkingDirectoryResolver.js'; import { IAgentHostNewSessionFolderService } from './agentHostNewSessionFolderService.js'; import { IAgentHostUntitledProvisionalSessionService } from './agentHostUntitledProvisionalSessionService.js'; -import { toAgentHostBackendSessionUri } from './agentHostSessionUri.js'; +import { resolveAgentHostChatSession, toAgentHostBackendSessionUri } from './agentHostSessionUri.js'; import { getCompactCodicon } from '../../chatIcons.js'; import { IChatPhoneInputPresenter } from '../../widget/input/chatPhoneInputPresenter.js'; import { AGENT_HOST_PERMISSIONS_SETTINGS_QUERY, createModePickerModeItems, createModePickerPermissionsItems, getModePermissionsPickerAccessibilityProvider, getModePermissionsPickerOptions, getModePickerAriaLabel, IModePickerPermissions, IModePickerTrigger, isWellKnownAutoApproveSchema, renderModePickerTrigger, shouldCombineModeAndPermissions } from './agentHostModePickerPresentation.js'; @@ -73,6 +74,19 @@ interface IConfigPickerItem { readonly checked?: boolean; } +interface IConfigPickerTarget { + readonly sessionResource: URI; + readonly backendSession: URI; + readonly connection: IAgentConnection; + readonly generation: number; +} + +interface IConfigPickerContext extends IConfigPickerTarget { + readonly provider: string; + readonly schema: SessionConfigPropertySchema; + readonly value: unknown; +} + function getConfigIcon(property: string, value: unknown | undefined): ThemeIcon | undefined { if (property === SessionConfigKey.Mode) { switch (value) { @@ -324,6 +338,12 @@ export function isClaimedByDedicatedPicker(property: string, schema: SessionConf return WELL_KNOWN_PICKER_PROPERTIES.has(property); } +export function isGenericConfigPickerProperty(property: string, schema: SessionConfigPropertySchema, isStartedSession: boolean): boolean { + return !isClaimedByDedicatedPicker(property, schema) + && (!isStartedSession || schema.sessionMutable === true) + && (schema.type === 'boolean' || (schema.type === 'string' && (!!schema.enumDynamic || !!schema.enum?.length))); +} + /** * Resolves which config value a chat-input chip should display, given the * server's session-state value and the workbench overlay value. @@ -362,12 +382,13 @@ export class AgentHostChatInputPicker extends Disposable { private readonly _pickerDisposables = this._register(new DisposableStore()); private readonly _sandboxConfigChanged = observableSignal(this); private readonly _filterDelayer = this._register(new Delayer[]>(200)); - private readonly _subRef = this._register(new MutableDisposable; readonly backendSession: URI }>()); + private readonly _subRef = this._register(new MutableDisposable }>()); + private _sessionGeneration = 0; constructor( private readonly _widget: IChatWidget, private readonly _property: string, - @IAgentHostService private readonly _agentHostService: IAgentHostService, + @IAgentHostConnectionsService private readonly _connectionsService: IAgentHostConnectionsService, @IActionWidgetService private readonly _actionWidgetService: IActionWidgetService, @IHoverService private readonly _hoverService: IHoverService, @IOpenerService private readonly _openerService: IOpenerService, @@ -387,6 +408,13 @@ export class AgentHostChatInputPicker extends Disposable { this._register(this._widget.onDidChangeViewModel(() => { this._reattach(); })); + this._register(this._connectionsService.onDidChangeSessionResolution(() => { + if (!this._subRef.value || !this._isCurrentTarget(this._subRef.value)) { + this._initialResolved = undefined; + this._cancelInitialResolve(); + this._reattach(); + } + })); this._register(this._provisional.onDidChange((sessionResource: URI) => { const current = this._widget.viewModel?.sessionResource; if (current && current.toString() === sessionResource.toString()) { @@ -446,17 +474,17 @@ export class AgentHostChatInputPicker extends Disposable { } show(anchor: HTMLElement): void { - void this._showPicker(anchor); + void this._showPicker(anchor).catch(onUnexpectedError); } private _reattach(): void { this._hidePicker(); + this._sessionGeneration++; const sessionResource = this._widget.viewModel?.sessionResource; const provisionalBackend = sessionResource ? this._provisional.get(sessionResource) : undefined; - const backendSession = provisionalBackend - ?? (sessionResource ? toAgentHostBackendSessionUri(sessionResource) : undefined); + const resolution = sessionResource ? resolveAgentHostChatSession(sessionResource, provisionalBackend, this._connectionsService) : undefined; - if (!sessionResource || !backendSession) { + if (!sessionResource || !resolution) { this._subRef.clear(); this._initialResolved = undefined; this._cancelInitialResolve(); @@ -464,11 +492,12 @@ export class AgentHostChatInputPicker extends Disposable { return; } - if (isUntitledChatSession(sessionResource) && !provisionalBackend) { + const localBackend = toAgentHostBackendSessionUri(sessionResource); + if (localBackend && isUntitledChatSession(sessionResource) && !provisionalBackend) { this._subRef.clear(); if (!this._initialResolved || this._initialResolved.sessionResource.toString() !== sessionResource.toString()) { this._initialResolved = undefined; - void this._refreshInitialResolved(sessionResource, backendSession); + void this._refreshInitialResolved(sessionResource, localBackend); } // Eagerly create a provisional backend session so even users // who never touch a chip still get their picker defaults @@ -483,7 +512,7 @@ export class AgentHostChatInputPicker extends Disposable { // `onDidChange` and we re-attach into the subscription path. void this._provisional.getOrCreate( sessionResource, - backendSession.scheme, + localBackend.scheme, this._readWorkingDirectory(), ); this._renderChip(); @@ -492,15 +521,17 @@ export class AgentHostChatInputPicker extends Disposable { this._initialResolved = undefined; this._cancelInitialResolve(); - const ref = this._agentHostService.getSubscription(StateComponents.Session, backendSession, 'AgentHostChatInputPicker'); + const ref = resolution.connection.getSubscription(StateComponents.Session, resolution.backendSession, 'AgentHostChatInputPicker'); const sub = ref.object; const listener = sub.onDidChange(() => { this._renderChip(); this._sandboxConfigChanged.trigger(undefined); }); this._subRef.value = { + ...resolution, sub, - backendSession, + sessionResource, + generation: this._sessionGeneration, dispose: () => { listener.dispose(); ref.dispose(); }, }; this._renderChip(); @@ -520,7 +551,7 @@ export class AgentHostChatInputPicker extends Disposable { const cts = new CancellationTokenSource(); this._initialResolveCts.value = cts; try { - const result = await this._agentHostService.resolveSessionConfig({ + const result = await this._connectionsService.ambientConnection.resolveSessionConfig({ provider: backendSession.scheme, workingDirectory: this._readWorkingDirectory(), }); @@ -583,7 +614,7 @@ export class AgentHostChatInputPicker extends Disposable { this._renderDisposables.add({ dispose: () => slot.remove() }); const isReadOnly = !!ctx.schema.readOnly || (isStartedSession && ctx.schema.sessionMutable === false); - const trigger = renderPickerTrigger(slot, isReadOnly, this._renderDisposables, () => this._showPicker(trigger), !!this._getModePickerPermissions()); + const trigger = renderPickerTrigger(slot, isReadOnly, this._renderDisposables, () => this._showPicker(trigger).catch(onUnexpectedError), !!this._getModePickerPermissions()); this._trigger = trigger; this._renderDisposables.add(this._hoverService.setupDelayedHover(trigger, () => ({ content: this._getModePickerPermissions() @@ -604,7 +635,7 @@ export class AgentHostChatInputPicker extends Disposable { if (permissions) { trigger.ariaLabel = getModePickerAriaLabel(label, permissions); this._splitTrigger.value = renderModePickerTrigger(trigger, { label, icon, labelClassName: 'agent-host-chat-input-picker-label' }, permissions, (anchor, openPermissions) => { - void this._showPicker(anchor, openPermissions); + void this._showPicker(anchor, openPermissions).catch(onUnexpectedError); }, previous); return; } @@ -676,7 +707,22 @@ export class AgentHostChatInputPicker extends Disposable { } : undefined; } - private _readContext(property = this._property): { backendSession: URI; schema: SessionConfigPropertySchema; value: unknown | undefined } | undefined { + private _isCurrentTarget(target: IConfigPickerTarget): boolean { + if (this._store.isDisposed || target.generation !== this._sessionGeneration || !isEqual(target.sessionResource, this._widget.viewModel?.sessionResource)) { + return false; + } + const resolution = resolveAgentHostChatSession(target.sessionResource, this._provisional.get(target.sessionResource), this._connectionsService); + return resolution?.connection === target.connection && isEqual(resolution.backendSession, target.backendSession); + } + + private _canEdit(context: IConfigPickerContext, property: string): boolean { + const isLocalDraft = isUntitledChatSession(context.sessionResource) && !!toAgentHostBackendSessionUri(context.sessionResource); + return !context.schema.readOnly && (isLocalDraft + || context.schema.sessionMutable === true + || (isClaimedByDedicatedPicker(property, context.schema) && context.schema.sessionMutable !== false)); + } + + private _readContext(property = this._property): IConfigPickerContext | undefined { const sessionResource = this._widget.viewModel?.sessionResource; if (!sessionResource) { return undefined; @@ -701,7 +747,7 @@ export class AgentHostChatInputPicker extends Disposable { const serverValue = state.config?.values?.[property]; const overlayValue = overlay?.values?.[property]; const value = resolveConfigChipValue(isUntitledChatSession(sessionResource), serverValue, overlayValue, schema.default); - return { backendSession: this._subRef.value.backendSession, schema, value }; + return { ...this._subRef.value, provider: state.provider, schema, value }; } if (this._initialResolved && this._initialResolved.sessionResource.toString() === sessionResource.toString()) { @@ -714,7 +760,7 @@ export class AgentHostChatInputPicker extends Disposable { return undefined; } const value = this._initialResolved.result.values?.[property] ?? schema.default; - return { backendSession, schema, value }; + return { sessionResource, backendSession, connection: this._connectionsService.ambientConnection, generation: this._sessionGeneration, provider: backendSession.scheme, schema, value }; } return undefined; @@ -752,11 +798,10 @@ export class AgentHostChatInputPicker extends Disposable { return; } const ctx = this._readContext(); - if (!ctx || ctx.schema.readOnly) { + if (!ctx || !this._canEdit(ctx, this._property)) { return; } - const sessionResource = this._widget.viewModel?.sessionResource; const anchor = trigger === this._trigger ? this._splitTrigger.value?.modeButton ?? trigger : trigger; const modeItems = await this._getActionItems(this._property, ctx.schema, ctx.value); if (modeItems.length === 0) { @@ -777,7 +822,7 @@ export class AgentHostChatInputPicker extends Disposable { label: value.label, checked: value.value === LEARN_MORE_VALUE ? undefined : value.checked, enabled: !item.disabled, - run: () => this._selectItem(SessionConfigKey.AutoApprove, permissionContext.backendSession, value), + run: () => this._selectItem(SessionConfigKey.AutoApprove, permissionContext, value), }) : undefined, }; }); @@ -786,16 +831,17 @@ export class AgentHostChatInputPicker extends Disposable { await this._preferencesService.openSettings({ jsonEditor: false, query: AGENT_HOST_PERMISSIONS_SETTINGS_QUERY }); })); } - if (this._store.isDisposed || this._actionWidgetService.isVisible || !isEqual(sessionResource, this._widget.viewModel?.sessionResource)) { + const currentContext = this._readContext(); + if (this._actionWidgetService.isVisible || !this._isCurrentTarget(ctx) || !currentContext || !this._canEdit(currentContext, this._property)) { return; } const delegate: IActionListDelegate = { - onSelect: item => hasKey(item, { run: true }) ? item.run() : this._selectItem(this._property, ctx.backendSession, item), + onSelect: item => hasKey(item, { run: true }) ? item.run() : this._selectItem(this._property, ctx, item), onFilter: ctx.schema.enumDynamic ? query => this._filterDelayer.trigger(async () => { const refreshed = this._readContext(); - if (!refreshed) { + if (!refreshed || !this._isCurrentTarget(ctx)) { return []; } return toActionItems(this._property, await this._getItems(refreshed.schema, query), refreshed.value, isAutoApprovePolicyRestricted(this._configurationService), this._getSandboxStandaloneToggle()); @@ -858,7 +904,7 @@ export class AgentHostChatInputPicker extends Disposable { } } - private async _selectItem(property: string, backendSession: URI, item: IConfigPickerItem): Promise { + private async _selectItem(property: string, context: IConfigPickerContext, item: IConfigPickerItem): Promise { this._actionWidgetService.hide(); if (item.value === LEARN_MORE_VALUE) { const url = getPermissionsLearnMoreUrl(property); @@ -866,7 +912,7 @@ export class AgentHostChatInputPicker extends Disposable { await this._openerService.open(URI.parse(url)); } } else { - await this._confirmAndSetValue(backendSession, item, property); + await this._confirmAndSetValue(context, item, property); } } @@ -905,10 +951,9 @@ export class AgentHostChatInputPicker extends Disposable { if (property !== SessionConfigKey.AutoApprove || !this._isSandboxToggleSettingEnabled() || !settingId || !context || !sessionResource) { return undefined; } - const workingDirectory = this._readWorkingDirectory(); return createAgentHostSandboxToggle(() => this._readSandboxToggleState(), checked => { const target = checked ? AgentSandboxEnabledValue.On : AgentSandboxEnabledValue.Off; - void this._setValue(context.backendSession, target, SessionConfigKey.SandboxEnabled, sessionResource, workingDirectory).catch(onUnexpectedError); + void this._setValue(context, target, SessionConfigKey.SandboxEnabled).catch(onUnexpectedError); }); } @@ -919,22 +964,22 @@ export class AgentHostChatInputPicker extends Disposable { { value: 'false', label: localize('agentHostChatInputPicker.boolean.false', "Off") }, ]; } - const sessionResource = this._widget.viewModel?.sessionResource; - const backendSession = this._subRef.value?.backendSession - ?? (sessionResource ? toAgentHostBackendSessionUri(sessionResource) : undefined); - if (schema.enumDynamic && backendSession) { - try { - const result = await this._agentHostService.sessionConfigCompletions({ - provider: backendSession.scheme, - property, - query, - workingDirectory: this._readWorkingDirectory(), - config: this._readCurrentValues(), - }); - return this._filterAutoApproveItems(result.items.map(item => this._fromCompletion(item)), property); - } catch { - // Fall through to the static enum below. + if (schema.enumDynamic) { + const context = this._readContext(property); + if (!context || !this._isCurrentTarget(context)) { + throw new CancellationError(); + } + const result = await context.connection.sessionConfigCompletions({ + provider: context.provider, + property, + query, + workingDirectory: this._readWorkingDirectory(), + config: this._readCurrentValues(), + }); + if (!this._isCurrentTarget(context)) { + throw new CancellationError(); } + return this._filterAutoApproveItems(result.items.map(item => this._fromCompletion(item)), property); } return this._filterAutoApproveItems((schema.enum ?? []).map((value, index) => ({ value: String(value), @@ -973,7 +1018,9 @@ export class AgentHostChatInputPicker extends Disposable { const overlay = sessionResource ? this._provisional.getResolvedConfig(sessionResource) : undefined; const state = this._subRef.value?.sub.value; if (state && !(state instanceof Error)) { - return { ...(state.config?.values ?? {}), ...(overlay?.values ?? {}) }; + return sessionResource && isUntitledChatSession(sessionResource) + ? { ...(state.config?.values ?? {}), ...(overlay?.values ?? {}) } + : { ...(overlay?.values ?? {}), ...(state.config?.values ?? {}) }; } return overlay?.values ?? this._initialResolved?.result.values; } @@ -982,8 +1029,7 @@ export class AgentHostChatInputPicker extends Disposable { * Surfaces the shared elevated-level warning before applying an approval * pick. Unknown non-default values fall back to the Bypass warning. */ - private async _confirmAndSetValue(backendSession: URI, item: IConfigPickerItem, property = this._property): Promise { - const sessionResource = this._widget.viewModel?.sessionResource; + private async _confirmAndSetValue(context: IConfigPickerContext, item: IConfigPickerItem, property = this._property): Promise { const value = item.value; if (property === SessionConfigKey.AutoApprove && !isPermissionLevelVisible(value, isAssistedPermissionsEnabled(this._configurationService))) { return; @@ -1002,36 +1048,31 @@ export class AgentHostChatInputPicker extends Disposable { } } } - if (isEqual(sessionResource, this._widget.viewModel?.sessionResource)) { - await this._setValue(backendSession, value, property); - } + await this._setValue(context, value, property); } private async _setValue( - backendSession: URI, + context: IConfigPickerContext, value: string, property = this._property, - sessionResource = this._widget.viewModel?.sessionResource, - workingDirectory = this._readWorkingDirectory(), ): Promise { - if (!sessionResource) { - return; + const ctx = this._readContext(property); + if (!ctx || !this._isCurrentTarget(context) || !this._canEdit(ctx, property)) { + throw new CancellationError(); } - - const isCurrentSession = isEqual(sessionResource, this._widget.viewModel?.sessionResource); - const ctx = isCurrentSession ? this._readContext(property) : undefined; - const normalizedValue = ctx?.schema.type === 'boolean' + const { sessionResource, backendSession, connection, provider } = ctx; + const workingDirectory = this._readWorkingDirectory(); + const normalizedValue = ctx.schema.type === 'boolean' ? value === 'true' : normalizeSessionConfigValue(property, value, isAutoApprovePolicyRestricted(this._configurationService)); const partial = { [property]: normalizedValue }; - const nextConfig = { ...(isCurrentSession ? this._readCurrentValues() : undefined), ...partial }; + const nextConfig = { ...this._readCurrentValues(), ...partial }; - if (isUntitledChatSession(sessionResource)) { + if (isUntitledChatSession(sessionResource) && toAgentHostBackendSessionUri(sessionResource)) { // Route through the provisional service so the workbench-owned // config cache is updated synchronously. `tryRebind` reads from // that cache, so a Send racing with this dispatch picks up the // new value without waiting for the agent to echo it back. - const provider = backendSession.scheme; const created = await this._provisional.applyConfigChange( sessionResource, provider, @@ -1047,14 +1088,14 @@ export class AgentHostChatInputPicker extends Disposable { return; } - this._agentHostService.dispatch(backendSession.toString(), { + connection.dispatch(backendSession.toString(), { type: ActionType.SessionConfigChanged, config: partial, }); - if (isCurrentSession) { + if (this._isCurrentTarget(context)) { await this._provisional.refreshResolvedConfig( sessionResource, - backendSession.scheme, + provider, workingDirectory, nextConfig, ); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostGenericConfigChips.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostGenericConfigChips.ts index 89c890813d6940..f3458ecd70faf3 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostGenericConfigChips.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostGenericConfigChips.ts @@ -6,21 +6,22 @@ import * as dom from '../../../../../../base/browser/dom.js'; import { CancellationTokenSource } from '../../../../../../base/common/cancellation.js'; import { Disposable, DisposableMap, IDisposable, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; +import { isEqual } from '../../../../../../base/common/resources.js'; import { URI } from '../../../../../../base/common/uri.js'; import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js'; -import { IAgentHostService } from '../../../../../../platform/agentHost/common/agentService.js'; +import { IAgentHostConnectionsService, IAgentHostSessionResolution } from '../../../../../../platform/agentHost/common/agentHostConnectionsService.js'; import type { ResolveSessionConfigResult, SessionConfigPropertySchema } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; import type { SessionState } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; import { StateComponents } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { type IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; import { isUntitledChatSession } from '../../../common/model/chatUri.js'; import type { IChatWidget } from '../../chat.js'; -import { AgentHostChatInputPicker, isClaimedByDedicatedPicker } from './agentHostChatInputPicker.js'; +import { AgentHostChatInputPicker, isGenericConfigPickerProperty } from './agentHostChatInputPicker.js'; import { IAgentHostSessionWorkingDirectoryResolver } from './agentHostSessionWorkingDirectoryResolver.js'; import { IAgentHostNewSessionFolderService } from './agentHostNewSessionFolderService.js'; import { IAgentHostUntitledProvisionalSessionService } from './agentHostUntitledProvisionalSessionService.js'; import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js'; -import { toAgentHostBackendSessionUri } from './agentHostSessionUri.js'; +import { resolveAgentHostChatSession, toAgentHostBackendSessionUri } from './agentHostSessionUri.js'; /** * Direct-render chip lane for agent-host session-config properties that are @@ -39,14 +40,10 @@ export class AgentHostGenericConfigChips extends Disposable { private readonly _chips = this._register(new DisposableMap()); private readonly _chipElements = new Map(); - /** - * Subscription to the active session's backend state. Maintained for the - * lifetime of any one (sessionResource, backendSession) pair; replaced - * via {@link _reattach} when the active session changes. - */ - private readonly _subRef = this._register(new MutableDisposable; - readonly backendSession: URI; + readonly sessionResource: URI; }>()); private _initialResolved: { readonly sessionResource: URI; readonly result: ResolveSessionConfigResult } | undefined; @@ -55,7 +52,7 @@ export class AgentHostGenericConfigChips extends Disposable { constructor( private readonly _widget: IChatWidget, @IInstantiationService private readonly _instantiationService: IInstantiationService, - @IAgentHostService private readonly _agentHostService: IAgentHostService, + @IAgentHostConnectionsService private readonly _connectionsService: IAgentHostConnectionsService, @IAgentHostUntitledProvisionalSessionService private readonly _provisional: IAgentHostUntitledProvisionalSessionService, @IAgentHostSessionWorkingDirectoryResolver private readonly _workingDirectoryResolver: IAgentHostSessionWorkingDirectoryResolver, @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, @@ -63,6 +60,7 @@ export class AgentHostGenericConfigChips extends Disposable { ) { super(); this._register(this._widget.onDidChangeViewModel(() => this._reattach())); + this._register(this._connectionsService.onDidChangeSessionResolution(() => this._reattach())); this._register(this._provisional.onDidChange((sessionResource: URI) => { const current = this._widget.viewModel?.sessionResource; if (current && current.toString() === sessionResource.toString()) { @@ -84,10 +82,9 @@ export class AgentHostGenericConfigChips extends Disposable { private _reattach(): void { const sessionResource = this._widget.viewModel?.sessionResource; const provisionalBackend = sessionResource ? this._provisional.get(sessionResource) : undefined; - const backendSession = provisionalBackend - ?? (sessionResource ? toAgentHostBackendSessionUri(sessionResource) : undefined); + const resolution = sessionResource ? resolveAgentHostChatSession(sessionResource, provisionalBackend, this._connectionsService) : undefined; - if (!sessionResource || !backendSession) { + if (!sessionResource || !resolution) { this._subRef.clear(); this._initialResolved = undefined; this._cancelInitialResolve(); @@ -95,11 +92,12 @@ export class AgentHostGenericConfigChips extends Disposable { return; } - if (isUntitledChatSession(sessionResource) && !provisionalBackend) { + const localBackend = toAgentHostBackendSessionUri(sessionResource); + if (localBackend && isUntitledChatSession(sessionResource) && !provisionalBackend) { this._subRef.clear(); if (!this._initialResolved || this._initialResolved.sessionResource.toString() !== sessionResource.toString()) { this._initialResolved = undefined; - void this._refreshInitialResolved(sessionResource, backendSession); + void this._refreshInitialResolved(sessionResource, localBackend); } this._sync(); return; @@ -107,12 +105,18 @@ export class AgentHostGenericConfigChips extends Disposable { this._initialResolved = undefined; this._cancelInitialResolve(); - const ref = this._agentHostService.getSubscription(StateComponents.Session, backendSession, 'AgentHostGenericConfigChips'); + const current = this._subRef.value; + if (current && isEqual(current.sessionResource, sessionResource) && current.connection === resolution.connection && isEqual(current.backendSession, resolution.backendSession)) { + this._sync(); + return; + } + const ref = resolution.connection.getSubscription(StateComponents.Session, resolution.backendSession, 'AgentHostGenericConfigChips'); const sub = ref.object; const listener = sub.onDidChange(() => this._sync()); this._subRef.value = { + ...resolution, sub, - backendSession, + sessionResource, dispose: () => { listener.dispose(); ref.dispose(); }, }; this._sync(); @@ -128,7 +132,7 @@ export class AgentHostGenericConfigChips extends Disposable { const cts = new CancellationTokenSource(); this._initialResolveCts.value = cts; try { - const result = await this._agentHostService.resolveSessionConfig({ + const result = await this._connectionsService.ambientConnection.resolveSessionConfig({ provider: backendSession.scheme, workingDirectory: this._readWorkingDirectory(), }); @@ -176,10 +180,12 @@ export class AgentHostGenericConfigChips extends Disposable { return; } const entries = this._readSchemaProperties(); + const sessionResource = this._widget.viewModel?.sessionResource; + const isStartedSession = !!sessionResource && !(isUntitledChatSession(sessionResource) && toAgentHostBackendSessionUri(sessionResource)); const desired = new Set(); if (entries) { for (const [property, schema] of entries) { - if (isClaimedByDedicatedPicker(property, schema)) { + if (!isGenericConfigPickerProperty(property, schema, isStartedSession)) { continue; } desired.add(property); @@ -206,8 +212,6 @@ export class AgentHostGenericConfigChips extends Disposable { // chips' container — required so the secondary-toolbar styling // in `chat.css` (height, padding, chevron) applies here too. const slot = dom.append(this._container, dom.$('.agent-host-generic-chip-slot.chat-input-picker-item')); - chip.render(slot); - this._chipElements.set(property, slot); this._chips.set(property, { dispose: () => { chip.dispose(); @@ -215,6 +219,8 @@ export class AgentHostGenericConfigChips extends Disposable { this._chipElements.delete(property); }, }); + this._chipElements.set(property, slot); + chip.render(slot); } } } diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionUri.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionUri.ts index 25929afa998ccb..3dceb2dabdbdee 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionUri.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionUri.ts @@ -4,6 +4,15 @@ *--------------------------------------------------------------------------------------------*/ import { URI } from '../../../../../../base/common/uri.js'; +import { AMBIENT_AGENT_HOST_AUTHORITY, IAgentHostConnectionsService, IAgentHostSessionResolution } from '../../../../../../platform/agentHost/common/agentHostConnectionsService.js'; + +/** Preserves local provisional resources while resolving all other sessions through their owning host. */ +export function resolveAgentHostChatSession(sessionResource: URI, provisionalBackend: URI | undefined, connectionsService: IAgentHostConnectionsService): IAgentHostSessionResolution | undefined { + const resolution = connectionsService.resolveSessionResource(sessionResource); + return resolution && provisionalBackend && resolution.connectionAuthority === AMBIENT_AGENT_HOST_AUTHORITY + ? { ...resolution, backendSession: provisionalBackend } + : resolution; +} export function toAgentHostBackendSessionUri(sessionResource: URI): URI | undefined { const scheme = sessionResource.scheme; 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 9c7716bf8828f7..e12457e3bfcc1c 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.ts @@ -58,6 +58,7 @@ import { isEqual } from '../../../../../../base/common/resources.js'; import { URI } from '../../../../../../base/common/uri.js'; import { generateUuid } from '../../../../../../base/common/uuid.js'; import { IAgentHostService } from '../../../../../../platform/agentHost/common/agentService.js'; +import { IAgentHostConnectionsService, IAgentHostSessionResolution } from '../../../../../../platform/agentHost/common/agentHostConnectionsService.js'; import { 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'; @@ -187,8 +188,8 @@ export interface IAgentHostUntitledProvisionalSessionService { getResolvedConfig(sessionResource: URI): ResolveSessionConfigResult | undefined; /** - * Re-resolve config for an already-created chat session and cache the - * schema/values overlay returned by the provider. + * Re-resolve config on an already-created session's owning host and cache + * its schema/values overlay until the connection or session identity changes. */ refreshResolvedConfig( sessionResource: URI, @@ -268,6 +269,8 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple private readonly _pending = new ResourceMap>(); private readonly _resolvedConfigs = new ResourceMap(); private readonly _resolvedConfigRequestSeq = new ResourceMap(); + private readonly _resolvedConfigConnections = new ResourceMap(); + private _resolvedConfigRequestSequence = 0; private readonly _sessionCreationMetadata = new ResourceMap>(); private readonly _pendingBackendDisposals = new ResourceSet(); // URIs that were the source of a successful `tryRebind`. The chat widget @@ -281,6 +284,7 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple constructor( @IAgentHostService private readonly _agentHostService: IAgentHostService, + @IAgentHostConnectionsService private readonly _agentHostConnectionsService: IAgentHostConnectionsService, @ILogService private readonly _logService: ILogService, @IChatService chatService: IChatService, @IConfigurationService private readonly _configurationService: IConfigurationService, @@ -306,6 +310,7 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple } this._resolvedConfigs.delete(sessionResource); this._resolvedConfigRequestSeq.delete(sessionResource); + this._resolvedConfigConnections.delete(sessionResource); this._sessionCreationMetadata.delete(sessionResource); // Drop any tombstone for the abandoned untitled URI so the // set doesn't grow unbounded across the workbench lifetime. @@ -313,6 +318,24 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple } })); + this._register(this._agentHostConnectionsService.onDidChangeSessionResolution(() => { + for (const [sessionResource, resolution] of this._resolvedConfigConnections) { + if (this._isCurrentConfigConnection(sessionResource, resolution)) { + continue; + } + this._resolvedConfigConnections.delete(sessionResource); + this._resolvedConfigRequestSeq.delete(sessionResource); + const entry = this._entries.get(sessionResource); + const hadConfig = this._resolvedConfigs.delete(sessionResource) || entry?.resolvedConfig !== undefined; + if (entry) { + entry.resolvedConfig = undefined; + } + if (hadConfig) { + this._onDidChange.fire(sessionResource); + } + } + })); + // A session's working directory is fixed at creation time. When the user // picks a different folder for a not-yet-started session that already has // a provisional backend session (built up by config chips), recreate that @@ -927,6 +950,7 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple this._pendingBackendDisposals.clear(); this._resolvedConfigs.clear(); this._resolvedConfigRequestSeq.clear(); + this._resolvedConfigConnections.clear(); this._sessionCreationMetadata.clear(); this._rebound.clear(); super.dispose(); @@ -945,21 +969,31 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple return this._entries.get(sessionResource)?.resolvedConfig ?? this._resolvedConfigs.get(sessionResource); } + private _isCurrentConfigConnection(sessionResource: URI, resolution: IAgentHostSessionResolution): boolean { + const current = this._agentHostConnectionsService.resolveSessionResource(sessionResource); + return current !== undefined && current.connection === resolution.connection && isEqual(current.backendSession, resolution.backendSession); + } + async refreshResolvedConfig( sessionResource: URI, provider: string, workingDirectory: URI | undefined, config: Record | undefined, ): Promise { - const seq = (this._resolvedConfigRequestSeq.get(sessionResource) ?? 0) + 1; + const seq = ++this._resolvedConfigRequestSequence; this._resolvedConfigRequestSeq.set(sessionResource, seq); try { - const resolved = await this._agentHostService.resolveSessionConfig({ + const resolution = this._agentHostConnectionsService.resolveSessionResource(sessionResource); + if (!resolution) { + throw new Error('No connected agent host is available for session configuration'); + } + this._resolvedConfigConnections.set(sessionResource, resolution); + const resolved = await resolution.connection.resolveSessionConfig({ provider, workingDirectory, config, }); - if (this._resolvedConfigRequestSeq.get(sessionResource) !== seq) { + if (this._resolvedConfigRequestSeq.get(sessionResource) !== seq || !this._isCurrentConfigConnection(sessionResource, resolution)) { return; } const entry = this._entries.get(sessionResource); 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 index e8fdce0e173ece..9f8e6814b775cc 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts @@ -16,6 +16,8 @@ import { EventType as TouchEventType } from '../../../../../../base/browser/touc import { IAction } from '../../../../../../base/common/actions.js'; import { IAgentHostEnablementService } from '../../../../../../platform/agentHost/common/agentHostEnablementService.js'; import { IAgentHostService } from '../../../../../../platform/agentHost/common/agentService.js'; +import { AMBIENT_AGENT_HOST_AUTHORITY, IAgentHostConnectionsService } from '../../../../../../platform/agentHost/common/agentHostConnectionsService.js'; +import { toAgentHostBackendSessionUri } from '../../../browser/agentSessions/agentHost/agentHostSessionUri.js'; import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; import { ConfigurationTarget, IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; @@ -146,11 +148,19 @@ suite('AgentHostChatInputPicker - combined mode and permissions', () => { const settingsRequests: IOpenSettingsOptions[] = []; const hoverTargets: HTMLElement[] = []; const instantiationService = store.add(new TestInstantiationService()); - instantiationService.stub(IAgentHostService, { - dispatch: (_session, action) => { + const connection = new class extends mock() { + override dispatch(_session: string, action: Parameters[1]): void { if (action.type === ActionType.SessionConfigChanged) { dispatches.push(action); } + } + }(); + instantiationService.stub(IAgentHostConnectionsService, { + ambientConnection: connection, + onDidChangeSessionResolution: Event.None, + resolveSessionResource: sessionResource => { + const backendSession = toAgentHostBackendSessionUri(sessionResource); + return backendSession ? { connection, backendSession, connectionAuthority: AMBIENT_AGENT_HOST_AUTHORITY } : undefined; }, }); instantiationService.set(IActionWidgetService, actionWidget); @@ -174,7 +184,7 @@ suite('AgentHostChatInputPicker - combined mode and permissions', () => { instantiationService.stub(IAgentHostSessionWorkingDirectoryResolver, { resolve: () => undefined }); instantiationService.stub(IWorkspaceContextService, { getWorkspace: () => ({ id: 'test', folders: [] }) }); instantiationService.stub(IAgentHostNewSessionFolderService, { getFolder: () => undefined, getDefaultFolder: () => undefined }); - instantiationService.stub(IAgentHostUntitledProvisionalSessionService, { onDidChange: Event.None, getResolvedConfig: () => undefined, refreshResolvedConfig: async () => { } }); + instantiationService.stub(IAgentHostUntitledProvisionalSessionService, { onDidChange: Event.None, get: () => undefined, getResolvedConfig: () => undefined, refreshResolvedConfig: async () => { } }); instantiationService.stub(IAgentHostEnablementService, { managedSandboxEnforced: constObservable(false), managedSandboxAllowsBypass: constObservable(false) }); instantiationService.stub(IChatPhoneInputPresenter, { enabled: constObservable(false) }); const modePicker = store.add(instantiationService.createInstance(AgentHostChatInputPicker, widget, SessionConfigKey.Mode)); @@ -726,12 +736,20 @@ suite('AgentHostChatInputPicker - sandbox toggle', () => { override readonly onDidChangeViewModel = Event.None; override viewModel: IChatViewModel | undefined; }(); + const connection = new class extends mock() { + override dispatch(channel: string, action: Parameters[1]): void { + writes.push({ channel, action }); + } + }(); const picker = store.add(new AgentHostChatInputPicker( widget, SessionConfigKey.AutoApprove, - new class extends mock() { - override dispatch(channel: string, action: Parameters[1]): void { - writes.push({ channel, action }); + new class extends mock() { + override readonly ambientConnection = connection; + override readonly onDidChangeSessionResolution = Event.None; + override resolveSessionResource(sessionResource: URI) { + const backendSession = toAgentHostBackendSessionUri(sessionResource); + return backendSession ? { connection, backendSession, connectionAuthority: AMBIENT_AGENT_HOST_AUTHORITY } : undefined; } }(), actionWidgetService, @@ -741,6 +759,7 @@ suite('AgentHostChatInputPicker - sandbox toggle', () => { new class extends mock() { }(), new class extends mock() { override readonly onDidChange = Event.None; + override get() { return undefined; } override getResolvedConfig() { return undefined; } override async refreshResolvedConfig(): Promise { } }(), diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostGenericConfigChips.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostGenericConfigChips.test.ts index 2e96a1670d6df5..f2a4c9b9e8b102 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostGenericConfigChips.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostGenericConfigChips.test.ts @@ -4,17 +4,43 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import { DeferredPromise, timeout } from '../../../../../../base/common/async.js'; +import { isCancellationError } from '../../../../../../base/common/errors.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; import { IReference } from '../../../../../../base/common/lifecycle.js'; +import { constObservable } from '../../../../../../base/common/observable.js'; import { URI } from '../../../../../../base/common/uri.js'; import { mock } from '../../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; -import { IAgentHostService } from '../../../../../../platform/agentHost/common/agentService.js'; +import { IAgentConnection, IAgentHostService } from '../../../../../../platform/agentHost/common/agentService.js'; +import { AMBIENT_AGENT_HOST_AUTHORITY, IAgentHostConnectionsService } from '../../../../../../platform/agentHost/common/agentHostConnectionsService.js'; +import { AgentHostConnectionsService } from '../../../../../../platform/agentHost/browser/agentHostConnectionsService.js'; +import { agentHostAuthority } from '../../../../../../platform/agentHost/common/agentHostUri.js'; +import { IRemoteAgentHostConnectionInfo, IRemoteAgentHostService } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; import { IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; import { type ComponentToState, StateComponents } from '../../../../../../platform/agentHost/common/state/sessionState.js'; +import { ActionType } from '../../../../../../platform/agentHost/common/state/protocol/actions.js'; +import { ResolveSessionConfigResult, SessionConfigCompletionsResult } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; +import { SessionState } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import { IAgentHostEnablementService } from '../../../../../../platform/agentHost/common/agentHostEnablementService.js'; +import { IActionWidgetService } from '../../../../../../platform/actionWidget/browser/actionWidget.js'; +import { IActionListDelegate, IActionListItem } from '../../../../../../platform/actionWidget/browser/actionList.js'; +import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; +import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { IDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; +import { TestDialogService } from '../../../../../../platform/dialogs/test/common/testDialogService.js'; +import { IHoverService } from '../../../../../../platform/hover/browser/hover.js'; +import { NullHoverService } from '../../../../../../platform/hover/test/browser/nullHoverService.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; +import { IOpenerService } from '../../../../../../platform/opener/common/opener.js'; +import { NullOpenerService } from '../../../../../../platform/opener/test/common/nullOpenerService.js'; +import { IStorageService } from '../../../../../../platform/storage/common/storage.js'; import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js'; -import { IChatWidget } from '../../../browser/chat.js'; +import { IChatWidget, IChatWidgetViewModelChangeEvent } from '../../../browser/chat.js'; +import { IChatViewModel } from '../../../common/model/chatViewModel.js'; +import { IChatPhoneInputPresenter } from '../../../browser/widget/input/chatPhoneInputPresenter.js'; +import { TestStorageService } from '../../../../../test/common/workbenchTestServices.js'; +import { IPreferencesService } from '../../../../../services/preferences/common/preferences.js'; import { AgentHostGenericConfigChips } from '../../../browser/agentSessions/agentHost/agentHostGenericConfigChips.js'; import { IAgentHostNewSessionFolderService } from '../../../browser/agentSessions/agentHost/agentHostNewSessionFolderService.js'; import { IAgentHostSessionWorkingDirectoryResolver } from '../../../browser/agentSessions/agentHost/agentHostSessionWorkingDirectoryResolver.js'; @@ -63,7 +89,12 @@ suite('AgentHostGenericConfigChips', () => { const chips = disposables.add(new AgentHostGenericConfigChips( widget, disposables.add(new TestInstantiationService()), - agentHostService, + new class extends mock() { + override readonly onDidChangeSessionResolution = Event.None; + override resolveSessionResource() { + return { connection: agentHostService, connectionAuthority: AMBIENT_AGENT_HOST_AUTHORITY, backendSession: firstBackend }; + } + }(), provisionalService, {} as IAgentHostSessionWorkingDirectoryResolver, {} as IWorkspaceContextService, @@ -84,3 +115,345 @@ suite('AgentHostGenericConfigChips', () => { chips.dispose(); }); }); + +suite('AgentHostGenericConfigChips - remote sessions', () => { + const store = ensureNoDisposablesAreLeakedInTestSuite(); + const firstResource = URI.parse('remote-host-one-test-agent:/same-id'); + const secondResource = URI.parse('remote-host-two-test-agent:/same-id'); + const backendSession = URI.parse('ahp-session:/same-id'); + const workingDirectory = URI.file('/remote/workspace'); + + function makeConfig(): ResolveSessionConfigResult { + return { + schema: { type: 'object', properties: { + customChoice: { type: 'string', title: 'Custom Choice', enum: ['first', 'second'], enumLabels: ['First Option', 'Second Option'], sessionMutable: true }, + toggle: { type: 'boolean', title: 'Toggle', sessionMutable: true }, + mode: { type: 'string', title: 'Mode', enum: ['interactive', 'plan'], sessionMutable: true }, + autoApprove: { type: 'string', title: 'Approvals', enum: ['default', 'autoApprove'], sessionMutable: true }, + immutable: { type: 'string', title: 'Immutable', enum: ['first'], sessionMutable: false }, + unspecified: { type: 'boolean', title: 'Unspecified' }, + locked: { type: 'string', title: 'Locked', enum: ['first'], sessionMutable: true, readOnly: true }, + structured: { type: 'object', title: 'Structured', sessionMutable: true }, + } }, + values: { customChoice: 'first', toggle: false, locked: 'first' }, + }; + } + + function makeHost(config: ResolveSessionConfigResult) { + const changed = store.add(new Emitter()); + let state = new class extends mock() { + override readonly provider = 'test-agent'; + override readonly workingDirectories = [workingDirectory.toString()]; + override readonly config = config; + }(); + const subscriptions: { [K in StateComponents]?: IAgentSubscription } = { + [StateComponents.Session]: { + get value() { return state; }, + get verifiedValue() { return state; }, + onDidChange: changed.event, onWillApplyAction: Event.None, onDidApplyAction: Event.None, + }, + }; + const connection = new class extends mock() { + acquired = 0; + released = 0; + readonly dispatches: { channel: string; config: Record }[] = []; + readonly completionRequests: Parameters[0][] = []; + completionResult: Promise = Promise.resolve({ items: [{ value: 'second', label: 'Dynamic Option' }] }); + + override getSubscription(kind: T, resource: URI): IReference> { + assert.strictEqual(resource.toString(), backendSession.toString()); + const subscription = subscriptions[kind]; + assert.ok(subscription); + this.acquired++; + return { object: subscription, dispose: () => this.released++ }; + } + override dispatch(channel: string, action: Parameters[1]): void { + assert.strictEqual(action.type, ActionType.SessionConfigChanged); + if (action.type === ActionType.SessionConfigChanged) { + this.dispatches.push({ channel, config: action.config }); + } + } + override async sessionConfigCompletions(params: Parameters[0]): Promise { + this.completionRequests.push(params); + return this.completionResult; + } + }(); + return { connection, update: (config: ResolveSessionConfigResult) => { + state = { ...state, config }; + changed.fire(state); + } }; + } + + function setup(config = makeConfig(), connected = true) { + const host = makeHost(config); + const secondHost = makeHost(makeConfig()); + const connectionsChanged = store.add(new Emitter()); + const viewModelChanged = store.add(new Emitter()); + const remoteConnections = new Map(); + if (connected) { + remoteConnections.set('host-one', host.connection); + } + remoteConnections.set('host-two', secondHost.connection); + const ambient = new class extends mock() { + override readonly onAgentHostStart = Event.None; + override readonly onAgentHostExit = Event.None; + }(); + const remoteService = new class extends mock() { + override readonly onDidChangeConnections = connectionsChanged.event; + override get connections(): readonly IRemoteAgentHostConnectionInfo[] { + return [...remoteConnections.keys()].map(address => ({ address, name: address, status: { kind: 'connected' } })); + } + override getConnection(address: string) { return remoteConnections.get(address); } + override getConnectionByAuthority(authority: string) { + return [...remoteConnections].find(([address]) => agentHostAuthority(address) === authority)?.[1]; + } + }(); + const connectionsService = store.add(new AgentHostConnectionsService(ambient, remoteService)); + const registerPolicy = (address: string) => store.add(connectionsService.registerSessionResolutionPolicy(agentHostAuthority(address), { + sessionSchemeAlias: { ui: 'test-agent', backend: 'ahp-session' }, + })); + registerPolicy('host-one'); + registerPolicy('host-two'); + const widget = new class extends mock() { + override readonly onDidChangeViewModel = viewModelChanged.event; + override viewModel = new class extends mock() { + override readonly sessionResource = firstResource; + }(); + }(); + const shown = store.add(new Emitter()); + const actionWidget = new class extends mock() { + override isVisible = false; + showCount = 0; + labels: (string | undefined)[] = []; + select: (label: string) => Promise = async () => { throw new Error('Picker is not open'); }; + onHide: (() => void) | undefined; + override show(_id: string, _preview: boolean, items: readonly IActionListItem[], delegate: IActionListDelegate): void { + this.isVisible = true; + this.showCount++; + this.labels = items.map(item => item.label); + this.onHide = delegate.onHide; + this.select = async label => { + const item = items.find(item => item.label === label)?.item; + assert.ok(item); + await delegate.onSelect(item); + }; + shown.fire(); + } + override hide(): void { + this.isVisible = false; + const onHide = this.onHide; + this.onHide = undefined; + onHide?.(); + } + }(); + const refreshes: Parameters[] = []; + const instantiationService = store.add(new TestInstantiationService()); + instantiationService.set(IAgentHostConnectionsService, connectionsService); + instantiationService.set(IActionWidgetService, actionWidget); + const configuration = new TestConfigurationService(); + store.add(configuration.onDidChangeConfigurationEmitter); + instantiationService.set(IConfigurationService, configuration); + instantiationService.set(IHoverService, NullHoverService); + instantiationService.set(IOpenerService, NullOpenerService); + instantiationService.set(IDialogService, new TestDialogService()); + instantiationService.set(IStorageService, store.add(new TestStorageService())); + instantiationService.stub(IPreferencesService, {}); + instantiationService.stub(IAgentHostSessionWorkingDirectoryResolver, { resolve: () => undefined }); + instantiationService.stub(IWorkspaceContextService, { getWorkspace: () => ({ id: 'test', folders: [] }) }); + instantiationService.stub(IAgentHostNewSessionFolderService, { getFolder: () => undefined, getDefaultFolder: () => undefined }); + instantiationService.stub(IAgentHostUntitledProvisionalSessionService, { + onDidChange: Event.None, get: () => undefined, getResolvedConfig: () => undefined, + refreshResolvedConfig: async (...args) => { refreshes.push(args); }, + }); + instantiationService.stub(IAgentHostEnablementService, { managedSandboxEnforced: constObservable(false), managedSandboxAllowsBypass: constObservable(false) }); + instantiationService.stub(IChatPhoneInputPresenter, { enabled: constObservable(false) }); + const lane = store.add(instantiationService.createInstance(AgentHostGenericConfigChips, widget)); + const container = document.createElement('div'); + lane.render(container); + function trigger(property: string): HTMLElement { + const trigger = container.querySelector(`.agent-host-chat-input-picker-host-${property} [role="button"]`); + assert.ok(trigger); + return trigger; + } + return { + host, secondHost, config, lane, container, widget, remoteConnections, connectionsChanged, actionWidget, refreshes, + trigger, + addHost: (address: string, connection: IAgentConnection) => { + remoteConnections.set(address, connection); + registerPolicy(address); + connectionsChanged.fire(); + }, + open: async (property = 'customChoice') => { + const opened = Event.toPromise(shown.event); + trigger(property).dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); + await opened; + }, + switchSession: (sessionResource = secondResource) => { + const previousSessionResource = widget.viewModel.sessionResource; + widget.viewModel = new class extends mock() { override readonly sessionResource = sessionResource; }(); + viewModelChanged.fire({ previousSessionResource, currentSessionResource: sessionResource }); + }, + }; + } + + test('renders only supported mutable generic properties and preserves read-only presentation', () => { + const { container } = setup(); + assert.deepStrictEqual({ + chips: container.querySelectorAll('.agent-host-generic-chip-slot').length, + buttons: [...container.querySelectorAll('[role="button"]')].map(element => element.getAttribute('aria-label')), + readOnly: container.querySelectorAll('[aria-readonly="true"]').length, + }, { + chips: 3, + buttons: ['Custom Choice: First Option', 'Toggle: Off'], + readOnly: 1, + }); + }); + + test('sends the raw selection to the owning session and refreshes with its advertised provider', async () => { + const { open, actionWidget, host, secondHost, refreshes, config, trigger } = setup(); + await open(); + await actionWidget.select('Second Option'); + host.update({ ...config, values: { ...config.values, customChoice: 'second' } }); + assert.deepStrictEqual({ + writes: host.connection.dispatches, + otherWrites: secondHost.connection.dispatches, + refreshes: refreshes.map(([resource, provider, directory, values]) => [resource.toString(), provider, directory?.toString(), values]), + label: trigger('customChoice').getAttribute('aria-label'), + }, { + writes: [{ channel: backendSession.toString(), config: { customChoice: 'second' } }], + otherWrites: [], + refreshes: [[firstResource.toString(), 'test-agent', workingDirectory.toString(), { ...config.values, customChoice: 'second' }]], + label: 'Custom Choice: Second Option', + }); + }); + + test('preserves boolean values when writing a generic property', async () => { + const { open, actionWidget, host } = setup(); + await open('toggle'); + await actionWidget.select('On'); + assert.deepStrictEqual(host.connection.dispatches, [{ channel: backendSession.toString(), config: { toggle: true } }]); + }); + + test('routes a reopened picker to the second host even with the same backend session URI', async () => { + const { switchSession, open, actionWidget, host, secondHost } = setup(); + switchSession(); + await open(); + await actionWidget.select('Second Option'); + assert.deepStrictEqual([host.connection.dispatches, secondHost.connection.dispatches], [ + [], [{ channel: backendSession.toString(), config: { customChoice: 'second' } }], + ]); + }); + + test('routes selections to a third, fourth and fifth host added after the picker was created', async () => { + const { addHost, switchSession, open, actionWidget, host, secondHost, refreshes } = setup(); + const additionalHosts = ['host-three', 'host-four', 'host-five'].map(address => ({ address, ...makeHost(makeConfig()) })); + for (const { address, connection } of additionalHosts) { + addHost(address, connection); + switchSession(URI.parse(`remote-${agentHostAuthority(address)}-test-agent:/same-id`)); + await open(); + await actionWidget.select('Second Option'); + } + assert.deepStrictEqual({ + originalWrites: [host.connection.dispatches, secondHost.connection.dispatches], + additionalWrites: additionalHosts.map(({ connection }) => connection.dispatches), + refreshTargets: refreshes.map(([resource, provider]) => [resource.toString(), provider]), + }, { + originalWrites: [[], []], + additionalWrites: additionalHosts.map(() => [{ channel: backendSession.toString(), config: { customChoice: 'second' } }]), + refreshTargets: additionalHosts.map(({ address }) => [`remote-${agentHostAuthority(address)}-test-agent:/same-id`, 'test-agent']), + }); + }); + + test('waits for the remote connection and releases subscriptions when disconnected', () => { + const { host, lane, container, remoteConnections, connectionsChanged } = setup(makeConfig(), false); + assert.strictEqual(container.childElementCount, 0); + remoteConnections.set('host-one', host.connection); + connectionsChanged.fire(); + const connectedChips = container.childElementCount; + remoteConnections.delete('host-one'); + connectionsChanged.fire(); + assert.deepStrictEqual({ + connectedChips, disconnectedChips: container.childElementCount, + acquired: host.connection.acquired, released: host.connection.released, + }, { connectedChips: 3, disconnectedChips: 0, acquired: 4, released: 4 }); + lane.dispose(); + }); + + test('keeps an open picker when another connection changes', async () => { + const { open, actionWidget, connectionsChanged, host } = setup(); + await open(); + const acquired = host.connection.acquired; + connectionsChanged.fire(); + assert.deepStrictEqual({ open: actionWidget.isVisible, acquired: host.connection.acquired }, { open: true, acquired }); + }); + + for (const change of ['session', 'connection', 'readOnly', 'sessionMutable'] as const) { + test(`rejects an obsolete selection after a ${change} change`, async () => { + const { open, actionWidget, host, secondHost, config, switchSession, remoteConnections, connectionsChanged } = setup(); + await open(); + const select = actionWidget.select; + if (change === 'session') { + switchSession(); + } else if (change === 'connection') { + remoteConnections.set('host-one', secondHost.connection); + connectionsChanged.fire(); + } else { + host.update({ + ...config, + schema: { ...config.schema, properties: { ...config.schema.properties, + customChoice: { ...config.schema.properties.customChoice, [change]: change === 'readOnly' }, + } }, + }); + } + await assert.rejects(select('Second Option'), isCancellationError); + assert.deepStrictEqual([host.connection.dispatches, secondHost.connection.dispatches], [[], []]); + }); + } + + test('requests dynamic options from the remote provider and current working directory', async () => { + const config = makeConfig(); + config.schema.properties.customChoice = { ...config.schema.properties.customChoice, enumDynamic: true }; + const { open, host, actionWidget } = setup(config); + await open(); + assert.deepStrictEqual({ + requests: host.connection.completionRequests.map(request => ({ ...request, workingDirectory: request.workingDirectory?.toString() })), labels: actionWidget.labels, + }, { + requests: [{ provider: 'test-agent', property: 'customChoice', query: undefined, workingDirectory: workingDirectory.toString(), config: config.values }], + labels: ['Dynamic Option'], + }); + }); + + test('does not show late completions after switching to a host with the same backend session URI', async () => { + const config = makeConfig(); + config.schema.properties.customChoice.enumDynamic = true; + const { trigger, host, switchSession, actionWidget } = setup(config); + const pending = new DeferredPromise(); + store.add({ dispose: () => pending.cancel() }); + host.connection.completionResult = pending.p; + trigger('customChoice').click(); + switchSession(); + pending.complete({ items: [{ value: 'second', label: 'Obsolete Option' }] }); + await timeout(0); + assert.strictEqual(actionWidget.showCount, 0); + }); + + test('does not open after the host makes a field read-only during a dynamic lookup', async () => { + const config = makeConfig(); + config.schema.properties.customChoice.enumDynamic = true; + const { trigger, host, actionWidget } = setup(config); + const pending = new DeferredPromise(); + store.add({ dispose: () => pending.cancel() }); + host.connection.completionResult = pending.p; + trigger('customChoice').click(); + host.update({ + ...config, + schema: { ...config.schema, properties: { ...config.schema.properties, + customChoice: { ...config.schema.properties.customChoice, readOnly: true }, + } }, + }); + pending.complete({ items: [{ value: 'second', label: 'Obsolete Option' }] }); + await timeout(0); + assert.strictEqual(actionWidget.showCount, 0); + }); + +}); diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostUntitledProvisionalSessionService.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostUntitledProvisionalSessionService.test.ts index 67b0e79b5dadf5..d59a2e5b6245a4 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostUntitledProvisionalSessionService.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostUntitledProvisionalSessionService.test.ts @@ -7,6 +7,7 @@ import assert from 'assert'; import { DeferredPromise, timeout } from '../../../../../../base/common/async.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; import { DisposableStore } from '../../../../../../base/common/lifecycle.js'; +import { ResourceMap } from '../../../../../../base/common/map.js'; import { constObservable, derived, observableValue } from '../../../../../../base/common/observable.js'; import { ExtUri } from '../../../../../../base/common/resources.js'; import { URI } from '../../../../../../base/common/uri.js'; @@ -17,6 +18,7 @@ import { TestConfigurationService } from '../../../../../../platform/configurati import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js'; import { IAgentCreateSessionConfig, IAgentHostService, IAgentResolveSessionConfigParams } from '../../../../../../platform/agentHost/common/agentService.js'; +import { AMBIENT_AGENT_HOST_AUTHORITY, IAgentHostConnectionsService, IAgentHostSessionResolution } from '../../../../../../platform/agentHost/common/agentHostConnectionsService.js'; import { ActionType } from '../../../../../../platform/agentHost/common/state/protocol/actions.js'; import type { ResolveSessionConfigResult } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; import { CustomizationType, type ClientPluginCustomization, type ConfigSchema, type SessionActiveClient } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; @@ -31,6 +33,7 @@ import { AgentHostUntitledProvisionalSessionService, IAgentHostUntitledProvision import { AgentHostNewSessionFolderService, IAgentHostNewSessionFolderService } from '../../../browser/agentSessions/agentHost/agentHostNewSessionFolderService.js'; import { AgentHostImportConversationStore, IAgentHostImportConversationStore } from '../../../browser/agentSessions/agentHost/agentHostImportConversationStore.js'; import { areCustomizationScopeRootsEqual, IAgentHostActiveClientService } from '../../../browser/agentSessions/agentHost/agentHostActiveClientService.js'; +import { toAgentHostBackendSessionUri } from '../../../browser/agentSessions/agentHost/agentHostSessionUri.js'; // ---- Mocks ----------------------------------------------------------------- @@ -180,6 +183,9 @@ suite('AgentHostUntitledProvisionalSessionService', () => { }); let agentHost: MockAgentHostService; + let sessionResolutions: ResourceMap; + let onDidChangeSessionResolution: Emitter; + let warnings: string[]; let importStore: AgentHostImportConversationStore; let provisional: IAgentHostUntitledProvisionalSessionService; let folderService: IAgentHostNewSessionFolderService; @@ -197,6 +203,9 @@ suite('AgentHostUntitledProvisionalSessionService', () => { setup(async () => { agentHost = ds.add(new MockAgentHostService()); + sessionResolutions = new ResourceMap(); + onDidChangeSessionResolution = ds.add(new Emitter()); + warnings = []; workspaceTrusted = true; untrustedFolders = new Set(); workspaceFolders = []; @@ -208,7 +217,19 @@ suite('AgentHostUntitledProvisionalSessionService', () => { onDidChangeWorkspaceFolders = ds.add(new Emitter()); const insta = ds.add(new TestInstantiationService()); insta.stub(IAgentHostService, agentHost); - insta.stub(ILogService, new NullLogService()); + insta.stub(IAgentHostConnectionsService, { + onDidChangeSessionResolution: onDidChangeSessionResolution.event, + resolveSessionResource: sessionResource => { + if (sessionResolutions.has(sessionResource)) { + return sessionResolutions.get(sessionResource); + } + const backendSession = toAgentHostBackendSessionUri(sessionResource); + return backendSession ? { connection: agentHost, connectionAuthority: AMBIENT_AGENT_HOST_AUTHORITY, backendSession } : undefined; + }, + }); + insta.stub(ILogService, new class extends NullLogService { + override warn(message: string): void { warnings.push(message); } + }()); insta.stub(IChatService, new MockChatService()); insta.stub(IConfigurationService, new TestConfigurationService()); insta.stub(IWorkbenchEnvironmentService, { get isSessionsWindow() { return isSessionsWindow; } } as Partial); @@ -945,6 +966,103 @@ suite('AgentHostUntitledProvisionalSessionService', () => { assert.deepStrictEqual(provisional.getResolvedConfig(ui), { schema: makeSchema(true), values: { isolation: 'folder' } }); }); + test('refreshResolvedConfig routes matching backend session IDs to their owning hosts', async () => { + const firstHost = ds.add(new MockAgentHostService()); + const secondHost = ds.add(new MockAgentHostService()); + const firstSession = URI.parse('remote-host-one-test-agent:/same-session'); + const secondSession = URI.parse('remote-host-two-test-agent:/same-session'); + const backendSession = URI.parse('ahp-session:/same-session'); + const workingDirectory = URI.file('/workspace'); + const firstConfig: ResolveSessionConfigResult = { schema: makeSchema(false), values: { isolation: 'worktree' } }; + const secondConfig: ResolveSessionConfigResult = { schema: makeSchema(true), values: { isolation: 'folder' } }; + sessionResolutions.set(firstSession, { connection: firstHost, connectionAuthority: 'host-one', backendSession }); + sessionResolutions.set(secondSession, { connection: secondHost, connectionAuthority: 'host-two', backendSession }); + firstHost.resolveQueue = [firstConfig]; + secondHost.resolveQueue = [secondConfig]; + + await Promise.all([ + provisional.refreshResolvedConfig(firstSession, 'test-agent', workingDirectory, firstConfig.values), + provisional.refreshResolvedConfig(secondSession, 'test-agent', workingDirectory, secondConfig.values), + ]); + + assert.deepStrictEqual({ + localCalls: agentHost.resolveCalls, + firstCalls: firstHost.resolveCalls, + secondCalls: secondHost.resolveCalls, + firstOverlay: provisional.getResolvedConfig(firstSession), + secondOverlay: provisional.getResolvedConfig(secondSession), + }, { + localCalls: [], + firstCalls: [{ provider: 'test-agent', workingDirectory, config: firstConfig.values }], + secondCalls: [{ provider: 'test-agent', workingDirectory, config: secondConfig.values }], + firstOverlay: firstConfig, + secondOverlay: secondConfig, + }); + }); + + test('refreshResolvedConfig reports a disconnected host instead of falling back to the local host', async () => { + const session = URI.parse('remote-host-test-agent:/disconnected'); + + await provisional.refreshResolvedConfig(session, 'test-agent', undefined, {}); + + assert.deepStrictEqual({ + localCalls: agentHost.resolveCalls, + overlay: provisional.getResolvedConfig(session), + warnings, + }, { + localCalls: [], + overlay: undefined, + warnings: ['[AgentHostProvisional] schema re-resolve failed: No connected agent host is available for session configuration'], + }); + }); + + test('refreshResolvedConfig discards an in-flight result across disconnect and reconnect', async () => { + const remoteHost = ds.add(new MockAgentHostService()); + const session = URI.parse('remote-host-test-agent:/reconnected'); + const resolution = { connection: remoteHost, connectionAuthority: 'host', backendSession: URI.parse('ahp-session:/reconnected') }; + sessionResolutions.set(session, resolution); + const stale = new DeferredPromise(); + cleanup.add({ dispose: () => stale.cancel() }); + remoteHost.resolveQueue = [stale.p]; + const pending = provisional.refreshResolvedConfig(session, 'test-agent', undefined, {}); + + sessionResolutions.set(session, undefined); + onDidChangeSessionResolution.fire(); + sessionResolutions.set(session, resolution); + onDidChangeSessionResolution.fire(); + stale.complete({ schema: makeSchema(false), values: { isolation: 'worktree' } }); + await pending; + + assert.strictEqual(provisional.getResolvedConfig(session), undefined); + }); + + for (const change of ['connection', 'backend session'] as const) { + test(`refreshResolvedConfig invalidates its overlay when the ${change} changes`, async () => { + const remoteHost = ds.add(new MockAgentHostService()); + const session = URI.parse('remote-host-test-agent:/running'); + const resolution = { connection: remoteHost, connectionAuthority: 'host', backendSession: URI.parse('ahp-session:/running') }; + const config: ResolveSessionConfigResult = { schema: makeSchema(false), values: { isolation: 'worktree' } }; + sessionResolutions.set(session, resolution); + remoteHost.resolveQueue = [config]; + let changes = 0; + cleanup.add(provisional.onDidChange(() => changes++)); + await provisional.refreshResolvedConfig(session, 'test-agent', undefined, {}); + + onDidChangeSessionResolution.fire(); + const unchanged = provisional.getResolvedConfig(session); + sessionResolutions.set(session, { + ...resolution, + connection: change === 'connection' ? ds.add(new MockAgentHostService()) : remoteHost, + backendSession: change === 'backend session' ? URI.parse('ahp-session:/replacement') : resolution.backendSession, + }); + onDidChangeSessionResolution.fire(); + + assert.deepStrictEqual({ unchanged, invalidated: provisional.getResolvedConfig(session), changes }, { + unchanged: config, invalidated: undefined, changes: 2, + }); + }); + } + test('optimistic merge: overlay.values reflects partial before re-resolve completes', async () => { const ui = untitledChatUri('d'); // First applyConfigChange: seed an overlay. From 99fa7a1dc28912185227e60dd9182e8b560c83bb Mon Sep 17 00:00:00 2001 From: Osvaldo Ortega Date: Thu, 10 Sep 2026 17:55:29 -0700 Subject: [PATCH 2/4] Fix remote session config recovery and option fallback Recover early configuration subscriptions when the host announces the session, and retain schema options when dynamic lookups fail. Preserve cancellation and stale-target handling, with regression coverage for both paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/agentHostModePicker.fixture.ts | 1 + .../agentHost/agentHostChatInputPicker.ts | 39 +++- .../agentHost/agentHostGenericConfigChips.ts | 6 +- .../agentHostSessionConfigSubscription.ts | 40 ++++ .../agentHostChatInputPicker.test.ts | 3 + .../agentHostGenericConfigChips.test.ts | 201 +++++++++++++++--- 6 files changed, 251 insertions(+), 39 deletions(-) create mode 100644 src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionConfigSubscription.ts diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostModePicker.fixture.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostModePicker.fixture.ts index 597c592807ebd7..6785a4fc475182 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostModePicker.fixture.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostModePicker.fixture.ts @@ -155,6 +155,7 @@ async function render(context: ComponentFixtureContext, mode: string, permission [StateComponents.Session]: { value: state, verifiedValue: state, onDidChange: changed.event, onWillApplyAction: Event.None, onDidApplyAction: Event.None }, }; const connection = new class extends mock() { + override readonly onDidNotification = Event.None; override getSubscription(kind: T): IReference> { const subscription = subscriptions[kind]; if (!subscription) { 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 3d4b439b9e30c8..2e173d3cffde5c 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts @@ -11,7 +11,7 @@ import { BaseActionViewItem } from '../../../../../../base/browser/ui/actionbar/ import { Delayer } from '../../../../../../base/common/async.js'; import { CancellationTokenSource } from '../../../../../../base/common/cancellation.js'; import { Codicon } from '../../../../../../base/common/codicons.js'; -import { CancellationError, onUnexpectedError } from '../../../../../../base/common/errors.js'; +import { CancellationError, isCancellationError, onUnexpectedError } from '../../../../../../base/common/errors.js'; import { Disposable, DisposableStore, IDisposable, MutableDisposable, toDisposable } from '../../../../../../base/common/lifecycle.js'; import { autorun, observableSignal } from '../../../../../../base/common/observable.js'; import { isEqual } from '../../../../../../base/common/resources.js'; @@ -31,7 +31,7 @@ import { SessionConfigKey } from '../../../../../../platform/agentHost/common/se import { ClaudeSessionConfigKey } from '../../../../../../platform/agentHost/common/claudeSessionConfigKeys.js'; import { CodexSessionConfigKey } from '../../../../../../platform/agentHost/common/codexSessionConfigKeys.js'; import { ActionType } from '../../../../../../platform/agentHost/common/state/protocol/actions.js'; -import type { ResolveSessionConfigResult, SessionConfigPropertySchema, SessionConfigValueItem } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; +import type { ResolveSessionConfigResult, SessionConfigCompletionsResult, SessionConfigPropertySchema, SessionConfigValueItem } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; import type { SessionState } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; import { StateComponents } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { type IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; @@ -39,6 +39,7 @@ 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 { ILogService } from '../../../../../../platform/log/common/log.js'; import { AgentSandboxEnabledSettingValue, AgentSandboxEnabledValue, isAgentSandboxEnabledValue } from '../../../../../../platform/sandbox/common/settings.js'; import { IAction, toAction } from '../../../../../../base/common/actions.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; @@ -54,6 +55,7 @@ import { IAgentHostSessionWorkingDirectoryResolver } from './agentHostSessionWor import { IAgentHostNewSessionFolderService } from './agentHostNewSessionFolderService.js'; import { IAgentHostUntitledProvisionalSessionService } from './agentHostUntitledProvisionalSessionService.js'; import { resolveAgentHostChatSession, toAgentHostBackendSessionUri } from './agentHostSessionUri.js'; +import { retrySessionConfigSubscriptionOnCreation } from './agentHostSessionConfigSubscription.js'; import { getCompactCodicon } from '../../chatIcons.js'; import { IChatPhoneInputPresenter } from '../../widget/input/chatPhoneInputPresenter.js'; import { AGENT_HOST_PERMISSIONS_SETTINGS_QUERY, createModePickerModeItems, createModePickerPermissionsItems, getModePermissionsPickerAccessibilityProvider, getModePermissionsPickerOptions, getModePickerAriaLabel, IModePickerPermissions, IModePickerTrigger, isWellKnownAutoApproveSchema, renderModePickerTrigger, shouldCombineModeAndPermissions } from './agentHostModePickerPresentation.js'; @@ -402,6 +404,7 @@ export class AgentHostChatInputPicker extends Disposable { @IAgentHostEnablementService private readonly _agentHostEnablementService: IAgentHostEnablementService, @IChatPhoneInputPresenter private readonly _phoneInputPresenter: IChatPhoneInputPresenter, @IPreferencesService private readonly _preferencesService: IPreferencesService, + @ILogService private readonly _logService: ILogService, ) { super(); @@ -527,12 +530,13 @@ export class AgentHostChatInputPicker extends Disposable { this._renderChip(); this._sandboxConfigChanged.trigger(undefined); }); + const creationListener = retrySessionConfigSubscriptionOnCreation(resolution.connection, resolution.backendSession, sub, () => this._reattach()); this._subRef.value = { ...resolution, sub, sessionResource, generation: this._sessionGeneration, - dispose: () => { listener.dispose(); ref.dispose(); }, + dispose: () => { creationListener.dispose(); listener.dispose(); ref.dispose(); }, }; this._renderChip(); } @@ -969,17 +973,30 @@ export class AgentHostChatInputPicker extends Disposable { if (!context || !this._isCurrentTarget(context)) { throw new CancellationError(); } - const result = await context.connection.sessionConfigCompletions({ - provider: context.provider, - property, - query, - workingDirectory: this._readWorkingDirectory(), - config: this._readCurrentValues(), - }); + let result: SessionConfigCompletionsResult | undefined; + try { + result = await context.connection.sessionConfigCompletions({ + provider: context.provider, + property, + query, + workingDirectory: this._readWorkingDirectory(), + config: this._readCurrentValues(), + }); + } catch (error) { + if (isCancellationError(error)) { + throw error; + } + if (!this._isCurrentTarget(context)) { + throw new CancellationError(); + } + this._logService.warn('[AgentHostChatInputPicker] Failed to load dynamic session configuration options; using schema options.'); + } if (!this._isCurrentTarget(context)) { throw new CancellationError(); } - return this._filterAutoApproveItems(result.items.map(item => this._fromCompletion(item)), property); + if (result) { + return this._filterAutoApproveItems(result.items.map(item => this._fromCompletion(item)), property); + } } return this._filterAutoApproveItems((schema.enum ?? []).map((value, index) => ({ value: String(value), diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostGenericConfigChips.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostGenericConfigChips.ts index f3458ecd70faf3..eb37a29f0f62a5 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostGenericConfigChips.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostGenericConfigChips.ts @@ -22,6 +22,7 @@ import { IAgentHostNewSessionFolderService } from './agentHostNewSessionFolderSe import { IAgentHostUntitledProvisionalSessionService } from './agentHostUntitledProvisionalSessionService.js'; import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js'; import { resolveAgentHostChatSession, toAgentHostBackendSessionUri } from './agentHostSessionUri.js'; +import { retrySessionConfigSubscriptionOnCreation } from './agentHostSessionConfigSubscription.js'; /** * Direct-render chip lane for agent-host session-config properties that are @@ -106,18 +107,19 @@ export class AgentHostGenericConfigChips extends Disposable { this._initialResolved = undefined; this._cancelInitialResolve(); const current = this._subRef.value; - if (current && isEqual(current.sessionResource, sessionResource) && current.connection === resolution.connection && isEqual(current.backendSession, resolution.backendSession)) { + if (current && !(current.sub.value instanceof Error) && isEqual(current.sessionResource, sessionResource) && current.connection === resolution.connection && isEqual(current.backendSession, resolution.backendSession)) { this._sync(); return; } const ref = resolution.connection.getSubscription(StateComponents.Session, resolution.backendSession, 'AgentHostGenericConfigChips'); const sub = ref.object; const listener = sub.onDidChange(() => this._sync()); + const creationListener = retrySessionConfigSubscriptionOnCreation(resolution.connection, resolution.backendSession, sub, () => this._reattach()); this._subRef.value = { ...resolution, sub, sessionResource, - dispose: () => { listener.dispose(); ref.dispose(); }, + dispose: () => { creationListener.dispose(); listener.dispose(); ref.dispose(); }, }; this._sync(); } diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionConfigSubscription.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionConfigSubscription.ts new file mode 100644 index 00000000000000..a8ebbf57ce6e63 --- /dev/null +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionConfigSubscription.ts @@ -0,0 +1,40 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { DisposableStore, IDisposable } from '../../../../../../base/common/lifecycle.js'; +import { isEqual } from '../../../../../../base/common/resources.js'; +import { URI } from '../../../../../../base/common/uri.js'; +import { IAgentConnection } from '../../../../../../platform/agentHost/common/agentService.js'; +import { IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; +import { SessionState } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import { NotificationType } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; + +/** Retries an early subscription failure once the host announces that the session exists. */ +export function retrySessionConfigSubscriptionOnCreation(connection: IAgentConnection, session: URI, subscription: IAgentSubscription, retry: () => void): IDisposable { + const store = new DisposableStore(); + let creationAnnounced = false; + const retryIfFailed = () => { + if (creationAnnounced && subscription.value instanceof Error) { + creationAnnounced = false; + // Let every consumer observe the error before retrying disposes the shared subscription. + queueMicrotask(() => { + if (!store.isDisposed) { + retry(); + } + }); + } + }; + store.add(connection.onDidNotification(notification => { + if (notification.type === NotificationType.SessionAdded && isEqual(URI.parse(notification.summary.resource), session)) { + creationAnnounced = subscription.value === undefined || subscription.value instanceof Error; + retryIfFailed(); + } + })); + store.add(subscription.onDidChange(() => creationAnnounced = false)); + if (subscription.onDidError) { + store.add(subscription.onDidError(retryIfFailed)); + } + return store; +} 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 index 9f8e6814b775cc..312c2acc74c352 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts @@ -21,6 +21,7 @@ import { toAgentHostBackendSessionUri } from '../../../browser/agentSessions/age import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; import { ConfigurationTarget, IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; +import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js'; import { IDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; import { TestDialogService } from '../../../../../../platform/dialogs/test/common/testDialogService.js'; import { IHoverService } from '../../../../../../platform/hover/browser/hover.js'; @@ -155,6 +156,7 @@ suite('AgentHostChatInputPicker - combined mode and permissions', () => { } } }(); + instantiationService.set(ILogService, new NullLogService()); instantiationService.stub(IAgentHostConnectionsService, { ambientConnection: connection, onDidChangeSessionResolution: Event.None, @@ -774,6 +776,7 @@ suite('AgentHostChatInputPicker - sandbox toggle', () => { override readonly enabled = constObservable(false); }(), new class extends mock() { }(), + new NullLogService(), )); widget.viewModel = new class extends mock() { override readonly sessionResource = URI.from({ scheme: SessionType.AgentHostCopilot, path: '/test-session' }); diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostGenericConfigChips.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostGenericConfigChips.test.ts index f2a4c9b9e8b102..c4b0e58123e55c 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostGenericConfigChips.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostGenericConfigChips.test.ts @@ -5,7 +5,7 @@ import assert from 'assert'; import { DeferredPromise, timeout } from '../../../../../../base/common/async.js'; -import { isCancellationError } from '../../../../../../base/common/errors.js'; +import { CancellationError, isCancellationError } from '../../../../../../base/common/errors.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; import { IReference } from '../../../../../../base/common/lifecycle.js'; import { constObservable } from '../../../../../../base/common/observable.js'; @@ -17,11 +17,13 @@ import { AMBIENT_AGENT_HOST_AUTHORITY, IAgentHostConnectionsService } from '../. import { AgentHostConnectionsService } from '../../../../../../platform/agentHost/browser/agentHostConnectionsService.js'; import { agentHostAuthority } from '../../../../../../platform/agentHost/common/agentHostUri.js'; import { IRemoteAgentHostConnectionInfo, IRemoteAgentHostService } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; -import { IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; +import { AgentSubscriptionManager, IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; import { type ComponentToState, StateComponents } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { ActionType } from '../../../../../../platform/agentHost/common/state/protocol/actions.js'; import { ResolveSessionConfigResult, SessionConfigCompletionsResult } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; -import { SessionState } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import { SessionState, SessionSummary } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import { INotification, NotificationType } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; +import { IStateSnapshot } from '../../../../../../platform/agentHost/common/state/sessionProtocol.js'; import { IAgentHostEnablementService } from '../../../../../../platform/agentHost/common/agentHostEnablementService.js'; import { IActionWidgetService } from '../../../../../../platform/actionWidget/browser/actionWidget.js'; import { IActionListDelegate, IActionListItem } from '../../../../../../platform/actionWidget/browser/actionList.js'; @@ -32,6 +34,7 @@ import { TestDialogService } from '../../../../../../platform/dialogs/test/commo import { IHoverService } from '../../../../../../platform/hover/browser/hover.js'; import { NullHoverService } from '../../../../../../platform/hover/test/browser/nullHoverService.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; +import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js'; import { IOpenerService } from '../../../../../../platform/opener/common/opener.js'; import { NullOpenerService } from '../../../../../../platform/opener/test/common/nullOpenerService.js'; import { IStorageService } from '../../../../../../platform/storage/common/storage.js'; @@ -42,6 +45,8 @@ import { IChatPhoneInputPresenter } from '../../../browser/widget/input/chatPhon import { TestStorageService } from '../../../../../test/common/workbenchTestServices.js'; import { IPreferencesService } from '../../../../../services/preferences/common/preferences.js'; import { AgentHostGenericConfigChips } from '../../../browser/agentSessions/agentHost/agentHostGenericConfigChips.js'; +import { AgentHostChatInputPicker } from '../../../browser/agentSessions/agentHost/agentHostChatInputPicker.js'; +import { retrySessionConfigSubscriptionOnCreation } from '../../../browser/agentSessions/agentHost/agentHostSessionConfigSubscription.js'; import { IAgentHostNewSessionFolderService } from '../../../browser/agentSessions/agentHost/agentHostNewSessionFolderService.js'; import { IAgentHostSessionWorkingDirectoryResolver } from '../../../browser/agentSessions/agentHost/agentHostSessionWorkingDirectoryResolver.js'; import { IAgentHostUntitledProvisionalSessionService } from '../../../browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.js'; @@ -73,6 +78,7 @@ suite('AgentHostGenericConfigChips', () => { const released: string[] = []; const agentHostService = new class extends mock() { declare readonly _serviceBrand: undefined; + override readonly onDidNotification = Event.None; override getSubscription(_kind: T, resource: URI, _owner: string): IReference> { acquired.push(resource.toString()); @@ -125,16 +131,18 @@ suite('AgentHostGenericConfigChips - remote sessions', () => { function makeConfig(): ResolveSessionConfigResult { return { - schema: { type: 'object', properties: { - customChoice: { type: 'string', title: 'Custom Choice', enum: ['first', 'second'], enumLabels: ['First Option', 'Second Option'], sessionMutable: true }, - toggle: { type: 'boolean', title: 'Toggle', sessionMutable: true }, - mode: { type: 'string', title: 'Mode', enum: ['interactive', 'plan'], sessionMutable: true }, - autoApprove: { type: 'string', title: 'Approvals', enum: ['default', 'autoApprove'], sessionMutable: true }, - immutable: { type: 'string', title: 'Immutable', enum: ['first'], sessionMutable: false }, - unspecified: { type: 'boolean', title: 'Unspecified' }, - locked: { type: 'string', title: 'Locked', enum: ['first'], sessionMutable: true, readOnly: true }, - structured: { type: 'object', title: 'Structured', sessionMutable: true }, - } }, + schema: { + type: 'object', properties: { + customChoice: { type: 'string', title: 'Custom Choice', enum: ['first', 'second'], enumLabels: ['First Option', 'Second Option'], sessionMutable: true }, + toggle: { type: 'boolean', title: 'Toggle', sessionMutable: true }, + mode: { type: 'string', title: 'Mode', enum: ['interactive', 'plan'], sessionMutable: true }, + autoApprove: { type: 'string', title: 'Approvals', enum: ['default', 'autoApprove'], sessionMutable: true }, + immutable: { type: 'string', title: 'Immutable', enum: ['first'], sessionMutable: false }, + unspecified: { type: 'boolean', title: 'Unspecified' }, + locked: { type: 'string', title: 'Locked', enum: ['first'], sessionMutable: true, readOnly: true }, + structured: { type: 'object', title: 'Structured', sessionMutable: true }, + } + }, values: { customChoice: 'first', toggle: false, locked: 'first' }, }; } @@ -154,6 +162,7 @@ suite('AgentHostGenericConfigChips - remote sessions', () => { }, }; const connection = new class extends mock() { + override readonly onDidNotification = Event.None; acquired = 0; released = 0; readonly dispatches: { channel: string; config: Record }[] = []; @@ -178,20 +187,22 @@ suite('AgentHostGenericConfigChips - remote sessions', () => { return this.completionResult; } }(); - return { connection, update: (config: ResolveSessionConfigResult) => { - state = { ...state, config }; - changed.fire(state); - } }; + return { + connection, update: (config: ResolveSessionConfigResult) => { + state = { ...state, config }; + changed.fire(state); + } + }; } - function setup(config = makeConfig(), connected = true) { + function setup(config = makeConfig(), connected = true, firstConnection?: IAgentConnection) { const host = makeHost(config); const secondHost = makeHost(makeConfig()); const connectionsChanged = store.add(new Emitter()); const viewModelChanged = store.add(new Emitter()); const remoteConnections = new Map(); if (connected) { - remoteConnections.set('host-one', host.connection); + remoteConnections.set('host-one', firstConnection ?? host.connection); } remoteConnections.set('host-two', secondHost.connection); const ambient = new class extends mock() { @@ -247,7 +258,11 @@ suite('AgentHostGenericConfigChips - remote sessions', () => { } }(); const refreshes: Parameters[] = []; + const warnings: string[] = []; const instantiationService = store.add(new TestInstantiationService()); + instantiationService.set(ILogService, new class extends NullLogService { + override warn(message: string): void { warnings.push(message); } + }()); instantiationService.set(IAgentHostConnectionsService, connectionsService); instantiationService.set(IActionWidgetService, actionWidget); const configuration = new TestConfigurationService(); @@ -276,7 +291,7 @@ suite('AgentHostGenericConfigChips - remote sessions', () => { return trigger; } return { - host, secondHost, config, lane, container, widget, remoteConnections, connectionsChanged, actionWidget, refreshes, + host, secondHost, config, lane, container, widget, instantiationService, remoteConnections, connectionsChanged, actionWidget, refreshes, warnings, trigger, addHost: (address: string, connection: IAgentConnection) => { remoteConnections.set(address, connection); @@ -379,6 +394,93 @@ suite('AgentHostGenericConfigChips - remote sessions', () => { lane.dispose(); }); + for (const notificationFirst of [false, true]) { + test(`recovers a not-yet-created session when the creation notification arrives ${notificationFirst ? 'before' : 'after'} the subscription error`, async () => { + const config = makeConfig(); + const notifications = store.add(new Emitter()); + const initial = new DeferredPromise(); + store.add({ dispose: () => initial.cancel() }); + let subscribeCalls = 0; + let seq = 0; + const state = new class extends mock() { + override readonly provider = 'test-agent'; + override readonly config = config; + }(); + const manager = store.add(new AgentSubscriptionManager('test-client', () => ++seq, () => { }, async resource => { + subscribeCalls++; + return subscribeCalls === 1 ? initial.p : { resource: resource.toString(), state, fromSeq: 0 }; + }, () => { })); + const connection = new class extends mock() { + override readonly onDidNotification = notifications.event; + override getSubscription(kind: T, resource: URI, owner: string): IReference> { + return manager.getSubscription(kind, resource, owner); + } + }(); + const { container, widget, instantiationService } = setup(config, true, connection); + const standalone = document.createElement('div'); + store.add(instantiationService.createInstance(AgentHostChatInputPicker, widget, 'customChoice')).render(standalone); + const notify = () => notifications.fire({ + type: NotificationType.SessionAdded, + channel: 'ahp-root://', + summary: new class extends mock() { override readonly resource = backendSession.toString(); }(), + }); + if (notificationFirst) { + notify(); + } + initial.error(new Error('Session has not been created yet')); + await timeout(0); + if (!notificationFirst) { + notify(); + } + await timeout(0); + + assert.deepStrictEqual({ + subscribeCalls, + buttons: [...container.querySelectorAll('[role="button"]')].map(element => element.getAttribute('aria-label')), + standaloneButton: standalone.querySelector('[role="button"]')?.getAttribute('aria-label'), + }, { + subscribeCalls: 2, + buttons: ['Custom Choice: First Option', 'Toggle: Off'], + standaloneButton: 'Custom Choice: First Option', + }); + }); + } + + test('retries only once per matching creation notification and stops listening on disposal', async () => { + const notifications = store.add(new Emitter()); + const errors = store.add(new Emitter()); + const subscription: IAgentSubscription = { + ...createSubscription(), + value: new Error('Session not available'), + onDidError: errors.event, + }; + const connection = new class extends mock() { + override readonly onDidNotification = notifications.event; + }(); + let retries = 0; + const listener = store.add(retrySessionConfigSubscriptionOnCreation(connection, backendSession, subscription, () => retries++)); + const notify = (resource: string) => notifications.fire({ + type: NotificationType.SessionAdded, + channel: 'ahp-root://', + summary: new class extends mock() { override readonly resource = resource; }(), + }); + errors.fire(new Error('Still unavailable')); + notify('ahp-session:/unrelated'); + const unrelatedRetries = retries; + notify(backendSession.toString()); + await timeout(0); + errors.fire(new Error('Retry also failed')); + await timeout(0); + const failedRetryCount = retries; + notify(backendSession.toString()); + listener.dispose(); + notify(backendSession.toString()); + await timeout(0); + assert.deepStrictEqual({ unrelatedRetries, failedRetryCount, afterDisposal: retries }, { + unrelatedRetries: 0, failedRetryCount: 1, afterDisposal: 1, + }); + }); + test('keeps an open picker when another connection changes', async () => { const { open, actionWidget, connectionsChanged, host } = setup(); await open(); @@ -400,9 +502,12 @@ suite('AgentHostGenericConfigChips - remote sessions', () => { } else { host.update({ ...config, - schema: { ...config.schema, properties: { ...config.schema.properties, - customChoice: { ...config.schema.properties.customChoice, [change]: change === 'readOnly' }, - } }, + schema: { + ...config.schema, properties: { + ...config.schema.properties, + customChoice: { ...config.schema.properties.customChoice, [change]: change === 'readOnly' }, + } + }, }); } await assert.rejects(select('Second Option'), isCancellationError); @@ -423,6 +528,47 @@ suite('AgentHostGenericConfigChips - remote sessions', () => { }); }); + test('uses schema options when a dynamic lookup fails', async () => { + const config = makeConfig(); + config.schema.properties.customChoice.enumDynamic = true; + const { open, host, actionWidget, warnings } = setup(config); + const pending = new DeferredPromise(); + store.add({ dispose: () => pending.cancel() }); + host.connection.completionResult = pending.p; + const opened = open(); + pending.error(new Error('Provider unavailable')); + await opened; + await actionWidget.select('Second Option'); + + assert.deepStrictEqual({ + labels: actionWidget.labels, + writes: host.connection.dispatches, + warnings, + }, { + labels: ['First Option', 'Second Option'], + writes: [{ channel: backendSession.toString(), config: { customChoice: 'second' } }], + warnings: ['[AgentHostChatInputPicker] Failed to load dynamic session configuration options; using schema options.'], + }); + }); + + for (const cancellation of ['provider cancellation', 'session switch'] as const) { + test(`does not show schema options after ${cancellation}`, async () => { + const config = makeConfig(); + config.schema.properties.customChoice.enumDynamic = true; + const { trigger, host, actionWidget, switchSession, warnings } = setup(config); + const pending = new DeferredPromise(); + store.add({ dispose: () => pending.cancel() }); + host.connection.completionResult = pending.p; + trigger('customChoice').click(); + if (cancellation === 'session switch') { + switchSession(); + } + pending.error(cancellation === 'provider cancellation' ? new CancellationError() : new Error('Provider unavailable')); + await timeout(0); + assert.deepStrictEqual({ shown: actionWidget.showCount, warnings }, { shown: 0, warnings: [] }); + }); + } + test('does not show late completions after switching to a host with the same backend session URI', async () => { const config = makeConfig(); config.schema.properties.customChoice.enumDynamic = true; @@ -447,9 +593,12 @@ suite('AgentHostGenericConfigChips - remote sessions', () => { trigger('customChoice').click(); host.update({ ...config, - schema: { ...config.schema, properties: { ...config.schema.properties, - customChoice: { ...config.schema.properties.customChoice, readOnly: true }, - } }, + schema: { + ...config.schema, properties: { + ...config.schema.properties, + customChoice: { ...config.schema.properties.customChoice, readOnly: true }, + } + }, }); pending.complete({ items: [{ value: 'second', label: 'Obsolete Option' }] }); await timeout(0); From d98157b442cf20454124ab68f95ceea3d9518bf4 Mon Sep 17 00:00:00 2001 From: Osvaldo Ortega Date: Thu, 10 Sep 2026 18:07:31 -0700 Subject: [PATCH 3/4] Fix agent host connection services in chat fixtures Register the connection resolver with inert local and remote fixture services so chat input fixtures can construct the generic configuration controls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/componentFixtures/chat/chatFixtureUtils.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/vs/workbench/test/browser/componentFixtures/chat/chatFixtureUtils.ts b/src/vs/workbench/test/browser/componentFixtures/chat/chatFixtureUtils.ts index e40e2c57c8c9dd..c242292d4c49ed 100644 --- a/src/vs/workbench/test/browser/componentFixtures/chat/chatFixtureUtils.ts +++ b/src/vs/workbench/test/browser/componentFixtures/chat/chatFixtureUtils.ts @@ -40,6 +40,9 @@ import { IViewDescriptorService } from '../../../../common/views.js'; import { ISCMService } from '../../../../contrib/scm/common/scm.js'; import { IBrowserViewWorkbenchService } from '../../../../contrib/browserView/common/browserView.js'; import { IAgentHostService } from '../../../../../platform/agentHost/common/agentService.js'; +import { AgentHostConnectionsService } from '../../../../../platform/agentHost/browser/agentHostConnectionsService.js'; +import { IAgentHostConnectionsService } from '../../../../../platform/agentHost/common/agentHostConnectionsService.js'; +import { IRemoteAgentHostService, NullRemoteAgentHostService } from '../../../../../platform/agentHost/common/remoteAgentHostService.js'; import { IAgentHostEnablementService } from '../../../../../platform/agentHost/common/agentHostEnablementService.js'; import { IAgentSubscription } from '../../../../../platform/agentHost/common/state/agentSubscription.js'; import { ResolveSessionConfigResult } from '../../../../../platform/agentHost/common/state/protocol/commands.js'; @@ -363,6 +366,8 @@ export function registerChatFixtureServices(reg: ServiceRegistration, options: I // render and nothing crashes. reg.defineInstance(IAgentHostService, new class extends mock() { override readonly onAgentHostStart = Event.None; + override readonly onAgentHostExit = Event.None; + override readonly onDidNotification = Event.None; override readonly rootState: IAgentSubscription = { value: undefined, verifiedValue: undefined, @@ -389,6 +394,8 @@ export function registerChatFixtureServices(reg: ServiceRegistration, options: I return options.agentHostSessionConfig ?? { schema: { type: 'object', properties: {} }, values: {} }; } }()); + reg.defineInstance(IRemoteAgentHostService, new NullRemoteAgentHostService()); + reg.define(IAgentHostConnectionsService, AgentHostConnectionsService); reg.defineInstance(IAgentHostUntitledProvisionalSessionService, new class extends mock() { override readonly onDidChange = Event.None; override get() { return undefined; } From 717596112c3500fcb32b89d3c45d4d744f242521 Mon Sep 17 00:00:00 2001 From: Osvaldo Ortega Date: Fri, 11 Sep 2026 12:43:23 -0700 Subject: [PATCH 4/4] Fix agent host connection services in inline chat fixtures Supply the connection resolver and inert host events needed by generic configuration controls so all inline chat fixtures render successfully. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../editor/inlineChatZoneWidget.fixture.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/vs/workbench/test/browser/componentFixtures/editor/inlineChatZoneWidget.fixture.ts b/src/vs/workbench/test/browser/componentFixtures/editor/inlineChatZoneWidget.fixture.ts index ef3128df68cb28..64cb60291ec924 100644 --- a/src/vs/workbench/test/browser/componentFixtures/editor/inlineChatZoneWidget.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/editor/inlineChatZoneWidget.fixture.ts @@ -55,6 +55,9 @@ import { IChatWidgetHistoryService } from '../../../../contrib/chat/common/widge import { IChatLayoutService } from '../../../../contrib/chat/common/widget/chatLayoutService.js'; import { IAgentSessionsService } from '../../../../contrib/chat/browser/agentSessions/agentSessionsService.js'; import { IAgentHostService } from '../../../../../platform/agentHost/common/agentService.js'; +import { AgentHostConnectionsService } from '../../../../../platform/agentHost/browser/agentHostConnectionsService.js'; +import { IAgentHostConnectionsService } from '../../../../../platform/agentHost/common/agentHostConnectionsService.js'; +import { IRemoteAgentHostService, NullRemoteAgentHostService } from '../../../../../platform/agentHost/common/remoteAgentHostService.js'; import { IAgentSubscription } from '../../../../../platform/agentHost/common/state/agentSubscription.js'; import { RootState } from '../../../../../platform/agentHost/common/state/sessionState.js'; import { IAgentHostUntitledProvisionalSessionService } from '../../../../contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.js'; @@ -286,6 +289,8 @@ function renderInlineChatZoneWidget({ container, disposableStore, theme, focus } }()); reg.defineInstance(IAgentHostService, new class extends mock() { override readonly onAgentHostStart = Event.None; + override readonly onAgentHostExit = Event.None; + override readonly onDidNotification = Event.None; override readonly rootState: IAgentSubscription = { value: undefined, verifiedValue: undefined, @@ -294,6 +299,8 @@ function renderInlineChatZoneWidget({ container, disposableStore, theme, focus } onDidApplyAction: Event.None, }; }()); + reg.defineInstance(IRemoteAgentHostService, new NullRemoteAgentHostService()); + reg.define(IAgentHostConnectionsService, AgentHostConnectionsService); reg.defineInstance(IAgentHostUntitledProvisionalSessionService, new class extends mock() { override readonly onDidChange = Event.None; override get() { return undefined; }