accounts: preserve sign-in during session replacement

extensions/github-authentication/src/github.ts

@@ -572,6 +572,16 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
 		}
 	}
 
+	private logSessionChange(source: 'secret-storage' | 'interactive-login', added: number, removed: number, changed: number): void {
+		const kind = added > 0 && removed > 0 ? 'replacement' : added > 0 ? 'addition' : removed > 0 ? 'removal' : 'change';
+		const message = `Session change: source=${source}, kind=${kind}, added=${added}, removed=${removed}, changed=${changed}.`;
+		if (kind === 'replacement') {
+			this._logger.info(message);
+		} else {
+			this._logger.trace(message);
+		}
+	}
+
 	private async checkForUpdates() {
 		// Only the persisted sessions are reconciled against the Keychain: transient sessions do not
 		// exist there, so they must never be diffed against it and reported as removed.
@@ -601,6 +611,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
 		});
 
 		if (added.length || removed.length) {
+			this.logSessionChange('secret-storage', added.length, removed.length, 0);
 			this._sessionChangeEmitter.fire({ added, removed, changed: [] });
 		}
 	}
@@ -748,6 +759,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
 			}
 			await this.storeSessions(sessions);
 
+			this.logSessionChange('interactive-login', 1, removed.length, 0);
 			this._sessionChangeEmitter.fire({ added: [session], removed, changed: [] });
 
 			this._logger.info('Login success!');

src/vs/workbench/services/accounts/browser/defaultAccount.ts

@@ -443,10 +443,22 @@ export class DefaultAccountProvider extends Disposable implements IDefaultAccoun
 			if (e.providerId !== defaultAccountProvider.id) {
 				return;
 			}
-			if (this.defaultAccount && e.event.removed?.some(session => session.id === this.defaultAccount?.sessionId)) {
+			// Providers can atomically replace a session, so only removal-only events represent an immediate sign-out.
+			const added = e.event.added?.length ?? 0;
+			const removed = e.event.removed?.length ?? 0;
+			const changed = e.event.changed?.length ?? 0;
+			const currentSessionRemoved = this.defaultAccount !== null && (e.event.removed?.some(session => session.id === this.defaultAccount?.sessionId) ?? false);
+			const hasReplacementCandidates = added > 0 || changed > 0;
+			const clearDefaultAccount = currentSessionRemoved && !hasReplacementCandidates;
+			const message = `[DefaultAccount] Authentication sessions changed: added=${added}, removed=${removed}, changed=${changed}, currentSessionRemoved=${currentSessionRemoved}, hasReplacementCandidates=${hasReplacementCandidates}, decision=${clearDefaultAccount ? 'clear' : 'reconcile'}`;
+			if (currentSessionRemoved || (added > 0 && removed > 0)) {
+				this.logService.info(message);
+			} else {
+				this.logService.trace(message);
+			}
+			if (clearDefaultAccount) {
 				this.setDefaultAccount(null);
 			} else {
-				this.logService.debug('[DefaultAccount] Sessions changed for default account provider, updating default account');
 				this.updateDefaultAccount();
 			}
 		}));
