Optimizes best remote detection (still wip)

Fixes leak with rich remote providers
Avoids needing repo remote subscriptions

src/annotations/lineAnnotationController.ts

@@ -15,6 +15,7 @@ import { detailsMessage } from '../hovers/hovers';
 import { configuration } from '../system/configuration';
 import { debug, log } from '../system/decorators/log';
 import { once } from '../system/event';
+import { debounce } from '../system/function';
 import { count, every, filter } from '../system/iterable';
 import { Logger } from '../system/logger';
 import type { LogScope } from '../system/logger.scope';
@@ -45,7 +46,9 @@ export class LineAnnotationController implements Disposable {
 			once(container.onReady)(this.onReady, this),
 			configuration.onDidChange(this.onConfigurationChanged, this),
 			container.fileAnnotations.onDidToggleAnnotations(this.onFileAnnotationsToggled, this),
-			container.richRemoteProviders.onDidChangeConnectionState(() => void this.refresh(window.activeTextEditor)),
+			container.richRemoteProviders.onAfterDidChangeConnectionState(
+				debounce(() => void this.refresh(window.activeTextEditor), 250),
+			),
 		);
 	}
 

src/container.ts

@@ -190,8 +190,6 @@ export class Container {
 			configuration.onDidChangeAny(this.onAnyConfigurationChanged, this),
 		];
 
