Skip to content

Track inaccessible repos, and don't try to use them again#8691

Merged
alexr00 merged 2 commits intomainfrom
alexr00/tremendous-mammal
Apr 23, 2026
Merged

Track inaccessible repos, and don't try to use them again#8691
alexr00 merged 2 commits intomainfrom
alexr00/tremendous-mammal

Conversation

@alexr00
Copy link
Copy Markdown
Member

@alexr00 alexr00 commented Apr 22, 2026

Co-authored-by: Copilot copilot@github.com

Co-authored-by: Copilot <copilot@github.com>
@alexr00 alexr00 self-assigned this Apr 22, 2026
Co-authored-by: Copilot <copilot@github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to avoid repeatedly attempting operations against GitHub repositories that the extension cannot access, by tracking “inaccessible” repos and short-circuiting future lookups/uses.

Changes:

  • Make FolderRepositoryManager.createGitHubRepositoryFromOwnerName return undefined when repo metadata cannot be fetched, and record the repo as inaccessible for the session.
  • Update callers to handle undefined repository creation results (e.g., PR category fetching, overview restoration, create PR view).
  • Emit telemetry when a repository is detected as inaccessible.
Show a summary per file
File Description
src/github/overviewRestorer.ts Disposes restored overview panels if repo creation fails.
src/github/folderRepositoryManager.ts Tracks inaccessible repos, returns undefined for inaccessible repos, and updates multiple call sites.
src/github/createPRViewProvider.ts Handles missing merge configuration when repo creation fails.

Copilot's findings

Comments suppressed due to low confidence (6)