@@ -561,8 +573,13 @@ export class DefaultAccountProvider extends Disposable implements IDefaultAccoun
 	}
 
 	private async doUpdateDefaultAccount(options?: IDefaultAccountRefreshOptions): Promise<void> {
+		const currentSessionId = this.defaultAccount?.sessionId;
 		try {
 			const defaultAccount = await this.fetchDefaultAccount(options);
+			if (currentSessionId && this.defaultAccount?.sessionId !== currentSessionId) {
+				this.logService.info('[DefaultAccount] Discarding default account update because the current session changed while the update was in progress');
+				return;
+			}
 			this.setDefaultAccount(defaultAccount);
 			this.scheduleAccountDataPoll();
 		} catch (error) {

src/vs/workbench/services/accounts/test/browser/defaultAccount.test.ts

@@ -4,9 +4,10 @@
  *--------------------------------------------------------------------------------------------*/
 
 import assert from 'assert';
-import { timeout } from '../../../../../base/common/async.js';
+import { DeferredPromise, timeout } from '../../../../../base/common/async.js';
 import { bufferToStream, VSBuffer } from '../../../../../base/common/buffer.js';
-import { Event } from '../../../../../base/common/event.js';
+import { Emitter, Event } from '../../../../../base/common/event.js';
+import { isWeb } from '../../../../../base/common/platform.js';
 import { IRequestContext, IRequestOptions } from '../../../../../base/parts/request/common/request.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
 import { ICommandService } from '../../../../../platform/commands/common/commands.js';
@@ -23,14 +24,14 @@ import { IRequestService } from '../../../../../platform/request/common/request.
 import { InMemoryStorageService, IStorageService } from '../../../../../platform/storage/common/storage.js';
 import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js';
 import { NullTelemetryService } from '../../../../../platform/telemetry/common/telemetryUtils.js';
-import { AuthenticationSession, IAuthenticationExtensionsService, IAuthenticationService } from '../../../authentication/common/authentication.js';
+import { AuthenticationSession, AuthenticationSessionsChangeEvent, IAuthenticationExtensionsService, IAuthenticationService } from '../../../authentication/common/authentication.js';
 import { IWorkbenchEnvironmentService } from '../../../environment/common/environmentService.js';
 import { IExtensionService } from '../../../extensions/common/extensions.js';
 import { IHostService } from '../../../host/browser/host.js';
 import { DefaultAccountProvider } from '../../browser/defaultAccount.js';
 import { TestProductService } from '../../../../test/common/workbenchTestServices.js';
 
-suite('DefaultAccountProvider managed settings', () => {
+suite('DefaultAccountProvider', () => {
 
 	const disposables = ensureNoDisposablesAreLeakedInTestSuite();
 	const accountId = 'account';
@@ -775,11 +776,122 @@ suite('DefaultAccountProvider managed settings', () => {
 		});
 	});
 
+	test('reconciles a replacement without a signed-out gap and preserves removal-only behavior', async () => {
+		const sessionChanges = disposables.add(new Emitter<{ providerId: string; label: string; event: AuthenticationSessionsChangeEvent }>());
+		let authenticationSessions = sessions;
+		const provider = await createProvider(
+			new TestRequestService(async () => jsonResponse({ chat_enabled: true })),
+			{},
+			{},
+			'',
+			{
+				getSessions: async () => authenticationSessions,
+				onDidChangeSessions: sessionChanges.event,
+			}
+		);
+		const observedSessionIds: Array<string | null> = [];
+		disposables.add(provider.onDidChangeDefaultAccount(account => observedSessionIds.push(account?.sessionId ?? null)));
+		const replacementSession = { ...sessions[0], id: 'replacement-session', accessToken: 'replacement-token' };
+		authenticationSessions = [replacementSession];
+		const beforeReplacement = provider.defaultAccount?.sessionId;
+		const replacement = Event.toPromise(Event.filter(
+			provider.onDidChangeDefaultAccount,
+			account => account?.sessionId === replacementSession.id
+		));
+
+		sessionChanges.fire({
+			providerId: 'github',
+			label: 'GitHub',
+			event: { added: [replacementSession], removed: sessions, changed: [] },
+		});
+		const afterReplacementEvent = provider.defaultAccount?.sessionId;
+		const afterReplacement = (await replacement)?.sessionId;
+
+		authenticationSessions = [];
+		sessionChanges.fire({
+			providerId: 'github',
+			label: 'GitHub',
+			event: { added: [], removed: [replacementSession], changed: [] },
+		});
+
+		assert.deepStrictEqual({
+			beforeReplacement,
+			afterReplacementEvent,
+			afterReplacement,
+			afterRemovalOnlyEvent: provider.defaultAccount?.sessionId,
+			observedSessionIds,
+		}, {
+			beforeReplacement: 'session',
+			afterReplacementEvent: 'session',
+			afterReplacement: 'replacement-session',
+			afterRemovalOnlyEvent: undefined,
+			observedSessionIds: ['replacement-session', null],
+		});
+	});
+
+	test('does not restore a removed session from an in-flight replacement refresh', async () => {
+		const sessionChanges = disposables.add(new Emitter<{ providerId: string; label: string; event: AuthenticationSessionsChangeEvent }>());
+		const refreshStarted = new DeferredPromise<void>();
+		const releaseRefresh = new DeferredPromise<IRequestContext>();
+		let authenticationSessions = sessions;
+		let blockRefresh = false;
+		const provider = await createProvider(
+			new TestRequestService(async options => {
+				if (blockRefresh && options.callSite === 'defaultAccount.entitlements') {
+					refreshStarted.complete();
+					return releaseRefresh.p;
+				}
+				return jsonResponse({ chat_enabled: true });
+			}),
+			{},
+			{},
+			'',
+			{
+				getSessions: async () => authenticationSessions,
+				onDidChangeSessions: sessionChanges.event,
+			}
+		);
+		const observedSessionIds: Array<string | null> = [];
+		disposables.add(provider.onDidChangeDefaultAccount(account => observedSessionIds.push(account?.sessionId ?? null)));
+		const replacementSession = { ...sessions[0], accessToken: 'replacement-token' };
+		authenticationSessions = [replacementSession];
+		blockRefresh = true;
+
+		sessionChanges.fire({
+			providerId: 'github',
+			label: 'GitHub',
+			event: { added: [replacementSession], removed: sessions, changed: [] },
+		});
+		const replacementRefresh = provider.refresh({ forceRefresh: true });
+		await refreshStarted.p;
+
+		authenticationSessions = [];
+		sessionChanges.fire({
+			providerId: 'github',
+			label: 'GitHub',
+			event: { added: [], removed: [replacementSession], changed: [] },
+		});
+		const afterRemoval = provider.defaultAccount?.sessionId;
+		releaseRefresh.complete(jsonResponse({ chat_enabled: false }));
+		await replacementRefresh;
+
+		assert.deepStrictEqual({
+			afterRemoval,
+			afterBlockedRefresh: provider.defaultAccount?.sessionId,
+			observedSessionIds,
+		}, {
+			afterRemoval: undefined,
+			afterBlockedRefresh: undefined,
+			observedSessionIds: [null],
+		});
+	});
+
 	async function createProvider(
 		requestService: TestRequestService,
 		nativeManagedSettings: ManagedSettingsData = {},
 		fileManagedSettings: ManagedSettingsData = {},
-		managedSettingsUrl = 'https://api.github.com/copilot_internal/managed_settings'
+		managedSettingsUrl = 'https://api.github.com/copilot_internal/managed_settings',
+		authenticationServiceOverrides: Partial<IAuthenticationService> = {},
 	): Promise<DefaultAccountProvider> {
 		const instantiationService = disposables.add(new TestInstantiationService());
 		instantiationService.stub(IConfigurationService, new TestConfigurationService());
@@ -792,6 +904,7 @@ suite('DefaultAccountProvider managed settings', () => {
 			onDidChangeSessions: Event.None,
 			onDidRegisterAuthenticationProvider: Event.None,
 			onDidUnregisterAuthenticationProvider: Event.None,
+			...authenticationServiceOverrides,
 		});
 		instantiationService.stub(IAuthenticationExtensionsService, {
 			getAccountPreference: () => undefined,
@@ -802,7 +915,7 @@ suite('DefaultAccountProvider managed settings', () => {
 		instantiationService.stub(IRequestService, requestService);
 		instantiationService.stub(ILogService, new NullLogService());
 		instantiationService.stub(IWorkbenchEnvironmentService, {
-			remoteAuthority: undefined,
+			remoteAuthority: isWeb ? 'test-remote' : undefined,
 			isSessionsWindow: false,
 		});
 		instantiationService.stub(IProductService, {