-		this._richRemoteProviders = new RichRemoteProviderService(this);
-
 		this._disposables.push((this._connection = new ServerConnection(this)));
 
 		this._disposables.push(
@@ -533,14 +531,6 @@ export class Container {
 		return this._lineTracker;
 	}
 
-	private _repositoryPathMapping: RepositoryPathMappingProvider | undefined;
-	get repositoryPathMapping() {
-		if (this._repositoryPathMapping == null) {
-			this._disposables.push((this._repositoryPathMapping = getSupportedRepositoryPathMappingProvider(this)));
-		}
-		return this._repositoryPathMapping;
-	}
-
 	private readonly _prerelease;
 	get prerelease() {
 		return this._prerelease;
@@ -566,21 +556,27 @@ export class Container {
 		return this._repositoriesView;
 	}
 
-	private readonly _searchAndCompareView: SearchAndCompareView;
-	get searchAndCompareView() {
-		return this._searchAndCompareView;
-	}
-
-	private _subscription: SubscriptionService;
-	get subscription() {
-		return this._subscription;
+	private _repositoryPathMapping: RepositoryPathMappingProvider | undefined;
+	get repositoryPathMapping() {
+		if (this._repositoryPathMapping == null) {
+			this._disposables.push((this._repositoryPathMapping = getSupportedRepositoryPathMappingProvider(this)));
+		}
+		return this._repositoryPathMapping;
 	}
 
-	private readonly _richRemoteProviders: RichRemoteProviderService;
+	private _richRemoteProviders: RichRemoteProviderService | undefined;
 	get richRemoteProviders(): RichRemoteProviderService {
+		if (this._richRemoteProviders == null) {
+			this._richRemoteProviders = new RichRemoteProviderService(this);
+		}
 		return this._richRemoteProviders;
 	}
 
+	private readonly _searchAndCompareView: SearchAndCompareView;
+	get searchAndCompareView() {
+		return this._searchAndCompareView;
+	}
+
 	private readonly _stashesView: StashesView;
 	get stashesView() {
 		return this._stashesView;
@@ -596,6 +592,11 @@ export class Container {
 		return this._storage;
 	}
 
+	private _subscription: SubscriptionService;
+	get subscription() {
+		return this._subscription;
+	}
+
 	private readonly _tagsView: TagsView;
 	get tagsView() {
 		return this._tagsView;

src/env/node/git/localGitProvider.ts

@@ -291,6 +291,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		}
 
 		if (e.changed(RepositoryChange.Remotes, RepositoryChange.RemoteProviders, RepositoryChangeComparisonMode.Any)) {
+			const remotes = this._remotesCache.get(repo.path);
+			void disposeRemotes([remotes]);
 			this._remotesCache.delete(repo.path);
 		}
 
@@ -1091,6 +1093,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		}
 
 		if (caches.length === 0 || caches.includes('remotes')) {
+			const remotes = this._remotesCache.get(repoPath);
+			void disposeRemotes([remotes]);
 			this._remotesCache.delete(repoPath);
 		}
 
@@ -1124,6 +1128,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		}
 
 		if (caches.length === 0 || caches.includes('remotes')) {
+			void disposeRemotes([...this._remotesCache.values()]);
 			this._remotesCache.clear();
 		}
 
@@ -5288,3 +5293,14 @@ async function getEncoding(uri: Uri): Promise {
 	const encodingExists = (await import(/* webpackChunkName: "encoding" */ 'iconv-lite')).encodingExists;
 	return encodingExists(encoding) ? encoding : 'utf8';
 }
+
+async function disposeRemotes(remotes: (Promise<GitRemote[]> | undefined)[]) {
+	const remotesResults = await Promise.allSettled(remotes);
+	for (const remotes of remotesResults) {
+		for (const remote of getSettledValue(remotes) ?? []) {
+			if (remote.hasRichProvider()) {
+				remote.provider?.dispose();
+			}
+		}
+	}
+}

src/git/gitProviderService.ts

@@ -35,6 +35,7 @@ import { Logger } from '../system/logger';
 import { getLogScope, setLogScopeExit } from '../system/logger.scope';
 import { getBestPath, getScheme, isAbsolute, maybeUri, normalizePath } from '../system/path';
 import { asSettled, cancellable, defer, getSettledValue, isPromise, PromiseCancelledError } from '../system/promise';
+import { sortCompare } from '../system/string';
 import { VisitedPathsTrie } from '../system/trie';
 import type {
 	GitCaches,
@@ -198,13 +199,14 @@ export class GitProviderService implements Disposable {
 
 	readonly supportedSchemes = new Set<string>();
 
+	private readonly _bestRemotesCache = new Map<
+		RepoComparisonKey,
+		Promise<GitRemote<RemoteProvider | RichRemoteProvider>[]>
+	>();
 	private readonly _disposable: Disposable;
 	private readonly _pendingRepositories = new Map<RepoComparisonKey, Promise<Repository | undefined>>();
 	private readonly _providers = new Map<GitProviderId, GitProvider>();
 	private readonly _repositories = new Repositories();
-	private readonly _bestRemotesCache: Map<RepoComparisonKey, GitRemote<RemoteProvider | RichRemoteProvider> | null> &
-		Map<`rich|${RepoComparisonKey}`, GitRemote<RichRemoteProvider> | null> &
-		Map<`rich+connected|${RepoComparisonKey}`, GitRemote<RichRemoteProvider> | null> = new Map();
 	private readonly _visitedPaths = new VisitedPathsTrie();
 
 	constructor(private readonly container: Container) {
@@ -213,7 +215,7 @@ export class GitProviderService implements Disposable {
 			window.onDidChangeWindowState(this.onWindowStateChanged, this),
 			workspace.onDidChangeWorkspaceFolders(this.onWorkspaceFoldersChanged, this),
 			configuration.onDidChange(this.onConfigurationChanged, this),
-			container.richRemoteProviders.onDidChangeConnectionState(e => {
+			container.richRemoteProviders.onAfterDidChangeConnectionState(e => {
 				if (e.reason === 'connected') {
 					resetAvatarCache('failed');
 				}
@@ -2113,189 +2115,108 @@ export class GitProviderService implements Disposable {
 		return provider.getIncomingActivity(path, options);
 	}
 
+	@log()
 	async getBestRemoteWithProvider(
-		repoPath: string | Uri | undefined,
-	): Promise<GitRemote<RemoteProvider | RichRemoteProvider> | undefined>;
-	async getBestRemoteWithProvider(
-		remotes: GitRemote[],
-	): Promise<GitRemote<RemoteProvider | RichRemoteProvider> | undefined>;
-	@gate<GitProviderService['getBestRemoteWithProvider']>(
-		remotesOrRepoPath =>
-			`${
-				remotesOrRepoPath == null || typeof remotesOrRepoPath === 'string'
-					? remotesOrRepoPath
-					: remotesOrRepoPath instanceof Uri
-					? remotesOrRepoPath.toString()
-					: `${remotesOrRepoPath.length}:${remotesOrRepoPath[0]?.repoPath ?? ''}`
-			}`,
-	)
-	@log<GitProviderService['getBestRemoteWithProvider']>({
-		args: {
-			0: remotesOrRepoPath =>
-				Array.isArray(remotesOrRepoPath) ? remotesOrRepoPath.map(r => r.name).join(',') : remotesOrRepoPath,
-		},
-	})
-	async getBestRemoteWithProvider(
-		remotesOrRepoPath: GitRemote[] | string | Uri | undefined,
+		repoPath: string | Uri,
 	): Promise<GitRemote<RemoteProvider | RichRemoteProvider> | undefined> {
-		if (remotesOrRepoPath == null) return undefined;
-
-		let remotes;
-		let repoPath;
-		if (Array.isArray(remotesOrRepoPath)) {
-			if (remotesOrRepoPath.length === 0) return undefined;
-
-			remotes = remotesOrRepoPath;
-			repoPath = remotesOrRepoPath[0].repoPath;
-		} else {
-			repoPath = remotesOrRepoPath;
-		}
+		const remotes = await this.getBestRemotesWithProviders(repoPath);
+		return remotes[0];
+	}
 
+	@log()
+	async getBestRemotesWithProviders(
+		repoPath: string | Uri,
+	): Promise<GitRemote<RemoteProvider | RichRemoteProvider>[]> {
+		if (repoPath == null) return [];
 		if (typeof repoPath === 'string') {
 			repoPath = this.getAbsoluteUri(repoPath);
 		}
 
 		const cacheKey = asRepoComparisonKey(repoPath);
-		let remote = this._bestRemotesCache.get(cacheKey);
-		if (remote !== undefined) return remote ?? undefined;
-
-		remotes = (remotes ?? (await this.getRemotesWithProviders(repoPath))).filter(
-			(r: GitRemote): r is GitRemote<RemoteProvider | RichRemoteProvider> => r.provider != null,
-		);
-
-		if (remotes.length === 0) return undefined;
-
-		if (remotes.length === 1) {
-			remote = remotes[0];
-		} else {
-			const weightedRemotes = new Map<string, number>([
-				['upstream', 15],
-				['origin', 10],
-			]);
-
-			const branch = await this.getBranch(remotes[0].repoPath);
-			const branchRemote = branch?.getRemoteName();
-
-			if (branchRemote != null) {
-				weightedRemotes.set(branchRemote, 100);
-			}
-
-			let bestRemote;
-			let weight = 0;
-			for (const r of remotes) {
-				if (r.default) {
-					bestRemote = r;
-					break;
-				}
-
-				// Don't choose a remote unless its weighted above
-				let matchedWeight = weightedRemotes.get(r.name) ?? -1;
+		let remotes = this._bestRemotesCache.get(cacheKey);
+		if (remotes == null) {
+			async function getBest(this: GitProviderService) {
+				const remotes = await this.getRemotesWithProviders(repoPath, { sort: true });
+				if (remotes.length === 0) return [];
+				if (remotes.length === 1) return [...remotes];
+
+				const defaultRemote = remotes.find(r => r.default)?.name;
+				const currentBranchRemote = (await this.getBranch(remotes[0].repoPath))?.getRemoteName();
+
+				const weighted: [number, GitRemote<RemoteProvider | RichRemoteProvider>][] = [];
+
+				let originalFound = false;
+
+				for (const remote of remotes) {
+					let weight;
+					switch (remote.name) {
+						case defaultRemote:
+							weight = 1000;
+							break;
+						case currentBranchRemote:
+							weight = 6;
+							break;
+						case 'upstream':
+							weight = 5;
+							break;
+						case 'origin':
+							weight = 4;
+							break;
+						default:
+							weight = 0;
+					}
 
-				const p = r.provider;
-				if (p.hasRichIntegration() && p.maybeConnected) {
-					const m = await p.getRepositoryMetadata();
-					if (m?.isFork === false) {
-						matchedWeight += 101;
+					// Only check remotes that have extra weighting and less than the default
+					if (weight > 0 && weight < 1000 && !originalFound) {
+						const p = remote.provider;
+						if (
+							p.hasRichIntegration() &&
+							(p.maybeConnected ||
+								(p.maybeConnected === undefined && p.shouldConnect && (await p.isConnected())))
+						) {
+							const repo = await p.getRepositoryMetadata();
+							if (repo != null) {
+								weight += repo.isFork ? -3 : 3;
+								// Once we've found the "original" (not a fork) don't bother looking for more
+								originalFound = !repo.isFork;
+							}
+						}
 					}
-				}
 
-				if (matchedWeight > weight) {
-					bestRemote = r;
-					weight = matchedWeight;
+					weighted.push([weight, remote]);
 				}
+
+				// Sort by the weight, but if both are 0 (no weight) then sort by name
+				weighted.sort(([aw, ar], [bw, br]) => (bw === 0 && aw === 0 ? sortCompare(ar.name, br.name) : bw - aw));
+				return weighted.map(wr => wr[1]);
 			}
 
-			remote = bestRemote ?? null;
+			remotes = getBest.call(this);
+			this._bestRemotesCache.set(cacheKey, remotes);
 		}
 
-		this._bestRemotesCache.set(cacheKey, remote);
-
-		return remote ?? undefined;
+		return [...(await remotes)];
 	}
 
+	@log()
 	async getBestRemoteWithRichProvider(
-		repoPath: string | Uri | undefined,
-		options?: { includeDisconnected?: boolean },
-	): Promise<GitRemote<RichRemoteProvider> | undefined>;
-	async getBestRemoteWithRichProvider(
-		remotes: GitRemote[],
-		options?: { includeDisconnected?: boolean },
-	): Promise<GitRemote<RichRemoteProvider> | undefined>;
-	@gate<GitProviderService['getBestRemoteWithRichProvider']>(
-		(remotesOrRepoPath, options) =>
-			`${
-				remotesOrRepoPath == null || typeof remotesOrRepoPath === 'string'
-					? remotesOrRepoPath
-					: remotesOrRepoPath instanceof Uri
-					? remotesOrRepoPath.toString()
-					: `${remotesOrRepoPath.length}:${remotesOrRepoPath[0]?.repoPath ?? ''}`
-			}|${options?.includeDisconnected ?? false}`,
-	)
-	@log<GitProviderService['getBestRemoteWithRichProvider']>({
-		args: {
-			0: remotesOrRepoPath =>
-				Array.isArray(remotesOrRepoPath) ? remotesOrRepoPath.map(r => r.name).join(',') : remotesOrRepoPath,
-		},
-	})
-	async getBestRemoteWithRichProvider(
-		remotesOrRepoPath: GitRemote[] | string | Uri | undefined,
+		repoPath: string | Uri,
 		options?: { includeDisconnected?: boolean },
 	): Promise<GitRemote<RichRemoteProvider> | undefined> {
-		if (remotesOrRepoPath == null) return undefined;
-
-		let remotes;
-		let repoPath;
-		if (Array.isArray(remotesOrRepoPath)) {
-			if (remotesOrRepoPath.length === 0) return undefined;
-
-			remotes = remotesOrRepoPath;
-			repoPath = remotesOrRepoPath[0].repoPath;
-		} else {
-			repoPath = remotesOrRepoPath;
-		}
-
-		if (typeof repoPath === 'string') {
-			repoPath = this.getAbsoluteUri(repoPath);
-		}
-
-		const cacheKey = asRepoComparisonKey(repoPath);
-
-		let richRemote = this._bestRemotesCache.get(`rich+connected|${cacheKey}`);
-		if (richRemote != null) return richRemote;
-		if (richRemote === null && !options?.includeDisconnected) return undefined;
-
-		if (options?.includeDisconnected) {
-			richRemote = this._bestRemotesCache.get(`rich|${cacheKey}`);
-			if (richRemote !== undefined) return richRemote ?? undefined;
-		}
-
-		const remote = await (remotes != null
-			? this.getBestRemoteWithProvider(remotes)
-			: this.getBestRemoteWithProvider(repoPath));
-
-		if (!remote?.hasRichProvider()) {
-			this._bestRemotesCache.set(`rich|${cacheKey}`, null);
-			this._bestRemotesCache.set(`rich+connected|${cacheKey}`, null);
-			return undefined;
-		}
-
-		const { provider } = remote;
-		const connected = provider.maybeConnected ?? (await provider.isConnected());
-		if (connected) {
-			this._bestRemotesCache.set(`rich|${cacheKey}`, remote);
-			this._bestRemotesCache.set(`rich+connected|${cacheKey}`, remote);
-		} else {
-			this._bestRemotesCache.set(`rich|${cacheKey}`, remote);
-			this._bestRemotesCache.set(`rich+connected|${cacheKey}`, null);
+		const remotes = await this.getBestRemotesWithProviders(repoPath);
 
-			if (!options?.includeDisconnected) return undefined;
+		const includeDisconnected = options?.includeDisconnected ?? false;
+		for (const r of remotes) {
+			if (r.hasRichProvider() && (includeDisconnected || r.provider.maybeConnected === true)) {
+				return r;
+			}
 		}
 
-		return remote;
+		return undefined;
 	}
 
-	@log({ args: { 1: false } })
-	async getRemotes(repoPath: string | Uri | undefined, options?: { sort?: boolean }): Promise<GitRemote[]> {
+	@log()
+	async getRemotes(repoPath: string | Uri, options?: { sort?: boolean }): Promise<GitRemote[]> {
 		if (repoPath == null) return [];
 
 		const { provider, path } = this.getProvider(repoPath);
@@ -2304,16 +2225,10 @@ export class GitProviderService implements Disposable {
 
 	@log()
 	async getRemotesWithProviders(
-		repoPath: string | Uri | undefined,
+		repoPath: string | Uri,
 		options?: { sort?: boolean },
 	): Promise<GitRemote<RemoteProvider | RichRemoteProvider>[]> {
-		if (repoPath == null) return [];
-
-		const repository = this.container.git.getRepository(repoPath);
-		const remotes = await (repository != null
-			? repository.getRemotes(options)
-			: this.getRemotes(repoPath, options));
-
+		const remotes = await this.getRemotes(repoPath, options);
 		return remotes.filter(
 			(r: GitRemote): r is GitRemote<RemoteProvider | RichRemoteProvider> => r.provider != null,
 		);

src/git/models/repository.ts

@@ -9,7 +9,7 @@ import type { Container } from '../../container';
 import type { FeatureAccess, Features, PlusFeatures } from '../../features';
 import { showCreatePullRequestPrompt, showGenericErrorMessage } from '../../messages';
 import { asRepoComparisonKey } from '../../repositories';
-import { filterMap, groupByMap } from '../../system/array';
+import { groupByMap } from '../../system/array';
 import { executeActionCommand, executeCoreGitCommand } from '../../system/command';
 import { configuration } from '../../system/configuration';
 import { formatDate, fromNow } from '../../system/date';
@@ -214,8 +214,6 @@ export class Repository implements Disposable {
 	private _fsWatcherDisposable: Disposable | undefined;
 	private _pendingFileSystemChange?: RepositoryFileSystemChangeEvent;
 	private _pendingRepoChange?: RepositoryChangeEvent;
-	private _remotes: Promise<GitRemote[]> | undefined;
-	private _remotesDisposable: Disposable | undefined;
 	private _suspended: boolean;
 
 	constructor(
@@ -256,6 +254,19 @@ export class Repository implements Disposable {
 		this._disposable = Disposable.from(
 			this.setupRepoWatchers(),
 			configuration.onDidChange(this.onConfigurationChanged, this),
+			// Sending this event in the `'git:cache:reset'` below to avoid unnecessary work. While we will refresh more than needed, this doesn't happen often
+			// container.richRemoteProviders.onAfterDidChangeConnectionState(async e => {
+			// 	const uniqueKeys = new Set<string>();
+			// 	for (const remote of await this.getRemotes()) {
+			// 		if (remote.provider?.hasRichIntegration()) {
+			// 			uniqueKeys.add(remote.provider.key);
+			// 		}
+			// 	}
+
+			// 	if (uniqueKeys.has(e.key)) {
+			// 		this.fireChange(RepositoryChange.RemoteProviders);
+			// 	}
+			// }),
 		);
 
 		this.onConfigurationChanged();
@@ -281,6 +292,10 @@ export class Repository implements Disposable {
 			this.container.events.on('git:cache:reset', e => {
 				if (!e.data.repoPath || e.data.repoPath === this.path) {
 					this.resetCaches(...(e.data.caches ?? emptyArray));
+
+					if (e.data.caches?.includes('providers')) {
+						this.fireChange(RepositoryChange.RemoteProviders);
+					}
 				}
 			}),
 		);
@@ -323,7 +338,6 @@ export class Repository implements Disposable {
 	dispose() {
 		this.stopWatchingFileSystem();
 
-		this._remotesDisposable?.dispose();
 		this._disposable.dispose();
 	}
 
@@ -685,32 +699,15 @@ export class Repository implements Disposable {
 	}
 
 	async getRemotes(options?: { filter?: (remote: GitRemote) => boolean; sort?: boolean }): Promise<GitRemote[]> {
-		if (this._remotes == null) {
-			// Since we are caching the results, always sort
-			this._remotes = this.container.git.getRemotes(this.uri, { sort: true });
-			void this.subscribeToRemotes(this._remotes);
-		}
-
-		return options?.filter != null ? (await this._remotes).filter(options.filter) : this._remotes;
+		const remotes = await this.container.git.getRemotes(
+			this.uri,
+			options?.sort != null ? { sort: options.sort } : undefined,
+		);
+		return options?.filter != null ? remotes.filter(options.filter) : remotes;
 	}
 
 	async getRichRemote(connectedOnly: boolean = false): Promise<GitRemote<RichRemoteProvider> | undefined> {
-		return this.container.git.getBestRemoteWithRichProvider(await this.getRemotes(), {
-			includeDisconnected: !connectedOnly,
-		});
-	}
-
-	private async subscribeToRemotes(remotes: Promise<GitRemote[]>) {
-		this._remotesDisposable?.dispose();
-		this._remotesDisposable = undefined;
-
-		this._remotesDisposable = Disposable.from(
-			...filterMap(await remotes, r => {
-				if (!r.provider?.hasRichIntegration()) return undefined;
-
-				return r.provider.onDidChange(() => this.fireChange(RepositoryChange.RemoteProviders));
-			}),
-		);
+		return this.container.git.getBestRemoteWithRichProvider(this.uri, { includeDisconnected: !connectedOnly });
 	}
 
 	getStash(): Promise<GitStash | undefined> {
@@ -955,11 +952,11 @@ export class Repository implements Disposable {
 			this._branch = undefined;
 		}
 
-		if (caches.length === 0 || caches.includes('remotes')) {
-			this._remotes = undefined;
-			this._remotesDisposable?.dispose();
-			this._remotesDisposable = undefined;
-		}
+		// if (caches.length === 0 || caches.includes('remotes')) {
+		// 	this._remotes = undefined;
+		// 	this._remotesDisposable?.dispose();
+		// 	this._remotesDisposable = undefined;
+		// }
 	}
 
 	resume() {

src/git/parsers/remoteParser.ts

@@ -55,6 +55,10 @@ export class GitRemoteParser {
 				remote.urls.push({ url: url, type: type as GitRemoteType });
 				if (remote.provider != null && type !== 'push') continue;
 
+				if (remote.provider?.hasRichIntegration()) {
+					remote.provider.dispose();
+				}
+
 				const provider = remoteProviderMatcher(url, domain, path);
 				if (provider == null) continue;
 

src/git/remotes/remoteProviderService.ts

@@ -13,6 +13,11 @@ export class RichRemoteProviderService {
 		return this._onDidChangeConnectionState.event;
 	}
 
+	private readonly _onAfterDidChangeConnectionState = new EventEmitter<ConnectionStateChangeEvent>();
+	get onAfterDidChangeConnectionState(): Event<ConnectionStateChangeEvent> {
+		return this._onAfterDidChangeConnectionState.event;
+	}
+
 	private readonly _connectedCache = new Set<string>();
 
 	constructor(private readonly container: Container) {}
@@ -25,6 +30,7 @@ export class RichRemoteProviderService {
 		this.container.telemetry.sendEvent('remoteProviders/connected', { 'remoteProviders.key': key });
 
 		this._onDidChangeConnectionState.fire({ key: key, reason: 'connected' });
+		setTimeout(() => this._onAfterDidChangeConnectionState.fire({ key: key, reason: 'connected' }), 250);
 	}
 
 	disconnected(key: string): void {
@@ -34,5 +40,10 @@ export class RichRemoteProviderService {
 		this.container.telemetry.sendEvent('remoteProviders/disconnected', { 'remoteProviders.key': key });
 
 		this._onDidChangeConnectionState.fire({ key: key, reason: 'disconnected' });
+		setTimeout(() => this._onAfterDidChangeConnectionState.fire({ key: key, reason: 'disconnected' }), 250);
+	}
+
+	isConnected(key?: string): boolean {
+		return key == null ? this._connectedCache.size !== 0 : this._connectedCache.has(key);
 	}
 }

src/git/remotes/remoteProviders.ts

@@ -171,6 +171,7 @@ function createBestRemoteProvider(
 
 		return undefined;
 	} catch (ex) {
+		debugger;
 		Logger.error(ex, 'createRemoteProvider');
 		return undefined;
 	}

src/git/remotes/richRemoteProvider.ts

@@ -1,5 +1,5 @@
 import type { AuthenticationSession, AuthenticationSessionsChangeEvent, Event, MessageItem } from 'vscode';
-import { authentication, EventEmitter, window } from 'vscode';
+import { authentication, Disposable, EventEmitter, window } from 'vscode';
 import { wrapForForcedInsecureSSL } from '@env/fetch';
 import { isWeb } from '@env/platform';
 import type { Container } from '../../container';
@@ -22,7 +22,7 @@ import { RemoteProvider } from './remoteProvider';
 
 // TODO@eamodio revisit how once authenticated, all remotes are always connected, even after a restart
 
-export abstract class RichRemoteProvider extends RemoteProvider {
+export abstract class RichRemoteProvider extends RemoteProvider implements Disposable {
 	override readonly type: 'simple' | 'rich' = 'rich';
 
 	private readonly _onDidChange = new EventEmitter<void>();
@@ -30,6 +30,8 @@ export abstract class RichRemoteProvider extends RemoteProvider {
 		return this._onDidChange.event;
 	}
 
+	private readonly _disposable: Disposable;
+
 	constructor(
 		protected readonly container: Container,
 		domain: string,
@@ -40,7 +42,7 @@ export abstract class RichRemoteProvider extends RemoteProvider {
 	) {
 		super(domain, path, protocol, name, custom);
 
-		container.context.subscriptions.push(
+		this._disposable = Disposable.from(
 			configuration.onDidChange(e => {
 				if (configuration.changed(e, 'remotes')) {
 					this._ignoreSSLErrors.clear();
@@ -58,6 +60,20 @@ export abstract class RichRemoteProvider extends RemoteProvider {
 			}),
 			authentication.onDidChangeSessions(this.onAuthenticationSessionsChanged, this),
 		);
+
+		container.context.subscriptions.push(this._disposable);
+
+		// If we think we should be connected, try to
+		if (this.shouldConnect) {
+			void this.isConnected();
+		}
+	}
+
+	disposed = false;
+
+	dispose() {
+		this._disposable.dispose();
+		this.disposed = true;
 	}
 
 	abstract get apiBaseUrl(): string;
@@ -78,6 +94,11 @@ export abstract class RichRemoteProvider extends RemoteProvider {
 		return this._session === undefined ? undefined : this._session !== null;
 	}
 
+	// This is a hack for now, since providers come and go with remotes
+	get shouldConnect(): boolean {
+		return this.container.richRemoteProviders.isConnected(this.key);
+	}
+
 	protected _session: AuthenticationSession | null | undefined;
 	protected session() {
 		if (this._session === undefined) {

src/hovers/hovers.ts

@@ -213,33 +213,35 @@ export async function detailsMessage(
 
 		if (options?.cancellationToken?.isCancellationRequested) return new MarkdownString();
 	}
-
-	const remotes = await Container.instance.git.getRemotesWithProviders(commit.repoPath, { sort: true });
-
-	if (options?.cancellationToken?.isCancellationRequested) return new MarkdownString();
-
-	const [previousLineComparisonUrisResult, autolinkedIssuesOrPullRequestsResult, prResult, presenceResult] =
-		await Promise.allSettled([
-			commit.isUncommitted ? commit.getPreviousComparisonUrisForLine(editorLine, uri.sha) : undefined,
-			getAutoLinkedIssuesOrPullRequests(message, remotes),
-			options?.pullRequests?.pr ??
-				getPullRequestForCommitOrBestRemote(commit.ref, remotes, {
-					pullRequests:
-						options?.pullRequests?.enabled !== false &&
-						CommitFormatter.has(
-							options.format,
-							'pullRequest',
-							'pullRequestAgo',
-							'pullRequestAgoOrDate',
-							'pullRequestDate',
-							'pullRequestState',
-						),
-				}),
-			Container.instance.vsls.maybeGetPresence(commit.author.email),
-		]);
+	const [
+		remotesResult,
+		previousLineComparisonUrisResult,
+		autolinkedIssuesOrPullRequestsResult,
+		prResult,
+		presenceResult,
+	] = await Promise.allSettled([
+		Container.instance.git.getRemotesWithProviders(commit.repoPath, { sort: true }),
+		commit.isUncommitted ? commit.getPreviousComparisonUrisForLine(editorLine, uri.sha) : undefined,
+		getAutoLinkedIssuesOrPullRequests(message, commit.repoPath),
+		options?.pullRequests?.pr ??
+			getPullRequestForCommitOrBestRemote(commit.ref, commit.repoPath, {
+				pullRequests:
+					options?.pullRequests?.enabled !== false &&
+					CommitFormatter.has(
+						options.format,
+						'pullRequest',
+						'pullRequestAgo',
+						'pullRequestAgoOrDate',
+						'pullRequestDate',
+						'pullRequestState',
+					),
+			}),
+		Container.instance.vsls.maybeGetPresence(commit.author.email),
+	]);
 
 	if (options?.cancellationToken?.isCancellationRequested) return new MarkdownString();
 
+	const remotes = getSettledValue(remotesResult);
 	const previousLineComparisonUris = getSettledValue(previousLineComparisonUrisResult);
 	const autolinkedIssuesOrPullRequests = getSettledValue(autolinkedIssuesOrPullRequestsResult);
 	const pr = getSettledValue(prResult);
@@ -286,7 +288,7 @@ function getDiffFromHunkLine(hunkLine: GitDiffHunkLine, diffStyle?: 'line' | 'hu
 	}\n\`\`\``;
 }
 
-async function getAutoLinkedIssuesOrPullRequests(message: string, remotes: GitRemote[]) {
+async function getAutoLinkedIssuesOrPullRequests(message: string, repoPath: string) {
 	const scope = getNewLogScope('Hovers.getAutoLinkedIssuesOrPullRequests');
 	Logger.debug(scope, `${GlyphChars.Dash} message=<message>`);
 
@@ -303,7 +305,7 @@ async function getAutoLinkedIssuesOrPullRequests(message: string, remotes: GitRe
 		return undefined;
 	}
 
-	const remote = await Container.instance.git.getBestRemoteWithRichProvider(remotes);
+	const remote = await Container.instance.git.getBestRemoteWithRichProvider(repoPath);
 	if (remote?.provider == null) {
 		Logger.debug(scope, `completed [${getDurationMilliseconds(start)}ms]`);
 
@@ -362,7 +364,7 @@ async function getAutoLinkedIssuesOrPullRequests(message: string, remotes: GitRe
 
 async function getPullRequestForCommitOrBestRemote(
 	ref: string,
-	remotes: GitRemote[],
+	repoPath: string,
 	options?: {
 		pullRequests?: boolean;
 	},
@@ -378,9 +380,7 @@ async function getPullRequestForCommitOrBestRemote(
 		return undefined;
 	}
 
-	const remote = await Container.instance.git.getBestRemoteWithRichProvider(remotes, {
-		includeDisconnected: true,
-	});
+	const remote = await Container.instance.git.getBestRemoteWithRichProvider(repoPath, { includeDisconnected: true });
 	if (remote?.provider == null) {
 		Logger.debug(scope, `completed [${getDurationMilliseconds(start)}ms]`);
 

src/views/nodes/commitNode.ts

@@ -236,13 +236,8 @@ export class CommitNode extends ViewRefNode
 	}
 
 	private async getTooltip() {
-		const remotes = await this.view.container.git.getRemotesWithProviders(this.commit.repoPath, { sort: true });
-		const remote = await this.view.container.git.getBestRemoteWithRichProvider(remotes);
-
-		// If we have a "best" remote, move it to the front of the list
-		if (remote != null) {
-			remotes.sort((a, b) => (a === remote ? -1 : b === remote ? 1 : 0));
-		}
+		const remotes = await this.view.container.git.getBestRemotesWithProviders(this.commit.repoPath);
+		const [remote] = remotes;
 
 		if (this.commit.message == null) {
 			await this.commit.ensureFullDetails();
@@ -251,7 +246,7 @@ export class CommitNode extends ViewRefNode
 		let autolinkedIssuesOrPullRequests;
 		let pr;
 
-		if (remote?.provider != null) {
+		if (remote?.hasRichProvider()) {
 			const [autolinkedIssuesOrPullRequestsResult, prResult] = await Promise.allSettled([
 				this.view.container.autolinks.getLinkedIssuesAndPullRequests(
 					this.commit.message ?? this.commit.summary,

src/views/nodes/fileRevisionAsCommitNode.ts

@@ -203,8 +203,8 @@ export class FileRevisionAsCommitNode extends ViewRefFileNode
 	}
 
 	private async getTooltip() {
-		const remotes = await this.view.container.git.getRemotesWithProviders(this.commit.repoPath);
-		const remote = await this.view.container.git.getBestRemoteWithRichProvider(remotes);
+		const remotes = await this.view.container.git.getBestRemotesWithProviders(this.commit.repoPath);
+		const [remote] = remotes;
 
 		if (this.commit.message == null) {
 			await this.commit.ensureFullDetails();
@@ -213,7 +213,7 @@ export class FileRevisionAsCommitNode extends ViewRefFileNode
 		let autolinkedIssuesOrPullRequests;
 		let pr;
 
-		if (remote?.provider != null) {
+		if (remote?.hasRichProvider()) {
 			const [autolinkedIssuesOrPullRequestsResult, prResult] = await Promise.allSettled([
 				this.view.container.autolinks.getLinkedIssuesAndPullRequests(
 					this.commit.message ?? this.commit.summary,

src/views/nodes/rebaseStatusNode.ts

@@ -195,8 +195,8 @@ export class RebaseCommitNode extends ViewRefNode
 	}
 
 	private async getTooltip() {
-		const remotes = await this.view.container.git.getRemotesWithProviders(this.commit.repoPath);
-		const remote = await this.view.container.git.getBestRemoteWithRichProvider(remotes);
+		const remotes = await this.view.container.git.getBestRemotesWithProviders(this.commit.repoPath);
+		const [remote] = remotes;
 
 		if (this.commit.message == null) {
 			await this.commit.ensureFullDetails();
@@ -205,7 +205,7 @@ export class RebaseCommitNode extends ViewRefNode
 		let autolinkedIssuesOrPullRequests;
 		let pr;
 
-		if (remote?.provider != null) {
+		if (remote?.hasRichProvider()) {
 			const [autolinkedIssuesOrPullRequestsResult, prResult] = await Promise.allSettled([
 				this.view.container.autolinks.getLinkedIssuesAndPullRequests(
 					this.commit.message ?? this.commit.summary,