From 79431e4c4fe283f4f74c2cd67d347114d12214d8 Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Fri, 17 Apr 2026 17:04:14 +0200 Subject: [PATCH 1/2] CCR feedback for #8674 Co-authored-by: Copilot --- src/view/reviewManager.ts | 64 +++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/src/view/reviewManager.ts b/src/view/reviewManager.ts index 8e6a9ff559..7a6eb0f292 100644 --- a/src/view/reviewManager.ts +++ b/src/view/reviewManager.ts @@ -77,6 +77,7 @@ export class ReviewManager extends Disposable { */ private _cachedMaxPRNumbers: Map | undefined; private _cachedBranchName: string | undefined; + private _pollHandle: NodeJS.Timeout | undefined; /** * Flag set when the "Checkout" action is used and cleared on the next git * state update, once review mode has been entered. Used to disambiguate @@ -136,6 +137,12 @@ export class ReviewManager extends Disposable { this.updateState(true); } this.pollForStateChange(); + this._register(toDisposable(() => { + if (this._pollHandle) { + clearTimeout(this._pollHandle); + this._pollHandle = undefined; + } + })); } private registerListeners(): void { @@ -214,12 +221,22 @@ export class ReviewManager extends Disposable { } private pollForStateChange() { - setTimeout(async () => { + this._pollHandle = setTimeout(async () => { + if (this.isDisposed) { + return; + } Logger.appendLine('Polling for state change...', this.id); - if (!this._validateStatusInProgress && !this._folderRepoManager.activePullRequest) { - await this.updateState(); + try { + if (!this._validateStatusInProgress && !this._folderRepoManager.activePullRequest) { + await this.updateState(); + } + } catch (e) { + Logger.warn(`Polling for state change failed: ${e}`, this.id); + } finally { + if (!this.isDisposed) { + this.pollForStateChange(); + } } - this.pollForStateChange(); }, 1000 * 60 * 5); } @@ -436,21 +453,26 @@ export class ReviewManager extends Disposable { if (!headRepo) { return []; } - const metadata = await headRepo.getMetadata(); - if (!metadata?.owner) { + try { + const metadata = await headRepo.getMetadata(); + if (!metadata?.owner) { + return [headRepo]; + } + const parentRepos = this._folderRepoManager.gitHubRepositories.filter(repo => { + if (metadata.fork) { + return repo.remote.owner === metadata.parent?.owner?.login && repo.remote.repositoryName === metadata.parent?.name; + } else { + return repo.remote.owner === metadata.owner?.login && repo.remote.repositoryName === metadata.name; + } + }); + // Include both head and parent repos, deduplicated + const result = new Set(parentRepos); + result.add(headRepo); + return [...result]; + } catch (e) { + Logger.warn(`Failed to get metadata for relevant repos: ${e}`, this.id); return [headRepo]; } - const parentRepos = this._folderRepoManager.gitHubRepositories.filter(repo => { - if (metadata.fork) { - return repo.remote.owner === metadata.parent?.owner?.login && repo.remote.repositoryName === metadata.parent?.name; - } else { - return repo.remote.owner === metadata.owner?.login && repo.remote.repositoryName === metadata.name; - } - }); - // Include both head and parent repos, deduplicated - const result = new Set(parentRepos); - result.add(headRepo); - return [...result]; } private async getMaxPullRequestNumbers(): Promise> { @@ -471,6 +493,11 @@ export class ReviewManager extends Disposable { return true; } const current = await this.getMaxPullRequestNumbers(); + // If we couldn't determine max PR for any repo, assume there may be new PRs + if (current.size === 0) { + this._cachedMaxPRNumbers = current; + return true; + } let result = false; for (const [key, value] of current) { const cached = this._cachedMaxPRNumbers.get(key); @@ -546,9 +573,6 @@ export class ReviewManager extends Disposable { await this.clear(true); return; } - if (!this._cachedMaxPRNumbers) { - this._cachedMaxPRNumbers = await this.getMaxPullRequestNumbers(); - } const branch = this._repository.state.HEAD; const ignoreBranches = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(IGNORE_PR_BRANCHES); From 705a4a705b01cd7503f8bcfc16311498add4b583 Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Fri, 17 Apr 2026 17:35:49 +0200 Subject: [PATCH 2/2] CCR feedback --- src/view/reviewManager.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/view/reviewManager.ts b/src/view/reviewManager.ts index 7a6eb0f292..57ab947136 100644 --- a/src/view/reviewManager.ts +++ b/src/view/reviewManager.ts @@ -231,7 +231,7 @@ export class ReviewManager extends Disposable { await this.updateState(); } } catch (e) { - Logger.warn(`Polling for state change failed: ${e}`, this.id); + Logger.warn(`Polling for state change failed: ${formatError(e)}`, this.id); } finally { if (!this.isDisposed) { this.pollForStateChange(); @@ -470,7 +470,7 @@ export class ReviewManager extends Disposable { result.add(headRepo); return [...result]; } catch (e) { - Logger.warn(`Failed to get metadata for relevant repos: ${e}`, this.id); + Logger.warn(`Failed to get metadata for relevant repos: ${formatError(e)}`, this.id); return [headRepo]; } } @@ -493,9 +493,9 @@ export class ReviewManager extends Disposable { return true; } const current = await this.getMaxPullRequestNumbers(); - // If we couldn't determine max PR for any repo, assume there may be new PRs + // If we couldn't determine max PR for any repo, assume there may be new PRs, + // but keep the last known good cache so transient failures do not discard it. if (current.size === 0) { - this._cachedMaxPRNumbers = current; return true; } let result = false;