Skip to content

Improve PR stack refreshing #9021

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Alex Ross (alexr00) merged 3 commits into from
Oct 7, 2026
Merged

Improve PR stack refreshing #9021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36110ce
Improve PR stack refreshing
alexr00 Oct 6, 2026
cd785d7
Merge branch 'main' into alexr00/spotless-buzzard
alexr00 Oct 7, 2026
52fb0dd
CCR
alexr00 Oct 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions src/github/githubRepository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ export class GitHubRepository extends Disposable {
public readonly onDidAddPullRequest: vscode.Event<PullRequestModel> = this._onDidAddPullRequest.event;
private _onDidChangePullRequests: vscode.EventEmitter<PullRequestChangeEvent[]> = this._register(new vscode.EventEmitter());
public readonly onDidChangePullRequests: vscode.Event<PullRequestChangeEvent[]> = this._onDidChangePullRequests.event;
private readonly _onDidChangeStack = this._register(new vscode.EventEmitter<readonly number[]>());
public readonly onDidChangeStack = this._onDidChangeStack.event;

notifyStackChanged(numbers: readonly number[]): void {
this._onDidChangeStack.fire(numbers);
Comment thread
alexr00 marked this conversation as resolved.
}

public get hub(): GitHub {
if (this._hub && this.remote.isEnterprise && (!this.authMatchesServer || !this.remote.matchesServerUri(this._hub.serverUri))) {
Expand Down Expand Up @@ -880,11 +886,11 @@ export class GitHubRepository extends Disposable {
return { parentPullRequestNumber: parent.number, stackNumber: stack.number, size: stack.pull_requests.length, url: parent.html_url };
}

async addPullRequestToStack(candidate: StackCandidate, number: number): Promise<void> {
async addPullRequestToStack(candidate: StackCandidate, number: number): Promise<number[]> {
return this.addPullRequestsToStack(candidate, [number]);
}

async addPullRequestsToStack(candidate: StackCandidate, numbers: number[]): Promise<void> {
async addPullRequestsToStack(candidate: StackCandidate, numbers: number[]): Promise<number[]> {
if (numbers.length === 0) {
throw new Error('At least one pull request is required to add to a stack.');
}
Expand All @@ -895,18 +901,30 @@ export class GitHubRepository extends Disposable {
headers: { 'X-GitHub-Api-Version': '2026-03-10' },
};
const stackNumber = candidate.stackNumber;
let data: unknown;
if (stackNumber !== undefined) {
await octokit.call(() => octokit.api.request('POST /repos/{owner}/{repo}/stacks/{stack_number}/add', {
({ data } = await octokit.call(() => octokit.api.request('POST /repos/{owner}/{repo}/stacks/{stack_number}/add', {
...params,
stack_number: stackNumber,
pull_requests: numbers,
}));
})));
} else {
await octokit.call(() => octokit.api.request('POST /repos/{owner}/{repo}/stacks', {
({ data } = await octokit.call(() => octokit.api.request('POST /repos/{owner}/{repo}/stacks', {
...params,
pull_requests: [candidate.parentPullRequestNumber, ...numbers],
}));
})));
}
if (!isObject(data) || !Array.isArray(data.pull_requests) || data.pull_requests.length === 0) {
throw new Error('GitHub returned an invalid result when adding pull requests to a stack.');
}
const members = data.pull_requests.map((pr: unknown) => {
if (!isObject(pr) || typeof pr.number !== 'number') {
throw new Error('GitHub returned an invalid pull request stack entry.');
}
return pr.number;
});
this.notifyStackChanged(members);
return members;
}

async unstackAll(pullRequestNumber: number, expectedPullRequests: readonly number[]): Promise<number[]> {
Expand Down Expand Up @@ -935,13 +953,16 @@ export class GitHubRepository extends Disposable {
stack_number: stacks[0].number,
}));
if (result.status === 204) {
this.notifyStackChanged(expectedPullRequests);
return [];
}
if (result.status !== 200 || !isObject(result.data) || !Array.isArray(result.data.pull_requests)
|| !result.data.pull_requests.every((pr: unknown) => isObject(pr) && typeof pr.number === 'number')) {
throw new Error('GitHub returned an invalid result when unstacking pull requests.');
}
return result.data.pull_requests.map((pr: { number: number }) => pr.number);
const remaining = result.data.pull_requests.map((pr: { number: number }) => pr.number);
this.notifyStackChanged(expectedPullRequests);
return remaining;
}

async canGetProjectsNow(): Promise<boolean> {
Expand Down
3 changes: 3 additions & 0 deletions src/github/issueModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ export interface IssueChangeEvent {
draft?: true;
reviewers?: true;
base?: true;
head?: true;
mergeability?: true;
mergeQueue?: true;
}

export class IssueModel<TItem extends Issue = Issue> extends Disposable {
Expand Down
19 changes: 19 additions & 0 deletions src/github/pullRequestModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,15 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
changes.draft = true;
this.isDraft = item.isDraft;
}
if (this.head && item.head && (this.head.ref !== item.head.ref || this.head.sha !== item.head.sha)) {
changes.head = true;
}
if (this.item.mergeable !== item.mergeable) {
changes.mergeability = true;
}
if (this.base && item.base && (this.base.ref !== item.base.ref || this.base.sha !== item.base.sha)) {
changes.base = true;
}

this.suggestedReviewers = item.suggestedReviewers;
this.closingIssues = item.closingIssues ?? [];
Expand All @@ -315,6 +324,11 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
this.base = new GitHubRef(item.base.ref, item.base!.label, item.base!.sha, item.base!.repo.cloneUrl, item.base.repo.owner, item.base.repo.name, item.base.repo.isInOrganization);
}
if (item.mergeQueueEntry !== undefined) {
if (this.mergeQueueEntry?.position !== item.mergeQueueEntry?.position
|| this.mergeQueueEntry?.state !== item.mergeQueueEntry?.state
|| this.mergeQueueEntry?.url !== item.mergeQueueEntry?.url) {
changes.mergeQueue = true;
}
this.mergeQueueEntry = item.mergeQueueEntry ?? undefined;
}
if (item.hasComments !== undefined) {
Expand Down Expand Up @@ -593,6 +607,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
throw new Error('GitHub returned an unknown stack merge result.');
}
Logger.debug(`Stack merge for #${this.number}: ${response.status}`, PullRequestModel.ID);
this.githubRepository.notifyStackChanged(stack.pullRequests.map(entry => entry.number));
return response.status;
}

Expand Down Expand Up @@ -2278,9 +2293,13 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe

Logger.debug(`Fetch pull request mergeability ${this.number} - done`, PullRequestModel.ID);
const mergeability = parseMergeability(data.repository?.pullRequest.mergeable, data.repository?.pullRequest.mergeStateStatus);
const previousMergeability = this.item.mergeable;
this.item.mergeable = mergeability;
this.conflicts = data.repository?.pullRequest.mergeRequirements?.conditions.find(condition => condition.__typename === 'PullRequestMergeConflictStateCondition')?.conflicts;
this.update(this.item);
if (previousMergeability !== mergeability) {
this._onDidChange.fire({ mergeability: true });
}
return { mergeability, conflicts: this.conflicts };
} catch (e) {
Logger.error(`Unable to fetch PR Mergeability: ${e}`, PullRequestModel.ID);
Expand Down
97 changes: 59 additions & 38 deletions src/github/pullRequestOverview.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
private _updateItemPromise: Promise<void> | undefined;
private _updateSequence = 0;
private _previewSequence = 0;
private _stackLoaded = false;
private _stackPullRequestNumbers = new Set<number>();
private _stackRefreshPending = false;
private _stackRefreshPromise: Promise<void> | undefined;
private _updatingStack = false;
private _canUpdateStackAccess = false;
private _updatableStackPullRequestNumbers: Set<number> | undefined;
Expand Down Expand Up @@ -251,13 +255,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
return super.findPanel(owner, repo, number) as PullRequestOverviewPanel | undefined;
}

public static async refreshStackPanels(owner: string, repo: string, numbers: readonly number[]): Promise<void> {
const panels = numbers
.map(number => this.findPanel(owner, repo, number))
.filter((panel): panel is PullRequestOverviewPanel => !!panel);
await Promise.all(panels.map(panel => panel.refreshPanel()));
}

/**
* Register the webview context-menu commands once globally,
* rather than per panel instance. Each command receives the
Expand Down Expand Up @@ -348,28 +345,52 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
this._prListeners.push(this._folderRepositoryManager.onDidChangeActivePullRequest(() => this.postCheckoutStatus()));

if (this._item) {
const repository = this._item.githubRepository;
this._prListeners.push(repository.onDidChangeStack(numbers => {
if (numbers.includes(this._item.number) || numbers.some(number => this._stackPullRequestNumbers.has(number))) {
void this.refreshStack();
}
}));
this._prListeners.push(repository.onDidChangePullRequests(changes => {
if (changes.some(({ model, event }) => model.number !== this._item.number
&& this._stackPullRequestNumbers.has(model.number)
&& (event.state || event.draft || event.title || event.base || event.head || event.mergeability || event.mergeQueue))) {
void this.refreshStack();
}
}));
this._prListeners.push(this._item.onDidChange(e => {
if (e.draft) {
const item = this._item;
void this.refreshPanel();
if (areStacksEnabled()) {
void item.getStack().then(stack => {
if (stack) {
return PullRequestOverviewPanel.refreshStackPanels(item.remote.owner, item.remote.repositoryName,
stack.pullRequests.filter(entry => entry.number !== item.number).map(entry => entry.number));
}
}).catch(error => {
Logger.error(`Failed to refresh pull request stack after draft change: ${formatError(error)}`, PullRequestOverviewPanel.ID);
void vscode.window.showErrorMessage(vscode.l10n.t('Unable to refresh pull request stack: {0}', formatError(error)));
});
}
} else if ((e.state || e.comments) && !this._refreshing && !this._updateItemPromise) {
this.refreshPanel();
} else if (e.title || e.base || e.head || e.mergeability || e.mergeQueue) {
void this.refreshStack();
}
}));
}
}

private refreshStack(): Promise<void> {
if (this.isDisposed || !areStacksEnabled()) {
return Promise.resolve();
}
this._stackRefreshPending = true;
this._stackRefreshPromise ??= Promise.resolve().then(async () => {
while (this._stackRefreshPending && !this.isDisposed && this._panel.visible && areStacksEnabled()) {
this._stackRefreshPending = false;
if (this._item) {
await this.loadStack(this._item, this._updateSequence);
}
}
}).finally(() => {
this._stackRefreshPromise = undefined;
if (this._stackRefreshPending && !this.isDisposed && this._panel.visible && areStacksEnabled()) {
void this.refreshStack();
}
});
return this._stackRefreshPromise;
}

/**
* Override to process permalinks with PR-specific logic (including diff links).
* Returns undefined if bodyHTML is undefined.
Expand Down Expand Up @@ -408,6 +429,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode

protected override onDidChangeViewState(e: vscode.WebviewPanelOnDidChangeViewStateEvent): void {
super.onDidChangeViewState(e);
if (e.webviewPanel.visible && this._stackRefreshPending) {
void this.refreshStack();
}
this.setVisibilityContext();

// If the panel becomes visible and we have an item, notify that this PR is active
Expand Down Expand Up @@ -492,6 +516,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
this._item = pullRequestModel;
this._diffLinkHashMapPromise = undefined;
const updateSequence = ++this._updateSequence;
this._stackLoaded = false;
const updateStart = performance.now();

try {
Expand Down Expand Up @@ -645,7 +670,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
}).catch(error => {
Logger.error(`Failed to update deferred assignable users: ${formatError(error)}`, PullRequestOverviewPanel.ID);
});
let stackLoaded = false;
const deferredDataPromise = Promise.all([
measureDeferred('statusChecks', pullRequestModel.getStatusChecks()),
reviewRequestsPromise,
Expand Down Expand Up @@ -685,7 +709,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
mergeable: mergeability.mergeability,
reviewers,
hasReviewDraft,
...(stackLoaded ? {} : { mergeQueueMethod }),
...(this._stackLoaded ? {} : { mergeQueueMethod }),
emailForCommit,
currentUserReviewState: this.getCurrentUserReviewState(reviewers, currentUser),
isCopilotOnMyBehalf: isCopilotOnBehalf,
Expand All @@ -700,7 +724,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
Logger.error(`Failed to update deferred pull request data: ${formatError(error)}`, PullRequestOverviewPanel.ID);
});
if (areStacksEnabled()) {
void this.loadStack(pullRequestModel, updateSequence, () => { stackLoaded = true; });
void this.refreshStack();
}
const timelineStart = performance.now();
void Promise.all([pullRequestModel.getTimelineEvents(), reviewRequestsPromise]).then(async ([latestTimelineEvents, requestedReviewers]) => {
Expand Down Expand Up @@ -731,13 +755,14 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
}
}

private async loadStack(pullRequestModel: PullRequestModel, updateSequence: number, onLoaded: () => void): Promise<void> {
private async loadStack(pullRequestModel: PullRequestModel, updateSequence: number): Promise<void> {
try {
const stack = await pullRequestModel.getStack();
if (updateSequence !== this._updateSequence || !areStacksEnabled()) {
if (this.isDisposed || updateSequence !== this._updateSequence || !areStacksEnabled()) {
return;
}
const stackQueueMethod = stack ? await this._folderRepositoryManager.mergeQueueMethodForBranch(stack.base, pullRequestModel.remote.owner, pullRequestModel.remote.repositoryName) : undefined;
const mergeQueueMethod = await this._folderRepositoryManager.mergeQueueMethodForBranch(
stack?.base ?? pullRequestModel.base.ref, pullRequestModel.remote.owner, pullRequestModel.remote.repositoryName);
const linkedStack = stack && {
...stack,
pullRequests: await Promise.all(stack.pullRequests.map(async entry => ({
Expand All @@ -749,24 +774,26 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
})).toString(),
}))),
};
if (updateSequence === this._updateSequence && areStacksEnabled()) {
if (!this.isDisposed && updateSequence === this._updateSequence && areStacksEnabled()) {
const updatableEntries = stack && getUpdatableStackEntries(stack);
this._updatableStackPullRequestNumbers = stack?.needsUpdate && updatableEntries
? new Set(updatableEntries.map(entry => entry.number)) : undefined;
onLoaded();
this._stackLoaded = true;
this._stackPullRequestNumbers = new Set(stack?.pullRequests.map(entry => entry.number));
await this._postMessage({
command: 'pr.update',
pullrequest: {
stack: linkedStack,
stackLoaded: true,
stackLoadError: false,
canUpdateStack: this.canUpdateStack(pullRequestModel),
...(stack ? { mergeQueueMethod: stackQueueMethod } : {}),
mergeQueueMethod,
} satisfies Partial<PullRequest>,
});
}
} catch (error) {
Logger.error(`Failed to load pull request stack: ${formatError(error)}`, PullRequestOverviewPanel.ID);
if (updateSequence === this._updateSequence && areStacksEnabled()) {
if (!this.isDisposed && updateSequence === this._updateSequence && areStacksEnabled()) {
this._updatableStackPullRequestNumbers = undefined;
void this._postMessage({ command: 'pr.update', pullrequest: { stackLoadError: true, canUpdateStack: false } satisfies Partial<PullRequest> });
}
Expand Down Expand Up @@ -1299,8 +1326,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
},
);
await this._replyMessage(message, { updatedPullRequests });
await PullRequestOverviewPanel.refreshStackPanels(this._identity.owner, this._identity.repo,
stackNumbers);
this._item.githubRepository.notifyStackChanged(stackNumbers);
void vscode.window.showInformationMessage(updatedPullRequests.length
? vscode.l10n.t('Pull request stack updated.')
: vscode.l10n.t('Pull request stack is already up to date.'));
Expand All @@ -1309,11 +1335,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
void vscode.window.showErrorMessage(vscode.l10n.t('Unable to update pull request stack: {0}', formatError(error)));
await this._throwError(message, formatError(error));
if (ownsUpdate && stackNumbers.length) {
try {
await PullRequestOverviewPanel.refreshStackPanels(this._identity.owner, this._identity.repo, stackNumbers);
} catch (refreshError) {
Logger.error(`Failed to refresh pull request stack after update error: ${formatError(refreshError)}`, PullRequestOverviewPanel.ID);
}
this._item.githubRepository.notifyStackChanged(stackNumbers);
}
} finally {
if (lockKey) {
Expand Down Expand Up @@ -1362,8 +1384,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
PullRequestOverviewPanel._updatingStacks.add(acquiredLockKey);
const remainingPullRequests = await this._item.githubRepository.unstackAll(this._item.number, expectedPullRequests);
await this._replyMessage(message, { cancelled: false, remainingPullRequests } satisfies UnstackAllResult);
await PullRequestOverviewPanel.refreshStackPanels(this._identity.owner, this._identity.repo,
stack.pullRequests.map(pr => pr.number));
if (remainingPullRequests.length === stack.size) {
void vscode.window.showInformationMessage(vscode.l10n.t('No pull requests were unstacked. Merged, queued, or currently merging pull requests remain in the stack.'));
} else {
Expand Down Expand Up @@ -1782,6 +1802,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode

override dispose() {
++this._updateSequence;
this._stackRefreshPending = false;
super.dispose();
disposeAll(this._prListeners);
}
Expand Down
Loading
Loading