diff --git a/src/vs/sessions/contrib/automations/browser/automationDialog.ts b/src/vs/sessions/contrib/automations/browser/automationDialog.ts index ab45fa0977f..5a6a9819a37 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialog.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialog.ts @@ -15,7 +15,7 @@ import { Codicon } from '../../../../base/common/codicons.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { KeyCode } from '../../../../base/common/keyCodes.js'; import { DisposableStore, IDisposable, MutableDisposable } from '../../../../base/common/lifecycle.js'; -import { autorun } from '../../../../base/common/observable.js'; +import { autorun, constObservable, observableValue } from '../../../../base/common/observable.js'; import { URI } from '../../../../base/common/uri.js'; import { ICodeEditorService } from '../../../../editor/browser/services/codeEditorService.js'; import { EditorContextKeys } from '../../../../editor/common/editorContextKeys.js'; @@ -35,16 +35,16 @@ import { IProductService } from '../../../../platform/product/common/productServ import { defaultCheckboxStyles, defaultInputBoxStyles, defaultSelectBoxStyles } from '../../../../platform/theme/browser/defaultStyles.js'; import { hasNativeContextMenu } from '../../../../platform/window/common/window.js'; import { WorkspacePicker } from '../../chat/browser/sessionWorkspacePicker.js'; -import { ISessionWorkspaceBrowseAction, SESSION_WORKSPACE_GROUP_LOCAL } from '../../../services/sessions/common/session.js'; +import { MobileSessionTypePicker } from '../../chat/browser/mobile/mobileSessionTypePicker.js'; +import { ISession, ISessionWorkspaceBrowseAction, SESSION_WORKSPACE_GROUP_LOCAL } from '../../../services/sessions/common/session.js'; import { IGitService } from '../../../../workbench/contrib/git/common/gitService.js'; import { AutomationInterval } from '../../../../workbench/contrib/chat/common/automations/automation.js'; import { IShowAutomationDialogOptions } from '../../../../workbench/contrib/chat/common/automations/automationDialogService.js'; -import { IAutomationSessionTypeChoice, IAutomationSessionTypeProvider } from '../../../../workbench/contrib/chat/common/automations/automationSessionTypes.js'; import { DAYS_OF_WEEK } from '../../../../workbench/contrib/chat/common/automations/schedule.js'; import { ChatContextKeys } from '../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; import { ILanguageModelsService } from '../../../../workbench/contrib/chat/common/languageModels.js'; import { ChatAgentLocation, isChatPermissionLevel } from '../../../../workbench/contrib/chat/common/constants.js'; -import { AgentSessionProviders, AgentSessionTarget } from '../../../../workbench/contrib/chat/browser/agentSessions/agentSessions.js'; +import { AgentSessionTarget } from '../../../../workbench/contrib/chat/browser/agentSessions/agentSessions.js'; import { IChatWidget, ISessionTypePickerDelegate } from '../../../../workbench/contrib/chat/browser/chat.js'; import { ChatInputPart, IChatInputPartOptions, IChatInputStyles } from '../../../../workbench/contrib/chat/browser/widget/input/chatInputPart.js'; import { isModeConsideredBuiltIn } from '../../../../workbench/contrib/chat/browser/widget/input/modePickerActionItem.js'; @@ -92,28 +92,10 @@ interface IRenderFormHandle { readonly getModelId: () => string | undefined; } + const AUTOMATIONS_HARNESS_CHIP_ACTION_ID = 'workbench.action.chat.renderAutomationsHarnessChip'; const AUTOMATIONS_ISOLATION_GROUP_ACTION_ID = 'workbench.action.chat.renderAutomationsIsolationGroup'; -function createAutomationHarnessChip(): HTMLElement { - const harnessChip = $('span.automation-form-harness-chip'); - DOM.append(harnessChip, renderIcon(Codicon.copilot)); - DOM.append(harnessChip, $('span.automation-form-harness-label', undefined, localize('automation.form.harness', "Copilot CLI"))); - return harnessChip; -} - -class AutomationHarnessChipActionViewItem extends BaseActionViewItem { - constructor(action: IAction, options?: IBaseActionViewItemOptions) { - super(undefined, action, options); - } - - override render(container: HTMLElement): void { - super.render(container); - DOM.clearNode(container); - DOM.append(container, createAutomationHarnessChip()); - } -} - class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { private readonly renderDisposables = this._register(new DisposableStore()); private readonly branchRepoDisposable = this._register(new MutableDisposable()); @@ -280,6 +262,28 @@ class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { } } +/** + * Hosts the shared {@link SessionTypePicker} inside the chat input's secondary + * toolbar, in the slot previously occupied by the hardcoded harness chip. The + * picker instance is owned by the dialog (registered on its disposables); this + * view item only renders it into the toolbar container. + */ +class AutomationSessionTypePickerActionViewItem extends BaseActionViewItem { + constructor( + action: IAction, + private readonly picker: MobileSessionTypePicker, + options?: IBaseActionViewItemOptions, + ) { + super(undefined, action, options); + } + + override render(container: HTMLElement): void { + super.render(container); + DOM.clearNode(container); + this.picker.render(container); + } +} + registerAction2(class OpenAutomationsHarnessChipAction extends Action2 { constructor() { super({ @@ -318,83 +322,6 @@ registerAction2(class OpenAutomationsIsolationGroupAction extends Action2 { override async run(): Promise { /* handled by action view item */ } }); -/** - * Two-way binding between the chat input's session-target chip and the form's - * providerId + sessionTypeId fields. - */ -function createSessionTypeBinder( - state: IFormState, - sessionTypeProvider: IAutomationSessionTypeProvider, - disposables: DisposableStore, -): ISessionTypePickerDelegate & { setFolder(folder: URI | undefined): void } { - const onDidChange = disposables.add(new Emitter()); - - const pickDefault = (available: readonly IAutomationSessionTypeChoice[]): IAutomationSessionTypeChoice | undefined => { - if (available.length === 0) { - return undefined; - } - return available.find(c => c.sessionTypeId === AgentSessionProviders.Background) - ?? available[0]; - }; - - const validateOrDefault = (folder: URI | undefined): void => { - if (!folder) { - state.providerId = undefined; - state.sessionTypeId = undefined; - return; - } - const available = sessionTypeProvider.getSessionTypesForFolder(folder); - if (state.providerId && state.sessionTypeId) { - const match = available.find(c => c.providerId === state.providerId && c.sessionTypeId === state.sessionTypeId); - if (match) { - return; - } - } - const def = pickDefault(available); - state.providerId = def?.providerId; - state.sessionTypeId = def?.sessionTypeId; - }; - - validateOrDefault(state.folderUri); - - return { - getActiveSessionProvider: () => state.sessionTypeId as AgentSessionTarget | undefined, - setActiveSessionProvider: (target: AgentSessionTarget) => { - // Safe against folder-change races: we read the current folderUri and - // validate target against it. If the folder changed while the picker was - // open, the old target won't match the new folder's available types. - if (!state.folderUri) { - return; - } - const available = sessionTypeProvider.getSessionTypesForFolder(state.folderUri); - const match = available.find(c => c.sessionTypeId === target); - if (!match) { - return; - } - state.providerId = match.providerId; - state.sessionTypeId = match.sessionTypeId; - onDidChange.fire(match.sessionTypeId as AgentSessionTarget); - }, - onDidChangeActiveSessionProvider: onDidChange.event, - setFolder: (folder: URI | undefined) => { - validateOrDefault(folder); - if (state.sessionTypeId) { - onDidChange.fire(state.sessionTypeId as AgentSessionTarget); - } - }, - isSessionTypeVisible: (type: AgentSessionTarget) => { - if (type !== AgentSessionProviders.Background) { - return false; - } - if (!state.folderUri) { - return true; - } - const available = sessionTypeProvider.getSessionTypesForFolder(state.folderUri); - return available.some(c => c.sessionTypeId === type); - }, - }; -} - export function renderForm( form: HTMLElement, state: IFormState, @@ -409,7 +336,6 @@ export function renderForm( layoutService: ILayoutService, logService: ILogService, productService: IProductService, - sessionTypeProvider: IAutomationSessionTypeProvider, initialPrompt: string, initialMode: string | undefined, initialPermissionLevel: string | undefined, @@ -495,7 +421,37 @@ export function renderForm( applyIntervalVisibility(); })); - const sessionTypeBinder = createSessionTypeBinder(state, sessionTypeProvider, disposables); + // The picker is authoritative for the session type + const folderObs = observableValue('automationFolder', state.folderUri); + const sessionTypePicker = disposables.add(instantiationService.createInstance(MobileSessionTypePicker, constObservable(undefined), { persistSelection: false, telemetrySource: 'AutomationSessionTypePicker' })); + sessionTypePicker.setFolderSource(folderObs, { + initialPick: state.providerId && state.sessionTypeId + ? { providerId: state.providerId, sessionTypeId: state.sessionTypeId } + : undefined, + }); + // The dialog has no session, so the input part reads the active session type from the picker via this delegate. + const onDidChangeSessionType = disposables.add(new Emitter()); + const sessionTypeDelegate: ISessionTypePickerDelegate = { + getActiveSessionProvider: () => sessionTypePicker.selectedPick?.sessionTypeId as AgentSessionTarget | undefined, + onDidChangeActiveSessionProvider: onDidChangeSessionType.event, + }; + const syncStateFromPicker = () => { + const pick = sessionTypePicker.selectedPick; + state.providerId = pick?.providerId; + state.sessionTypeId = pick?.sessionTypeId; + if (pick?.sessionTypeId) { + onDidChangeSessionType.fire(pick.sessionTypeId as AgentSessionTarget); + } + }; + // Seed state from the picker's initial default (edit: saved type; create: folder default). + syncStateFromPicker(); + // Covers both explicit user picks and recomputes (e.g. an agent host + // advertising its session types after the dialog opened), so the saved + // automation always matches the chip the picker displays. + disposables.add(sessionTypePicker.onDidChangeSelectedPick(() => { + syncStateFromPicker(); + revalidate(); + })); const workspacePicker = disposables.add(instantiationService.createInstance(AutomationsWorkspacePicker)); @@ -505,13 +461,16 @@ export function renderForm( disposables.add(workspacePicker.onDidSelectWorkspace(uri => { state.folderUri = uri; - sessionTypeBinder.setFolder(uri); + // Setting the folder recomputes the picker's default; onDidChangeSelectedPick + // mirrors any resulting pick change into state. revalidate() still runs here + // because folder validity can change even when the pick does not. + folderObs.set(uri, undefined); revalidate(); })); if (!state.folderUri && workspacePicker.selectedFolderUri) { state.folderUri = workspacePicker.selectedFolderUri; - sessionTypeBinder.setFolder(state.folderUri); + folderObs.set(state.folderUri, undefined); } const promptRow = DOM.append(form, $('.automation-form-row')); @@ -545,11 +504,11 @@ export function renderForm( // reserve the default 24px margin and lay the editor out too narrow, // leaving its scrollbar floating ~24px in from the right wall. inputPartHorizontalPadding: 0, - sessionTypePickerDelegate: sessionTypeBinder, + sessionTypePickerDelegate: sessionTypeDelegate, workspacePickerInput: workspacePicker, secondaryToolbarActionViewItemProvider: (action, itemOptions) => { if (action.id === AUTOMATIONS_HARNESS_CHIP_ACTION_ID) { - return new AutomationHarnessChipActionViewItem(action, itemOptions); + return new AutomationSessionTypePickerActionViewItem(action, sessionTypePicker, itemOptions); } if (action.id === AUTOMATIONS_ISOLATION_GROUP_ACTION_ID) { const actionWidgetService = instantiationService.invokeFunction(accessor => accessor.get(IActionWidgetService)); diff --git a/src/vs/sessions/contrib/automations/browser/automationDialogService.ts b/src/vs/sessions/contrib/automations/browser/automationDialogService.ts index 192f2c5b5e5..af4f0f4d114 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialogService.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialogService.ts @@ -21,13 +21,10 @@ import { createWorkbenchDialogOptions } from '../../../../workbench/browser/part import { IAutomationSchedule } from '../../../../workbench/contrib/chat/common/automations/automation.js'; import { IAutomationDialogResult, IAutomationDialogService, IShowAutomationDialogOptions } from '../../../../workbench/contrib/chat/common/automations/automationDialogService.js'; import { ICreateAutomationOptions, IUpdateAutomationOptions } from '../../../../workbench/contrib/chat/common/automations/automationService.js'; -import { IAutomationSessionTypeProvider } from '../../../../workbench/contrib/chat/common/automations/automationSessionTypes.js'; -import { SessionType } from '../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { IHostService } from '../../../../workbench/services/host/browser/host.js'; import { IFormState, IValidationState, isAutomationDialogPopupTarget, renderForm, updateSaveButtonState } from './automationDialog.js'; const $ = DOM.$; -const COPILOT_PROVIDER_ID = 'default-copilot'; /** * Owns the Automations create/edit dialog in the sessions layer, where the @@ -48,7 +45,6 @@ export class AutomationDialogService implements IAutomationDialogService { @ILogService private readonly logService: ILogService, @IProductService private readonly productService: IProductService, @IHostService private readonly hostService: IHostService, - @IAutomationSessionTypeProvider private readonly sessionTypeProvider: IAutomationSessionTypeProvider, ) { } async showAutomationDialog(options: IShowAutomationDialogOptions): Promise { @@ -122,7 +118,7 @@ export class AutomationDialogService implements IAutomationDialogService { const formPane = DOM.append(container, $('.automation-form-pane')); const form = DOM.append(formPane, $('.automation-form')); - const handle = renderForm(form, state, options, disposables, validation, () => revalidate(), this.instantiationService, this.contextKeyService, this.contextViewService, this.configurationService, this.layoutService, this.logService, this.productService, this.sessionTypeProvider, initial?.prompt ?? '', initial?.mode, initial?.permissionLevel, initial?.modelId); + const handle = renderForm(form, state, options, disposables, validation, () => revalidate(), this.instantiationService, this.contextKeyService, this.contextViewService, this.configurationService, this.layoutService, this.logService, this.productService, initial?.prompt ?? '', initial?.mode, initial?.permissionLevel, initial?.modelId); getPrompt = handle.getPrompt; getMode = handle.getMode; getPermissionLevel = handle.getPermissionLevel; @@ -165,8 +161,8 @@ export class AutomationDialogService implements IAutomationDialogService { prompt, schedule, folderUri: state.folderUri, - providerId: state.providerId ?? COPILOT_PROVIDER_ID, - sessionTypeId: state.sessionTypeId ?? SessionType.CopilotCLI, + providerId: state.providerId ?? null, + sessionTypeId: state.sessionTypeId ?? null, modelId: modelId ?? null, mode: mode ?? null, permissionLevel: permissionLevel ?? null, @@ -182,8 +178,8 @@ export class AutomationDialogService implements IAutomationDialogService { prompt, schedule, folderUri: state.folderUri, - providerId: state.providerId ?? COPILOT_PROVIDER_ID, - sessionTypeId: state.sessionTypeId ?? SessionType.CopilotCLI, + providerId: state.providerId, + sessionTypeId: state.sessionTypeId, modelId, mode, permissionLevel, diff --git a/src/vs/sessions/contrib/chat/browser/mobile/mobileSessionTypePicker.ts b/src/vs/sessions/contrib/chat/browser/mobile/mobileSessionTypePicker.ts index 829058e53ed..2a879c1b6ca 100644 --- a/src/vs/sessions/contrib/chat/browser/mobile/mobileSessionTypePicker.ts +++ b/src/vs/sessions/contrib/chat/browser/mobile/mobileSessionTypePicker.ts @@ -17,7 +17,7 @@ import { ISessionsProvidersService } from '../../../../services/sessions/browser import { ISession } from '../../../../services/sessions/common/session.js'; import { IObservable } from '../../../../../base/common/observable.js'; import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js'; -import { SessionTypePicker } from '../sessionTypePicker.js'; +import { SessionTypePicker, ISessionTypePickerOptions } from '../sessionTypePicker.js'; import { isPhoneLayout } from '../../../../browser/parts/mobile/mobileLayout.js'; import { IMobilePickerSheetItem, showMobilePickerSheet } from '../../../../browser/parts/mobile/mobilePickerSheet.js'; @@ -36,6 +36,7 @@ export class MobileSessionTypePicker extends SessionTypePicker { constructor( session: IObservable, + options: ISessionTypePickerOptions | undefined, @IActionWidgetService actionWidgetService: IActionWidgetService, @ISessionsManagementService sessionsManagementService: ISessionsManagementService, @ISessionsProvidersService private readonly _sessionsProvidersService: ISessionsProvidersService, @@ -47,7 +48,7 @@ export class MobileSessionTypePicker extends SessionTypePicker { @IWorkbenchLayoutService private readonly layoutService: IWorkbenchLayoutService, @IContextKeyService contextKeyService: IContextKeyService, ) { - super(session, actionWidgetService, sessionsManagementService, _sessionsProvidersService, storageService, telemetryService, chatSessionsService, chatEntitlementService, languageModelsService, contextKeyService); + super(session, options, actionWidgetService, sessionsManagementService, _sessionsProvidersService, storageService, telemetryService, chatSessionsService, chatEntitlementService, languageModelsService, contextKeyService); } override render(container: HTMLElement, options?: { className?: string }): void { diff --git a/src/vs/sessions/contrib/chat/browser/newChatInput.ts b/src/vs/sessions/contrib/chat/browser/newChatInput.ts index f9589620657..19ec49aee26 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatInput.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatInput.ts @@ -337,7 +337,7 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation // the same class regardless of construction-time viewport // avoids a class-mismatch when the user resizes across the // phone breakpoint after the chat input mounted. - this.sessionTypePicker = this._register(this.instantiationService.createInstance(MobileSessionTypePicker, this.options.session)); + this.sessionTypePicker = this._register(this.instantiationService.createInstance(MobileSessionTypePicker, this.options.session, undefined)); this._register(this._contextAttachments.onDidChangeContext(() => { this._updateDraftState(); this._updateSendButtonState(); diff --git a/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts b/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts index 1d0e8eaced4..8661a7b3c1b 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts @@ -53,11 +53,36 @@ export interface IPreferredSessionType { readonly sessionTypeId: string; } +function pickEquals(a: IPreferredSessionType | undefined, b: IPreferredSessionType | undefined): boolean { + return a?.providerId === b?.providerId && a?.sessionTypeId === b?.sessionTypeId; +} + interface IStoredSessionTypePick { readonly providerId?: string; readonly sessionTypeId: string; } +/** Default telemetry source used when the picker serves the New Session composer. */ +const DEFAULT_TELEMETRY_SOURCE = 'NewChatSessionTypePicker'; + +/** + * Configures how the picker behaves when reused outside the New Session + * composer (e.g. the automations dialog), where profile-wide persistence and + * new-chat telemetry would be incorrect side effects. + */ +export interface ISessionTypePickerOptions { + /** + * When `false` (used e.g. by the automations dialog), an explicit pick is + * never written to or cleared from the profile-wide + * {@link STORAGE_KEY_LAST_SESSION_TYPE} preference, so picking a type here + * cannot change the New Session default. The stored preference is still read + * to seed a sensible initial default. Defaults to `true`. + */ + readonly persistSelection?: boolean; + /** Telemetry id/name reported on selection. Defaults to {@link DEFAULT_TELEMETRY_SOURCE}. */ + readonly telemetrySource?: string; +} + /** * Row item rendered inside the session type picker — carries both the * provider id and the session type so we can dispatch creation through @@ -89,6 +114,16 @@ export class SessionTypePicker extends Disposable { protected readonly _onDidSelectSessionType = this._register(new Emitter()); readonly onDidSelectSessionType = this._onDidSelectSessionType.event; + /** + * Fires whenever the effective {@link selectedPick} changes for any reason: + * an explicit user pick OR a recompute (e.g. a provider advertising its + * session types late). Unlike {@link onDidSelectSessionType}, which only + * covers explicit picks, this lets consumers that cache the pick stay in + * sync when the displayed default shifts on its own. + */ + protected readonly _onDidChangeSelectedPick = this._register(new Emitter()); + readonly onDidChangeSelectedPick = this._onDidChangeSelectedPick.event; + /** Session types the active session's folder can be served by, across all providers. */ protected _folderSessionTypes: IProviderSessionType[] = []; @@ -109,6 +144,7 @@ export class SessionTypePicker extends Disposable { constructor( private readonly _session: IObservable, + private readonly _options: ISessionTypePickerOptions | undefined, @IActionWidgetService private readonly actionWidgetService: IActionWidgetService, @ISessionsManagementService private readonly sessionsManagementService: ISessionsManagementService, @ISessionsProvidersService private readonly sessionsProvidersService: ISessionsProvidersService, @@ -143,8 +179,12 @@ export class SessionTypePicker extends Disposable { */ protected _recompute(): void { this._folderSessionTypes = this._resolveFolderSessionTypes(); + const previous = this._picked; this._picked = this._computeCurrentPick(); this._updateTriggerLabel(); + if (!pickEquals(previous, this._picked)) { + this._onDidChangeSelectedPick.fire(this._picked); + } } /** @@ -405,9 +445,10 @@ export class SessionTypePicker extends Disposable { const beforeLabel = this._folderSessionTypes.find(t => t.sessionType.id === beforeId)?.sessionType.label; const afterLabel = this._folderSessionTypes.find(t => t.providerId === pick.providerId && t.sessionType.id === pick.sessionTypeId)?.sessionType.label; + const telemetrySource = this._options?.telemetrySource ?? DEFAULT_TELEMETRY_SOURCE; reportNewChatPickerClosed(this.telemetryService, { - id: 'NewChatSessionTypePicker', - name: 'NewChatSessionTypePicker', + id: telemetrySource, + name: telemetrySource, optionIdBefore: beforeId, optionIdAfter: pick.sessionTypeId, optionLabelBefore: beforeLabel, @@ -424,10 +465,15 @@ export class SessionTypePicker extends Disposable { const preferred = this._folderSessionTypes[0]; const isDefault = !!preferred && preferred.providerId === pick.providerId && preferred.sessionType.id === pick.sessionTypeId; const visiblePickChanged = pick.providerId !== this._picked?.providerId || pick.sessionTypeId !== this._picked?.sessionTypeId; - if (isDefault) { - this._clearStoredPick(pick); - } else { - this._writeStoredPick(pick); + // profile-wide preference is gated so non-persisting callers (e.g. the + // automations dialog) can pick a type without changing the New Session default + this._picked = pick; + if (this._options?.persistSelection !== false) { + if (isDefault) { + this._clearStoredPick(); + } else { + this._writeStoredPick(pick); + } } // Folder-driven callers have no session change to re-run the refresh autorun, so refresh the label here. this._updateTriggerLabel(); @@ -435,6 +481,7 @@ export class SessionTypePicker extends Disposable { // actually changed, to avoid unnecessary work. if (visiblePickChanged) { this._onDidSelectSessionType.fire(pick); + this._onDidChangeSelectedPick.fire(this._picked); } } @@ -461,7 +508,6 @@ export class SessionTypePicker extends Disposable { } private _writeStoredPick(pick: IPickedSessionType): void { - this._picked = pick; const stored: IStoredSessionTypePick = { providerId: pick.providerId, sessionTypeId: pick.sessionTypeId }; this.storageService.store(STORAGE_KEY_LAST_SESSION_TYPE, JSON.stringify(stored), StorageScope.PROFILE, StorageTarget.MACHINE); } @@ -471,8 +517,7 @@ export class SessionTypePicker extends Disposable { * type). The display still reflects the in-memory pick, but consumers * reading {@link getUserPickedSessionType} fall back to the preferred type. */ - private _clearStoredPick(pick: IPickedSessionType): void { - this._picked = pick; + private _clearStoredPick(): void { this.storageService.remove(STORAGE_KEY_LAST_SESSION_TYPE, StorageScope.PROFILE); } diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts index 2f63eeddc04..13d6b991561 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts @@ -24,7 +24,7 @@ import { TestStorageService } from '../../../../../workbench/test/common/workben import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; import { IProviderSessionType, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { ISession, ISessionWorkspace } from '../../../../services/sessions/common/session.js'; -import { IPickedSessionType, IPreferredSessionType, SessionTypePicker } from '../../browser/sessionTypePicker.js'; +import { IPickedSessionType, IPreferredSessionType, ISessionTypePickerOptions, SessionTypePicker } from '../../browser/sessionTypePicker.js'; // ---- Mocks ------------------------------------------------------------------ @@ -109,6 +109,7 @@ function createPicker( session: ISettableObservable, managementService: MockSessionsManagementService, storage: IStorageService, + options?: ISessionTypePickerOptions, ): TestSessionTypePicker { const instantiationService = disposables.add(new TestInstantiationService()); instantiationService.stub(IActionWidgetService, { isVisible: false, hide: () => { }, show: () => { } }); @@ -127,7 +128,7 @@ function createPicker( lookupLanguageModel: () => undefined, }); instantiationService.stub(IContextKeyService, new MockContextKeyService()); - return disposables.add(instantiationService.createInstance(TestSessionTypePicker, session)); + return disposables.add(instantiationService.createInstance(TestSessionTypePicker, session, options)); } // ---- Tests ------------------------------------------------------------------ @@ -252,6 +253,57 @@ suite('SessionTypePicker', () => { assert.strictEqual(picker.getUserPickedSessionType(), undefined); }); + test('persistSelection false never mutates the shared New Session preference', () => { + management.setSessionTypes([ + sessionType('local-1', 'local', 'Local'), + sessionType('copilot', 'copilot-cli', 'Copilot CLI'), + sessionType('anthropic', 'claude', 'Claude'), + ]); + + // The New Session composer stored an explicit, non-default preference. + const shared = createPicker(disposables, session, management, storage); + shared.pick({ providerId: 'copilot', sessionTypeId: 'copilot-cli' }); + assert.deepStrictEqual(shared.getUserPickedSessionType(), { providerId: 'copilot', sessionTypeId: 'copilot-cli' }); + + // The automations dialog picker still reads that stored preference to seed + // a sensible default, but must never write or clear it. + const scopedSession = observableValue('scoped', undefined); + const scoped = createPicker(disposables, scopedSession, management, storage, { persistSelection: false }); + assert.deepStrictEqual(scoped.getUserPickedSessionType(), { providerId: 'copilot', sessionTypeId: 'copilot-cli' }); + // Give the scoped picker a folder so 'local' is its default type. + scopedSession.set(createFakeSession('local-1', 'local', folder), undefined); + + // A different non-default pick would normally be written — it must not be. + scoped.pick({ providerId: 'anthropic', sessionTypeId: 'claude' }); + assert.deepStrictEqual(shared.getUserPickedSessionType(), { providerId: 'copilot', sessionTypeId: 'copilot-cli' }); + + // Picking the default type would normally clear the stored pick — it must not. + scoped.pick({ providerId: 'local-1', sessionTypeId: 'local' }); + assert.deepStrictEqual(shared.getUserPickedSessionType(), { providerId: 'copilot', sessionTypeId: 'copilot-cli' }); + }); + + test('onDidChangeSelectedPick fires when session types are advertised after the picker is created', () => { + // No types advertised yet (e.g. the agent host has not connected). + management.setSessionTypes([]); + const picker = createPicker(disposables, session, management, storage); + const folderObs = observableValue('folder', folder); + picker.setFolderSource(folderObs); + assert.strictEqual(picker.selectedPick, undefined); + + const fired: (IPreferredSessionType | undefined)[] = []; + disposables.add(picker.onDidChangeSelectedPick(pick => fired.push(pick))); + + // A provider advertises its types late; the displayed default shifts on its + // own (no explicit user pick), and consumers that cache the pick are notified. + management.setSessionTypes([ + sessionType('local-1', 'local', 'Local'), + sessionType('copilot', 'copilot-cli', 'Copilot CLI'), + ]); + + assert.deepStrictEqual(picker.selectedPick, { providerId: 'local-1', sessionTypeId: 'local' }); + assert.deepStrictEqual(fired, [{ providerId: 'local-1', sessionTypeId: 'local' }]); + }); + test('a quick chat sources its types from the quick-chat list, not the folder list', () => { // Folder list is empty (workspace-less); quick-chat list drives defaults. management.setSessionTypes([]);