Skip to content

Commit dea21f8

Browse files
Copilotalexr00
andauthored
Serialize resolveCommentThread to fix race condition with rapid resolves (#8651)
* Initial plan * 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> * CCR feedback --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 0f05d0b commit dea21f8

File tree

1 file changed

+27
-14
lines changed

1 file changed

+27
-14
lines changed

src/github/pullRequestOverview.ts

Lines changed: 27 additions & 14 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,32 @@ 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);
795-
}
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-
}
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+
// Normalize any prior rejection so one unexpected failure does not permanently
794+
// block later resolve/unresolve requests from running.
795+
this._resolveCommentThreadQueue = this._resolveCommentThreadQueue
796+
.catch(error => {
797+
Logger.error(`Resolve comment thread queue failed: ${formatError(error)}`, PullRequestOverviewPanel.ID);
798+
})
799+
.then(async () => {
800+
try {
801+
if (message.args.toResolve) {
802+
await this._item.resolveReviewThread(message.args.threadId);
803+
}
804+
else {
805+
await this._item.unresolveReviewThread(message.args.threadId);
806+
}
807+
const timelineEvents = await this._getTimeline();
808+
this._replyMessage(message, timelineEvents);
809+
} catch (e) {
810+
Logger.error(`Failed to ${message.args.toResolve ? 'resolve' : 'unresolve'} comment thread: ${formatError(e)}`, PullRequestOverviewPanel.ID);
811+
vscode.window.showErrorMessage(vscode.l10n.t('Failed to {0} comment thread: {1}', message.args.toResolve ? 'resolve' : 'unresolve', formatError(e)));
812+
this._replyMessage(message, undefined);
813+
}
814+
});
802815
}
803816

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

0 commit comments

Comments
 (0)