src/github/folderRepositoryManager.ts:2940

  • No tests cover the new “inaccessible repo” behavior (marking a repo inaccessible after getMetadata() failure, skipping future creations, and ensuring the repo is removed/cleaned up). Since this file already has unit tests, please add coverage for the new branch, including the telemetry call and the behavior when a repo becomes inaccessible.
	async createGitHubRepositoryFromOwnerName(owner: string, repositoryName: string): Promise<GitHubRepository | undefined> {
		const existing = this.findExistingGitHubRepository({ owner, repositoryName });
		if (existing) {
			return existing;
		}
		const repoKey = `${owner.toLowerCase()}/${repositoryName.toLowerCase()}`;
		if (this._inaccessibleRepos.has(repoKey)) {
			Logger.debug(`Skipping inaccessible repository: ${owner}/${repositoryName}`, this.id);
			return undefined;
		}
		const gitRemotes = parseRepositoryRemotes(this.repository);
		const gitRemote = gitRemotes.find(r => r.owner === owner && r.repositoryName === repositoryName);
		const uri = gitRemote?.url ?? `https://github.com/${owner}/${repositoryName}`;
		const repo = await this.createAndAddGitHubRepository(new Remote(gitRemote?.remoteName ?? repositoryName, uri, new Protocol(uri)), this._credentialStore);
		let reason: string;
		try {
			await repo.getMetadata();
			return repo;
		} catch (e) {
			reason = 'error';
			Logger.appendLine(`Repository ${owner}/${repositoryName} is not accessible: ${e}`, this.id);
		}
		Logger.appendLine(`Repository ${owner}/${repositoryName} is not accessible.`, this.id);
		this._inaccessibleRepos.add(repoKey);
		this.removeGitHubRepository(repo.remote);
		/* __GDPR__
			"repository.inaccessible" : {
				"hasLocalRemote" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
				"reason" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
			}
		*/
		this.telemetry.sendTelemetryEvent('repository.inaccessible', { hasLocalRemote: (!!gitRemote).toString(), reason });
		return undefined;

src/github/folderRepositoryManager.ts:2933

  • createGitHubRepositoryFromOwnerName creates a GitHubRepository with silent undefined (=> comments controller is created/registered) and on metadata failure only removes it from _githubRepositories via removeGitHubRepository. The repo is never disposed, which can leak the VS Code comment controller and other disposables for each inaccessible repo. Consider creating the repo in silent mode until accessibility is verified, and/or explicitly disposing the repo when marking it inaccessible (in addition to removing it from the list).
		const repo = await this.createAndAddGitHubRepository(new Remote(gitRemote?.remoteName ?? repositoryName, uri, new Protocol(uri)), this._credentialStore);
		let reason: string;
		try {
			await repo.getMetadata();
			return repo;
		} catch (e) {
			reason = 'error';
			Logger.appendLine(`Repository ${owner}/${repositoryName} is not accessible: ${e}`, this.id);
		}
		Logger.appendLine(`Repository ${owner}/${repositoryName} is not accessible.`, this.id);
		this._inaccessibleRepos.add(repoKey);
		this.removeGitHubRepository(repo.remote);
		/* __GDPR__

src/github/folderRepositoryManager.ts:2920

  • gitRemote detection uses a case-sensitive comparison (r.owner === owner && r.repositoryName === repositoryName). Callers often pass owner/repo from URLs or stored state with different casing, so this can incorrectly miss an existing local remote, change the chosen remoteName, and skew the hasLocalRemote telemetry. Consider using compareIgnoreCase (as elsewhere in this file) when matching owner/repo here.
		const gitRemotes = parseRepositoryRemotes(this.repository);
		const gitRemote = gitRemotes.find(r => r.owner === owner && r.repositoryName === repositoryName);
		const uri = gitRemote?.url ?? `https://github.com/${owner}/${repositoryName}`;

src/github/folderRepositoryManager.ts:2926

  • createGitHubRepositoryFromOwnerName adds the new repo to _githubRepositories before verifying access via getMetadata(). If two callers race, the second can receive the unverified repo via findExistingGitHubRepository and proceed to use it (and potentially trigger the same failures) before the first call marks it inaccessible/removes it. To make the “don’t try again” behavior reliable, consider validating metadata before adding to the shared list, or tracking an in-flight creation per repoKey so other callers await the same verification result.
		const existing = this.findExistingGitHubRepository({ owner, repositoryName });
		if (existing) {
			return existing;
		}
		const repoKey = `${owner.toLowerCase()}/${repositoryName.toLowerCase()}`;
		if (this._inaccessibleRepos.has(repoKey)) {
			Logger.debug(`Skipping inaccessible repository: ${owner}/${repositoryName}`, this.id);
			return undefined;
		}
		const gitRemotes = parseRepositoryRemotes(this.repository);
		const gitRemote = gitRemotes.find(r => r.owner === owner && r.repositoryName === repositoryName);
		const uri = gitRemote?.url ?? `https://github.com/${owner}/${repositoryName}`;
		const repo = await this.createAndAddGitHubRepository(new Remote(gitRemote?.remoteName ?? repositoryName, uri, new Protocol(uri)), this._credentialStore);
		let reason: string;
		try {
			await repo.getMetadata();
			return repo;
		} catch (e) {

src/github/folderRepositoryManager.ts:2568

  • githubRepos can now be an empty array if both createGitHubRepositoryFromOwnerName calls return undefined. ConflictResolutionCoordinator passes these repos into GitHubContentProvider, which assumes at least one repo (this._gitHubRepositories[0]) and will throw when empty. Add a guard for the empty case (e.g., skip entering conflict resolution and surface an error, or fall back to pullRequest.githubRepository when available).
				const githubRepoResults = await Promise.all([this.createGitHubRepositoryFromOwnerName(pullRequest.head!.owner, pullRequest.head!.repositoryCloneUrl.repositoryName), this.createGitHubRepositoryFromOwnerName(pullRequest.base.owner, pullRequest.base.repositoryCloneUrl.repositoryName)]);
				const githubRepos = githubRepoResults.filter((r): r is GitHubRepository => !!r);
				const coordinator = new ConflictResolutionCoordinator(this.telemetry, conflictModel, githubRepos);
				continueWithMerge = await coordinator.enterConflictResolutionAndWaitForExit();

src/github/folderRepositoryManager.ts:1344

  • When prRepo is undefined, the mapped promise returns undefined, which later gets logged as "Pull request doesn't appear to exist." (since response?.data is falsy). This is misleading for the “repo inaccessible” case and will add noise to logs. Consider handling the undefined response separately (e.g., filter it out before the logging loop, or log a distinct message/reason).

This issue also appears in the following locations of the same file:

  • line 2565
  • line 2908
  • line 2909
  • line 2918
  • line 2921
				const prRepo = await this.createGitHubRepositoryFromOwnerName(protocol.owner, protocol.repositoryName);
				if (!prRepo) {
					return undefined;
				}
				const data = await prRepo.getPullRequest(item.number, 'FolderRepositoryManager.getPullRequestsForCategory', false, true);
				return { data, repo: prRepo };
			});

			const hasMorePages = !!headers.link && headers.link.indexOf('rel="next"') > -1;
			const pullRequestResponses = await Promise.all(promises);

			const pullRequests = (await Promise.all(pullRequestResponses
				.map(async response => {
					if (!response?.data) {
						Logger.appendLine('Pull request doesn\'t appear to exist.', this.id);
						return null;
  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment thread src/github/createPRViewProvider.ts
@alexr00 alexr00 marked this pull request as ready for review April 23, 2026 09:47
@alexr00 alexr00 enabled auto-merge (squash) April 23, 2026 09:47
@alexr00 alexr00 merged commit b52a2ee into main Apr 23, 2026
10 checks passed
@alexr00 alexr00 deleted the alexr00/tremendous-mammal branch April 23, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants