Skip to content

Commit deb6ad5

Browse files
Copilotalexr00
andauthored
Serialize resolveCommentThread operations to fix race condition
When resolving multiple PR comments quickly in the webview, concurrent resolveCommentThread handlers would race: each fetches the full timeline after its mutation, and a stale response from an earlier call can overwrite a newer one. Serialize these operations using a promise queue so each mutation + timeline fetch completes before the next starts. Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/cbcba014-095c-41d9-a8bd-d041f599c908 Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 83f7827 commit deb6ad5

File tree

1 file changed

+19
-13
lines changed

1 file changed

+19
-13
lines changed

src/github/pullRequestOverview.ts

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
6464

6565
private _prListeners: vscode.Disposable[] = [];
6666
private _updatingPromise: Promise<unknown> | undefined;
67+
private _resolveCommentThreadQueue: Promise<void> = Promise.resolve();
6768

6869
public static override async createOrShow(
6970
telemetry: ITelemetry,
@@ -785,20 +786,25 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
785786
return PullRequestModel.openChanges(this._folderRepositoryManager, this._item, openToTheSide);
786787
}
787788

788-
private async resolveCommentThread(message: IRequestMessage<{ threadId: string, toResolve: boolean, thread: IComment[] }>) {
789-
try {
790-
if (message.args.toResolve) {
791-
await this._item.resolveReviewThread(message.args.threadId);
792-
}
793-
else {
794-
await this._item.unresolveReviewThread(message.args.threadId);
789+
private resolveCommentThread(message: IRequestMessage<{ threadId: string, toResolve: boolean, thread: IComment[] }>) {
790+
// Serialize resolve/unresolve operations so that concurrent calls don't race.
791+
// Each call fetches the full timeline after its mutation, and without serialization
792+
// a stale timeline response from an earlier call can overwrite a newer one.
793+
this._resolveCommentThreadQueue = this._resolveCommentThreadQueue.then(async () => {
794+
try {
795+
if (message.args.toResolve) {
796+
await this._item.resolveReviewThread(message.args.threadId);
797+
}
798+
else {
799+
await this._item.unresolveReviewThread(message.args.threadId);
800+
}
801+
const timelineEvents = await this._getTimeline();
802+
this._replyMessage(message, timelineEvents);
803+
} catch (e) {
804+
vscode.window.showErrorMessage(e);
805+
this._replyMessage(message, undefined);
795806
}
796-
const timelineEvents = await this._getTimeline();
797-
this._replyMessage(message, timelineEvents);
798-
} catch (e) {
799-
vscode.window.showErrorMessage(e);
800-
this._replyMessage(message, undefined);
801-
}
807+
});
802808
}
803809

804810
private checkoutPullRequest(message: IRequestMessage<any>): void {

0 commit comments

Comments
 (0)