diff --git a/src/vs/sessions/contrib/chat/browser/newChatWidget.ts b/src/vs/sessions/contrib/chat/browser/newChatWidget.ts index 18c4f69c919..89efa459c8a 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatWidget.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatWidget.ts @@ -22,7 +22,7 @@ import { IUriIdentityService } from '../../../../platform/uriIdentity/common/uri import { IDefaultAccountService } from '../../../../platform/defaultAccount/common/defaultAccount.js'; import { localize } from '../../../../nls.js'; import { IActiveSession, ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js'; -import { ISession, SESSION_WORKSPACE_GROUP_GITHUB, SessionTypeAuthRequirement } from '../../../services/sessions/common/session.js'; +import { ISession, SESSION_WORKSPACE_GROUP_GITHUB } from '../../../services/sessions/common/session.js'; import { IOpenNewSessionResult, ISessionsService } from '../../../services/sessions/browser/sessionsService.js'; import { isAllowSignedOutWhenUsableEnabled, shouldShowGitHubWorkspaceGroupSignIn } from '../../../browser/sessionsAuthGate.js'; import { AGENTIC_SIGN_IN_COMMAND_ID } from '../../../common/sessionCommands.js'; @@ -603,19 +603,12 @@ export class NewChatWidget extends Disposable { const preferredPick = userPick && this._isPreferredServable(folderUri, userPick) ? userPick : this._newChatInput.sessionTypePicker.getPreferredSessionType(folderUri); - // A signed-out user (under the conditional-auth opt-in) can't run a type - // that requires GitHub, so default to the first offered type usable - // without it. No-op when signed in or the opt-in is off — today's behavior. - // TODO: reconsider silently switching away from the remembered selection; - // instead keep it and surface an inline "sign in for this type" affordance - // for GitHub-only types. - const effectivePick = this._preferUsableSessionTypeWhenSignedOut(folderUri, preferredPick); const fallbackProviderId = this._workspacePicker.selectedResolved?.providerId; try { return await this.sessionsService.openNewSession({ folderUri, - ...(effectivePick - ? { providerId: effectivePick.providerId, sessionTypeId: effectivePick.sessionTypeId } + ...(preferredPick + ? { providerId: preferredPick.providerId, sessionTypeId: preferredPick.sessionTypeId } : fallbackProviderId ? { providerId: fallbackProviderId } : undefined), @@ -626,29 +619,6 @@ export class NewChatWidget extends Disposable { } } - /** - * While the user is signed out and the conditional-auth opt-in is on, replace - * a pick that requires GitHub with the first offered session type usable - * without it. A no-op when signed in, when the opt-in is off (today's - * behavior), or when no offered type is usable — in which case the caller's - * existing fallbacks still apply. - */ - private _preferUsableSessionTypeWhenSignedOut(folderUri: URI, pick: IPreferredSessionType | undefined): IPreferredSessionType | undefined { - if (this.defaultAccountService.currentDefaultAccount !== null || !isAllowSignedOutWhenUsableEnabled(this.configurationService)) { - return pick; - } - const usable = this.sessionsManagementService.getSessionTypesForFolder(folderUri) - .filter(type => type.sessionType.authRequirement === SessionTypeAuthRequirement.None); - // Match on provider too when the pick names one: two providers can offer - // the same session type id, and only one of them may be usable. - const pickIsUsable = usable.some(type => type.sessionType.id === pick?.sessionTypeId - && (pick?.providerId === undefined || type.providerId === pick.providerId)); - if (usable.length === 0 || pickIsUsable) { - return pick; - } - return { providerId: usable[0].providerId, sessionTypeId: usable[0].sessionType.id }; - } - private _scheduleRecreateOnProviderChange(folderUri: URI, userPick: IPreferredSessionType | undefined, created: ISession | undefined, replayMissedChange: boolean): void { const store = new DisposableStore(); store.add(this.sessionsManagementService.onDidChangeSessionTypes(() => this._recreateOnProviderChange(folderUri, userPick, created))); @@ -668,6 +638,12 @@ export class NewChatWidget extends Disposable { if (!this._isPreferredServable(folderUri, userPick)) { return; // the preferred provider still cannot serve the folder } + // Already running the pick: nothing left to upgrade to, so stop watching. + if (userPick.sessionTypeId === active.sessionType + && (userPick.providerId === undefined || userPick.providerId === active.providerId)) { + this._pendingPreferredUpgrade.clear(); + return; + } } else { // No explicit pick: keep the draft on the preferred (first) // type. Recreate only when that preferred actually changed. diff --git a/src/vs/sessions/contrib/chat/test/browser/newChatWidget.test.ts b/src/vs/sessions/contrib/chat/test/browser/newChatWidget.test.ts index b2723b59166..414ea9c6d1d 100644 --- a/src/vs/sessions/contrib/chat/test/browser/newChatWidget.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/newChatWidget.test.ts @@ -7,31 +7,59 @@ import assert from 'assert'; import { DeferredPromise } from '../../../../../base/common/async.js'; import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { Emitter, Event } from '../../../../../base/common/event.js'; -import { IDisposable, MutableDisposable } from '../../../../../base/common/lifecycle.js'; -import { IObservable, observableValue } from '../../../../../base/common/observable.js'; +import { IDisposable, MutableDisposable, toDisposable } from '../../../../../base/common/lifecycle.js'; +import { constObservable, IObservable, observableValue } from '../../../../../base/common/observable.js'; import { extUri } from '../../../../../base/common/resources.js'; import { URI } from '../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; -import { IActiveSession } from '../../../../services/sessions/common/sessionsManagement.js'; import { ISession } from '../../../../services/sessions/common/session.js'; -import { IOpenNewSessionResult } from '../../../../services/sessions/browser/sessionsService.js'; +import { IOpenNewSessionOptions, IOpenNewSessionResult } from '../../../../services/sessions/browser/sessionsService.js'; import { IPreferredSessionType } from '../../browser/sessionTypePicker.js'; import { NewChatWidget } from '../../browser/newChatWidget.js'; -interface INewChatWidgetHarness { +/** The part of the active session `_recreateOnProviderChange` actually reads. */ +interface IActiveDraft { + readonly sessionId: string; + readonly isCreated: IObservable; + readonly providerId: string; + readonly sessionType: string; +} + +interface IRecreateHarness { readonly _pendingPreferredUpgrade: MutableDisposable; + readonly _session: IObservable; + readonly _newChatInput: { + readonly sessionTypePicker: { + getPreferredSessionType(folderUri: URI): IPreferredSessionType | undefined; + }; + }; + _isPreferredServable(folderUri: URI, pick: IPreferredSessionType): boolean; + _createNewSession(folderUri: URI): Promise; +} + +/** The collaborators `_createSessionNow` reads while assembling the `openNewSession` options. */ +interface ICreateSessionNowHarness { + readonly _newChatInput: { + readonly sessionTypePicker: { + getPreferredSessionType(folderUri: URI): IPreferredSessionType | undefined; + }; + }; + readonly _workspacePicker: { readonly selectedResolved: { readonly providerId: string } | undefined }; + readonly sessionsService: { openNewSession(options: IOpenNewSessionOptions, token: CancellationToken): Promise }; + readonly logService: { error(message: string, ...args: unknown[]): void }; + _isPreferredServable(folderUri: URI, pick: IPreferredSessionType): boolean; +} + +interface INewChatWidgetHarness extends IRecreateHarness { readonly _newSessionCreation: MutableDisposable; readonly sessionsManagementService: { readonly onDidChangeSessionTypes: Event }; - readonly _session: IObservable; readonly _newChatInput: { readonly sessionTypePicker: { getUserPickedSessionType(): IPreferredSessionType | undefined; getPreferredSessionType(folderUri: URI): IPreferredSessionType | undefined; }; }; - _isPreferredServable(folderUri: URI, pick: IPreferredSessionType): boolean; _createSessionNow(folderUri: URI, userPick: IPreferredSessionType | undefined, token: CancellationToken): Promise; - _createNewSession(folderUri: URI): Promise; _scheduleRecreateOnProviderChange(folderUri: URI, userPick: IPreferredSessionType | undefined, created: ISession | undefined, replayMissedChange: boolean): void; _recreateOnProviderChange(folderUri: URI, userPick: IPreferredSessionType | undefined, created: ISession | undefined): void; } @@ -40,8 +68,19 @@ const createNewSession = Reflect.get(NewChatWidget.prototype, '_createNewSession this: INewChatWidgetHarness, folderUri: URI, ) => Promise; +const createSessionNow = Reflect.get(NewChatWidget.prototype, '_createSessionNow') as ( + this: ICreateSessionNowHarness, + folderUri: URI, + userPick: IPreferredSessionType | undefined, + token: CancellationToken, +) => Promise; const scheduleRecreateOnProviderChange = Reflect.get(NewChatWidget.prototype, '_scheduleRecreateOnProviderChange') as INewChatWidgetHarness['_scheduleRecreateOnProviderChange']; -const recreateOnProviderChange = Reflect.get(NewChatWidget.prototype, '_recreateOnProviderChange') as INewChatWidgetHarness['_recreateOnProviderChange']; +const recreateOnProviderChange = Reflect.get(NewChatWidget.prototype, '_recreateOnProviderChange') as ( + this: IRecreateHarness, + folderUri: URI, + userPick: IPreferredSessionType | undefined, + created: { readonly sessionId: string } | undefined, +) => void; const handlePromptOptionsWorkspaceChange = Reflect.get(NewChatWidget.prototype, '_handlePromptOptionsWorkspaceChange') as (this: IPromptOptionsWorkspaceHarness, previousFolderUri: URI | undefined, folderUri: URI | undefined) => void; const hasEnoughSessionsForFirstRunNotices = Reflect.get(NewChatWidget.prototype, '_hasEnoughSessionsForFirstRunNotices') as (this: ISessionCountHarness) => boolean; @@ -59,13 +98,13 @@ function createHarness( pendingPreferredUpgrade: MutableDisposable, newSessionCreation: MutableDisposable, onDidChangeSessionTypes: Event, - createSessionNow: (token: CancellationToken) => Promise, + stubCreateSessionNow: (token: CancellationToken) => Promise, ): INewChatWidgetHarness { const harness: INewChatWidgetHarness = { _pendingPreferredUpgrade: pendingPreferredUpgrade, _newSessionCreation: newSessionCreation, sessionsManagementService: { onDidChangeSessionTypes }, - _session: observableValue('session', undefined), + _session: observableValue('session', undefined), _newChatInput: { sessionTypePicker: { getUserPickedSessionType: () => undefined, @@ -73,7 +112,7 @@ function createHarness( }, }, _isPreferredServable: () => false, - _createSessionNow: (_folderUri, _userPick, token) => createSessionNow(token), + _createSessionNow: (_folderUri, _userPick, token) => stubCreateSessionNow(token), _createNewSession: folderUri => createNewSession.call(harness, folderUri), _scheduleRecreateOnProviderChange: (folderUri, userPick, created, replayMissedChange) => scheduleRecreateOnProviderChange.call(harness, folderUri, userPick, created, replayMissedChange), _recreateOnProviderChange: (folderUri, userPick, created) => recreateOnProviderChange.call(harness, folderUri, userPick, created), @@ -145,6 +184,79 @@ suite('NewChatWidget', () => { assert.deepStrictEqual({ tokenCount: tokens.length, firstCancelledWhenSecondStarted }, { tokenCount: 2, firstCancelledWhenSecondStarted: true }); }); + test('sends the user pick to openNewSession, falling back to the preferred type', async () => { + const folder = URI.file('/project'); + const userPick: IPreferredSessionType = { providerId: 'agent-host', sessionTypeId: 'claude' }; + const preferredType: IPreferredSessionType = { providerId: 'copilot', sessionTypeId: 'copilot-cli' }; + const cases: { pick: IPreferredSessionType | undefined; servable: boolean; preferred: IPreferredSessionType | undefined }[] = [ + { pick: userPick, servable: true, preferred: preferredType }, + { pick: userPick, servable: false, preferred: preferredType }, + { pick: undefined, servable: true, preferred: preferredType }, + { pick: undefined, servable: true, preferred: undefined }, + ]; + + const requested = await Promise.all(cases.map(async ({ pick, servable, preferred }) => { + let options: IOpenNewSessionOptions | undefined; + await createSessionNow.call({ + _newChatInput: { sessionTypePicker: { getPreferredSessionType: () => preferred } }, + _workspacePicker: { selectedResolved: { providerId: 'workspace-provider' } }, + sessionsService: { + openNewSession: async opts => { + options = opts; + return { session: undefined, trustDeclined: false }; + }, + }, + logService: { error: () => { } }, + _isPreferredServable: () => servable, + }, folder, pick, CancellationToken.None); + return { providerId: options?.providerId, sessionTypeId: options?.sessionTypeId }; + })); + + assert.deepStrictEqual(requested, [ + { providerId: 'agent-host', sessionTypeId: 'claude' }, + { providerId: 'copilot', sessionTypeId: 'copilot-cli' }, + { providerId: 'copilot', sessionTypeId: 'copilot-cli' }, + { providerId: 'workspace-provider', sessionTypeId: undefined }, + ]); + }); + + test('a provider change only recreates the draft when the pick differs from it', () => { + const folder = URI.file('/project'); + const draft: IActiveDraft = { sessionId: 's1', isCreated: constObservable(false), providerId: 'agent-host', sessionType: 'claude' }; + const cases: { name: string; pick: IPreferredSessionType; servable: boolean }[] = [ + { name: 'pick matches the draft', pick: { providerId: 'agent-host', sessionTypeId: 'claude' }, servable: true }, + { name: 'pick names no provider, type matches', pick: { sessionTypeId: 'claude' }, servable: true }, + { name: 'pick names another provider', pick: { providerId: 'other', sessionTypeId: 'claude' }, servable: true }, + { name: 'pick names another type', pick: { providerId: 'agent-host', sessionTypeId: 'codex' }, servable: true }, + { name: 'pick cannot be served yet', pick: { providerId: 'other', sessionTypeId: 'codex' }, servable: false }, + ]; + + const outcomes = cases.map(({ name, pick, servable }) => { + let recreated = false; + const watcher = disposables.add(new MutableDisposable()); + watcher.value = toDisposable(() => { }); + recreateOnProviderChange.call({ + _pendingPreferredUpgrade: watcher, + _session: constObservable(draft), + _newChatInput: { sessionTypePicker: { getPreferredSessionType: () => undefined } }, + _isPreferredServable: () => servable, + _createNewSession: async () => { + recreated = true; + return { session: undefined, trustDeclined: false }; + }, + }, folder, pick, { sessionId: 's1' }); + return `${name}: ${recreated ? 'recreated' : watcher.value ? 'still watching' : 'settled'}`; + }); + + assert.deepStrictEqual(outcomes, [ + 'pick matches the draft: settled', + 'pick names no provider, type matches: settled', + 'pick names another provider: recreated', + 'pick names another type: recreated', + 'pick cannot be served yet: still watching', + ]); + }); + test('refreshes prompt options when the draft workspace changes', () => { const changes: string[] = []; const harness: IPromptOptionsWorkspaceHarness = {