Coding Agent: Unclear which PR the number indicator refers to (#7203)

alexr00 · web-flow · commit c45e36d875f0 · 2025-07-02T09:50:53.000-07:00
Fixes #7169
diff --git a/src/common/uri.ts b/src/common/uri.ts
@@ -426,6 +426,20 @@ export function createPRNodeIdentifier(pullRequest: PullRequestModel | { remote:
 	return identifier;
 }
 
+export function parsePRNodeIdentifier(identifier: string): { remote: string, prNumber: number } | undefined {
+	const lastColon = identifier.lastIndexOf(':');
+	if (lastColon === -1) {
+		return undefined;
+	}
+	const remote = identifier.substring(0, lastColon);
+	const prNumberStr = identifier.substring(lastColon + 1);
+	const prNumber = Number(prNumberStr);
+	if (!remote || isNaN(prNumber) || prNumber <= 0) {
+		return undefined;
+	}
+	return { remote, prNumber };
+}
+
 export function createPRNodeUri(
 	pullRequest: PullRequestModel | { remote: string, prNumber: number } | string
 ): vscode.Uri {
diff --git a/src/github/copilotPrWatcher.ts b/src/github/copilotPrWatcher.ts
@@ -8,6 +8,7 @@ import { COPILOT_LOGINS, copilotEventToStatus, CopilotPRStatus } from '../common
 import { Disposable } from '../common/lifecycle';
 import { PR_SETTINGS_NAMESPACE, QUERIES } from '../common/settingKeys';
 import { FolderRepositoryManager } from './folderRepositoryManager';
+import { PullRequestModel } from './pullRequestModel';
 import { RepositoriesManager } from './repositoriesManager';
 import { variableSubstitution } from './utils';
 
@@ -18,11 +19,11 @@ export function isCopilotQuery(query: string): boolean {
 
 export class CopilotStateModel extends Disposable {
 	private _isInitialized = false;
-	private readonly _states: Map<string, CopilotPRStatus> = new Map();
+	private readonly _states: Map<string, { item: PullRequestModel, status: CopilotPRStatus }> = new Map();
 	private readonly _showNotification: Set<string> = new Set();
 	private readonly _onDidChangeStates = this._register(new vscode.EventEmitter<void>());
 	readonly onDidChangeStates = this._onDidChangeStates.event;
-	private readonly _onDidChangeNotifications = this._register(new vscode.EventEmitter<void>());
+	private readonly _onDidChangeNotifications = this._register(new vscode.EventEmitter<PullRequestModel[]>());
 	readonly onDidChangeNotifications = this._onDidChangeNotifications.event;
 
 	makeKey(owner: string, repo: string, prNumber: number): string {
@@ -37,39 +38,41 @@ export class CopilotStateModel extends Disposable {
 		if (this._states.has(key)) {
 			this._states.delete(key);
 			if (this._showNotification.has(key)) {
+				const item = this._states.get(key)!;
 				this._showNotification.delete(key);
-				this._onDidChangeNotifications.fire();
+				this._onDidChangeNotifications.fire([item.item]);
 			}
 			this._onDidChangeStates.fire();
 		}
 	}
 
-	set(owner: string, repo: string, prNumber: number, status: CopilotPRStatus): void {
-		const key = this.makeKey(owner, repo, prNumber);
+	set(pullRequestModel: PullRequestModel, status: CopilotPRStatus): void {
+		const key = this.makeKey(pullRequestModel.remote.owner, pullRequestModel.remote.repositoryName, pullRequestModel.number);
 		const currentStatus = this._states.get(key);
-		if (currentStatus === status) {
+		if (currentStatus?.status === status) {
 			return;
 		}
-		this._states.set(key, status);
+		this._states.set(key, { item: pullRequestModel, status });
 		if (this._isInitialized) {
 			this._showNotification.add(key);
-			this._onDidChangeNotifications.fire();
+			this._onDidChangeNotifications.fire(pullRequestModel ? [pullRequestModel] : []);
 		}
 		this._onDidChangeStates.fire();
 	}
 
 	get(owner: string, repo: string, prNumber: number): CopilotPRStatus {
 		const key = this.makeKey(owner, repo, prNumber);
-		return this._states.get(key) ?? CopilotPRStatus.None;
+		return this._states.get(key)?.status ?? CopilotPRStatus.None;
 	}
 
 	keys(): string[] {
 		return Array.from(this._states.keys());
 	}
 
 	clearNotifications(): void {
+		const items = Array.from(this._showNotification).map(key => this._states.get(key)?.item).filter((item): item is PullRequestModel => !!item);
 		this._showNotification.clear();
-		this._onDidChangeNotifications.fire();
+		this._onDidChangeNotifications.fire(items);
 	}
 
 	get notifications(): ReadonlySet<string> {
@@ -158,7 +161,7 @@ export class CopilotPRWatcher extends Disposable {
 						prNumber: pr.number,
 						status: latestEvent
 					});
-					this._model.set(pr.remote.owner, pr.remote.repositoryName, pr.number, latestEvent);
+					this._model.set(pr, latestEvent);
 				}
 			}
 
diff --git a/src/github/copilotRemoteAgent.ts b/src/github/copilotRemoteAgent.ts
@@ -54,7 +54,7 @@ export class CopilotRemoteAgentManager extends Disposable {
 	private readonly _stateModel: CopilotStateModel;
 	private readonly _onDidChangeStates = this._register(new vscode.EventEmitter<void>());
 	readonly onDidChangeStates = this._onDidChangeStates.event;
-	private readonly _onDidChangeNotifications = this._register(new vscode.EventEmitter<void>());
+	private readonly _onDidChangeNotifications = this._register(new vscode.EventEmitter<PullRequestModel[]>());
 	readonly onDidChangeNotifications = this._onDidChangeNotifications.event;
 	private readonly _onDidCreatePullRequest = this._register(new vscode.EventEmitter<number>());
 	readonly onDidCreatePullRequest = this._onDidCreatePullRequest.event;
@@ -70,7 +70,7 @@ export class CopilotRemoteAgentManager extends Disposable {
 		this._stateModel = new CopilotStateModel();
 		this._register(new CopilotPRWatcher(this.repositoriesManager, this._stateModel));
 		this._register(this._stateModel.onDidChangeStates(() => this._onDidChangeStates.fire()));
-		this._register(this._stateModel.onDidChangeNotifications(() => this._onDidChangeNotifications.fire()));
+		this._register(this._stateModel.onDidChangeNotifications(items => this._onDidChangeNotifications.fire(items)));
 
 		this._register(this.repositoriesManager.onDidChangeFolderRepositories((event) => {
 			if (event.added) {
@@ -627,7 +627,12 @@ export class CopilotRemoteAgentManager extends Disposable {
 		this._stateModel.clearNotifications();
 	}
 
-	get notifications(): ReadonlySet<string> {
-		return this._stateModel.notifications;
+	get notificationsCount(): number {
+		return this._stateModel.notifications.size;
+	}
+
+	hasNotification(owner: string, repo: string, pullRequestNumber: number): boolean {
+		const key = this._stateModel.makeKey(owner, repo, pullRequestNumber);
+		return this._stateModel.notifications.has(key);
 	}
 }
diff --git a/src/view/prStatusDecorationProvider.ts b/src/view/prStatusDecorationProvider.ts
@@ -5,7 +5,8 @@
 
 import * as vscode from 'vscode';
 import { Disposable } from '../common/lifecycle';
-import { COPILOT_QUERY, createPRNodeUri, fromPRNodeUri, Schemes } from '../common/uri';
+import { Protocol } from '../common/protocol';
+import { COPILOT_QUERY, createPRNodeUri, fromPRNodeUri, parsePRNodeIdentifier, PRNodeUriParams, Schemes } from '../common/uri';
 import { CopilotRemoteAgentManager } from '../github/copilotRemoteAgent';
 import { getStatusDecoration } from '../github/markdownUtils';
 import { PrsTreeModel } from './prsTreeModel';
@@ -26,8 +27,12 @@ export class PRStatusDecorationProvider extends Disposable implements vscode.Fil
 			})
 		);
 
-		this._register(this._copilotManager.onDidChangeNotifications(() => {
-			this._onDidChangeFileDecorations.fire(COPILOT_QUERY);
+		this._register(this._copilotManager.onDidChangeNotifications(items => {
+			const uris = [COPILOT_QUERY];
+			for (const item of items) {
+				uris.push(createPRNodeUri(item));
+			}
+			this._onDidChangeFileDecorations.fire(uris);
 		}));
 	}
 
@@ -46,19 +51,40 @@ export class PRStatusDecorationProvider extends Disposable implements vscode.Fil
 		if (!params) {
 			return;
 		}
+
+		const copilotDecoration = this._getCopilotDecoration(params);
+		if (copilotDecoration) {
+			return copilotDecoration;
+		}
+
 		const status = this._prsTreeModel.cachedPRStatus(params.prIdentifier);
 		if (!status) {
 			return;
 		}
 
-		return getStatusDecoration(status.status) as vscode.FileDecoration;
+		const decoration = getStatusDecoration(status.status) as vscode.FileDecoration;
+		return decoration;
+	}
+
+	private _getCopilotDecoration(params: PRNodeUriParams): vscode.FileDecoration | undefined {
+		const idParts = parsePRNodeIdentifier(params.prIdentifier);
+		if (!idParts) {
+			return;
+		}
+		const protocol = new Protocol(idParts.remote);
+		if (this._copilotManager.hasNotification(protocol.owner, protocol.repositoryName, idParts.prNumber)) {
+			return {
+				badge: new vscode.ThemeIcon('copilot') as any,
+				color: new vscode.ThemeColor('pullRequests.notification')
+			};
+		}
 	}
 
 	private _queryDecoration(uri: vscode.Uri): vscode.ProviderResult<vscode.FileDecoration> {
 		if (uri.path === 'copilot') {
-			if (this._copilotManager.notifications.size > 0) {
+			if (this._copilotManager.notificationsCount > 0) {
 				return {
-					tooltip: vscode.l10n.t('Coding agent has made changes', this._copilotManager.notifications.size),
+					tooltip: vscode.l10n.t('Coding agent has made changes', this._copilotManager.notificationsCount),
 					badge: new vscode.ThemeIcon('copilot') as any,
 					color: new vscode.ThemeColor('pullRequests.notification'),
 				};
diff --git a/src/view/prsTreeDataProvider.ts b/src/view/prsTreeDataProvider.ts
@@ -93,10 +93,10 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
 		});
 
 		this._register(this._copilotManager.onDidChangeStates(() => {
-			if (this._copilotManager.notifications.size > 0) {
+			if (this._copilotManager.notificationsCount > 0) {
 				this._view.badge = {
-					tooltip: this._copilotManager.notifications.size === 1 ? vscode.l10n.t('Coding agent has 1 pull request to view') : vscode.l10n.t('Coding agent has {0} pull requests to view', this._copilotManager.notifications.size),
-					value: this._copilotManager.notifications.size
+					tooltip: this._copilotManager.notificationsCount === 1 ? vscode.l10n.t('Coding agent has 1 pull request to view') : vscode.l10n.t('Coding agent has {0} pull requests to view', this._copilotManager.notificationsCount),
+					value: this._copilotManager.notificationsCount
 				};
 				this.refresh();
 			} else {
diff --git a/src/view/treeNodes/categoryNode.ts b/src/view/treeNodes/categoryNode.ts
@@ -146,7 +146,7 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
 
 		this.prs = new Map();
 
-		const hasCopilotChanges = _categoryQuery && isCopilotQuery(_categoryQuery) && this._copilotManager.notifications.size > 0;
+		const hasCopilotChanges = _categoryQuery && isCopilotQuery(_categoryQuery) && this._copilotManager.notificationsCount > 0;
 
 		switch (this.type) {
 			case PRType.All:
