From da7c126a093d2c8dffef22f570283cde502eb9a9 Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Thu, 1 Oct 2026 12:39:04 +0200 Subject: [PATCH 1/2] Add "unstack all" to stack view --- src/github/githubRepository.ts | 31 ++++ src/github/pullRequestOverview.ts | 64 +++++++- src/github/views.ts | 5 + src/test/github/pullRequestModel.test.ts | 53 +++++++ src/test/github/pullRequestOverview.test.ts | 139 ++++++++++++++++++ src/test/mocks/queryProvider.ts | 3 +- src/test/view/prsTree.test.ts | 53 ++++++- src/view/prsTreeDataProvider.ts | 8 +- webviews/common/context.tsx | 5 +- webviews/components/pullRequestStack.tsx | 30 +++- webviews/editorWebview/index.css | 32 ++++ webviews/editorWebview/test/overview.test.tsx | 60 ++++++++ 12 files changed, 474 insertions(+), 9 deletions(-) diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index 40d9bd9d8b..b2f1954da7 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -909,6 +909,37 @@ export class GitHubRepository extends Disposable { } } + async unstackAll(pullRequestNumber: number): Promise { + const { octokit, remote } = await this.ensure(); + const params = { + owner: remote.owner, + repo: remote.repositoryName, + headers: { 'X-GitHub-Api-Version': '2026-03-10' }, + }; + const { data: stacks } = await octokit.call(() => octokit.api.request('GET /repos/{owner}/{repo}/stacks', { + ...params, + pull_request: pullRequestNumber, + per_page: 1, + })); + if (!Array.isArray(stacks) || stacks.length !== 1 || !isObject(stacks[0]) + || typeof stacks[0].number !== 'number' || !Array.isArray(stacks[0].pull_requests) + || !stacks[0].pull_requests.some((pr: unknown) => isObject(pr) && pr.number === pullRequestNumber)) { + throw new Error(`Could not find the stack containing pull request #${pullRequestNumber}.`); + } + const result = await octokit.call(() => octokit.api.request('POST /repos/{owner}/{repo}/stacks/{stack_number}/unstack', { + ...params, + stack_number: stacks[0].number, + })); + if (result.status === 204) { + 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); + } + async canGetProjectsNow(): Promise { let { schema } = await this.ensure(); if (schema.GetRepoProjects && schema.GetOrgProjects) { diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 6b751e1677..06dc25f20e 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -29,7 +29,7 @@ import { isCopilotOnMyBehalf, PullRequestModel } from './pullRequestModel'; import { PullRequestReviewCommon, ReviewContext } from './pullRequestReviewCommon'; import { branchPicks, pickEmail, reviewersQuickPick } from './quickPicks'; import { getIssueOrURLExpression, parseIssueExpressionOutput, parseReviewers, processDiffLinks, processPermalinks } from './utils'; -import { CancelCodingAgentReply, ChangeBaseReply, ChangeReviewersReply, DeleteReviewResult, MergeArguments, MergeResult, PullRequest, ReadyForReviewAndMergeContext, ReadyForReviewContext, ReviewCommentContext, ReviewType, SubmitReviewArgs, UnresolvedIdentity } from './views'; +import { CancelCodingAgentReply, ChangeBaseReply, ChangeReviewersReply, DeleteReviewResult, MergeArguments, MergeResult, PullRequest, ReadyForReviewAndMergeContext, ReadyForReviewContext, ReviewCommentContext, ReviewType, SubmitReviewArgs, UnresolvedIdentity, UnstackAllResult } from './views'; import { debounce } from '../common/async'; import { COPILOT_ACCOUNTS, IComment } from '../common/comment'; import { COPILOT_REVIEWER, COPILOT_REVIEWER_ACCOUNT, COPILOT_SWE_AGENT, copilotEventToStatus, CopilotPRStatus, mostRecentCopilotEvent } from '../common/copilot'; @@ -181,6 +181,13 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { + 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 @@ -266,7 +273,19 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { - if ((e.state || e.comments) && !this._refreshing && !this._updateItemPromise) { + if (e.draft) { + const item = this._item; + void this.refreshPanel(); + 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(); } })); @@ -750,6 +769,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel): Promise { + try { + const access = await this._folderRepositoryManager.getPullRequestRepositoryAccessAndMergeMethods(this._item); + if (!access.hasWritePermission) { + throw new Error(vscode.l10n.t('You do not have permission to unstack these pull requests.')); + } + const stack = await this._item.getStack(); + if (!stack || !stack.pullRequests.some(pr => pr.state !== GithubItemStateEnum.Merged)) { + throw new Error(vscode.l10n.t('No unmerged pull requests are available to unstack.')); + } + const action = vscode.l10n.t('Unstack all'); + const answer = await vscode.window.showWarningMessage( + vscode.l10n.t('Unstack all eligible pull requests?'), + { + modal: true, + detail: vscode.l10n.t('Open, draft, and closed pull requests will be removed from this stack. Their base branches will not change. Merged and queued pull requests will remain in the stack.'), + }, + action, + ); + if (answer !== action) { + await this._replyMessage(message, { cancelled: true } satisfies UnstackAllResult); + return; + } + const remainingPullRequests = await this._item.githubRepository.unstackAll(this._item.number); + 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 or queued pull requests remain in the stack.')); + } else { + void vscode.window.showInformationMessage(vscode.l10n.t('Eligible pull requests unstacked. {0} merged or queued pull requests remain in the stack.', remainingPullRequests.length)); + } + } catch (error) { + Logger.error(`Failed to unstack pull requests: ${formatError(error)}`, PullRequestOverviewPanel.ID); + void vscode.window.showErrorMessage(vscode.l10n.t('Unable to unstack pull requests: {0}', formatError(error))); + await this._throwError(message, formatError(error)); + } + } + private async mergePullRequest( message: IRequestMessage, ): Promise { diff --git a/src/github/views.ts b/src/github/views.ts index 016c7992f1..67c7cb195d 100644 --- a/src/github/views.ts +++ b/src/github/views.ts @@ -189,6 +189,11 @@ export interface StackMergeResult { state?: GithubItemStateEnum; } +export interface UnstackAllResult { + cancelled: boolean; + remainingPullRequests?: number[]; +} + export interface DeleteReviewResult { deletedReviewId: number; deletedReviewComments: IComment[]; diff --git a/src/test/github/pullRequestModel.test.ts b/src/test/github/pullRequestModel.test.ts index c040a344b0..7729c74497 100644 --- a/src/test/github/pullRequestModel.test.ts +++ b/src/test/github/pullRequestModel.test.ts @@ -612,6 +612,59 @@ describe('PullRequestModel', function () { }); }); + describe('unstackAll', function () { + const headers = { 'X-GitHub-Api-Version': '2026-03-10' }; + const params = { owner: 'github', repo: 'test', headers }; + const listRoute = 'GET /repos/{owner}/{repo}/stacks'; + const unstackRoute = 'POST /repos/{owner}/{repo}/stacks/{stack_number}/unstack'; + const listArgs = [listRoute, { ...params, pull_request: 795, per_page: 1 }]; + const unstackArgs = [unstackRoute, { ...params, stack_number: 12 }]; + + it('dissolves a stack when GitHub returns 204', async function () { + repo.queryProvider.expectOctokitRequest(['request'], listArgs, [{ + number: 12, pull_requests: [{ number: 794 }, { number: 795 }], + }]); + repo.queryProvider.expectOctokitRequest(['request'], unstackArgs, undefined, 204); + + assert.deepStrictEqual(await repo.unstackAll(795), []); + }); + + it('reports merged or queued PRs remaining after unstacking', async function () { + repo.queryProvider.expectOctokitRequest(['request'], listArgs, [{ + number: 12, pull_requests: [{ number: 794 }, { number: 795 }], + }]); + repo.queryProvider.expectOctokitRequest(['request'], unstackArgs, { + number: 12, pull_requests: [{ number: 794 }], + }, 200); + + assert.deepStrictEqual(await repo.unstackAll(795), [794]); + }); + + it('does not unstack a different or missing stack', async function () { + repo.queryProvider.expectOctokitRequest(['request'], listArgs, [{ + number: 12, pull_requests: [{ number: 794 }], + }]); + await assert.rejects(repo.unstackAll(795), /Could not find the stack/); + }); + + it('reports an invalid successful response rather than assuming the stack dissolved', async function () { + repo.queryProvider.expectOctokitRequest(['request'], listArgs, [{ + number: 12, pull_requests: [{ number: 795 }], + }]); + repo.queryProvider.expectOctokitRequest(['request'], unstackArgs, { pull_requests: null }, 200); + await assert.rejects(repo.unstackAll(795), /invalid result/); + }); + + it('surfaces an unstack failure rather than reporting success', async function () { + repo.queryProvider.expectOctokitRequest(['request'], listArgs, [{ + number: 12, pull_requests: [{ number: 795 }], + }]); + repo.queryProvider.expectOctokitError(['request'], unstackArgs, new Error('Stack is locked')); + + await assert.rejects(repo.unstackAll(795), /Stack is locked/); + }); + }); + describe('openReadonlyChanges', function () { const baseCommit = '1111111111111111111111111111111111111111'; const mergeBase = '2222222222222222222222222222222222222222'; diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index 7212301f7c..3274ffa9cc 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -796,6 +796,145 @@ describe('PullRequestOverview', function () { }); }); + describe('unstackAll', function () { + async function createPanel() { + const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); + const model = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem); + const identity = { owner: remote.owner, repo: remote.repositoryName, number: model.number }; + await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, model); + const panel = PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number)!; + const stackQuery = sinon.stub(model, 'getStack').resolves({ + position: 2, size: 2, base: 'main', + pullRequests: [ + { position: 1, number: 999, title: 'First', url: '', head: 'D1', state: GithubItemStateEnum.Merged, isDraft: false, mergeable: PullRequestMergeability.Unknown }, + { position: 2, number: 1000, title: 'Second', url: '', head: 'D2', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, + ], + }); + const access = sinon.stub(pullRequestManager, 'getPullRequestRepositoryAccessAndMergeMethods').resolves({ + hasWritePermission: true, + mergeMethodsAvailability: { merge: true, squash: true, rebase: true }, + viewerCanAutoMerge: false, + }); + return { panel, model, access, stackQuery }; + } + + it('confirms unstacking all eligible PRs and reports remaining locked PRs', async function () { + const { panel } = await createPanel(); + const confirm = sinon.stub(vscode.window, 'showWarningMessage').resolves('Unstack all' as never); + const information = sinon.stub(vscode.window, 'showInformationMessage').resolves(undefined); + const unstack = sinon.stub(repo, 'unstackAll').resolves([999]); + const reply = sinon.stub(panel as any, '_replyMessage').resolves(); + const refresh = sinon.stub(panel, 'refreshPanel').resolves(); + const message = { req: '1', command: 'pr.unstack-all', args: undefined }; + + await (panel as any).unstackAll(message); + + assert.strictEqual((confirm.firstCall.args[1] as vscode.MessageOptions).modal, true); + assert.match((confirm.firstCall.args[1] as vscode.MessageOptions).detail!, /Merged and queued pull requests will remain/); + assert(unstack.calledOnceWithExactly(1000)); + sinon.assert.calledWithExactly(reply, message, { cancelled: false, remainingPullRequests: [999] }); + assert(refresh.calledOnce); + assert(information.calledOnce); + sinon.assert.callOrder(unstack, reply, refresh); + }); + + it('refreshes other visible PR panels in the unstacked stack', async function () { + const { panel } = await createPanel(); + const siblingModel = new PullRequestModel(credentialStore, telemetry, repo, remote, + convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(999).build(), repo)); + await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, + { owner: remote.owner, repo: remote.repositoryName, number: 999 }, siblingModel); + const sibling = PullRequestOverviewPanel.findPanel(remote.owner, remote.repositoryName, 999)!; + const refreshSibling = sinon.stub(sibling, 'refreshPanel').resolves(); + sinon.stub(panel, 'refreshPanel').resolves(); + sinon.stub(panel as any, '_replyMessage').resolves(); + sinon.stub(vscode.window, 'showWarningMessage').resolves('Unstack all' as never); + sinon.stub(vscode.window, 'showInformationMessage').resolves(undefined); + sinon.stub(repo, 'unstackAll').resolves([]); + + await (panel as any).unstackAll({ req: '5', command: 'pr.unstack-all', args: undefined }); + + assert(refreshSibling.calledOnce); + }); + + it('refreshes the stack entry in other open panels when a PR changes draft state', async function () { + const { panel, model } = await createPanel(); + const siblingModel = new PullRequestModel(credentialStore, telemetry, repo, remote, + convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(999).build(), repo)); + await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, + { owner: remote.owner, repo: remote.repositoryName, number: 999 }, siblingModel); + const sibling = PullRequestOverviewPanel.findPanel(remote.owner, remote.repositoryName, 999)!; + const refreshCurrent = sinon.stub(panel, 'refreshPanel').resolves(); + let finishRefresh: () => void; + const refreshedSibling = new Promise(resolve => { finishRefresh = resolve; }); + const refreshSibling = sinon.stub(sibling, 'refreshPanel').callsFake(async () => finishRefresh()); + + (model as any)._onDidChange.fire({ draft: true }); + await refreshedSibling; + + assert(refreshCurrent.calledOnce); + assert(refreshSibling.calledOnce); + }); + + it('refreshes only the requested open stack panels', async function () { + const { panel } = await createPanel(); + const refresh = sinon.stub(panel, 'refreshPanel').resolves(); + + await PullRequestOverviewPanel.refreshStackPanels(remote.owner, remote.repositoryName, [1000, 999]); + + assert(refresh.calledOnce); + }); + + it('does not call the Stacks API when confirmation is cancelled', async function () { + const { panel } = await createPanel(); + sinon.stub(vscode.window, 'showWarningMessage').resolves(undefined); + const unstack = sinon.stub(repo, 'unstackAll'); + const reply = sinon.stub(panel as any, '_replyMessage').resolves(); + const message = { req: '2', command: 'pr.unstack-all', args: undefined }; + + await (panel as any).unstackAll(message); + + assert(unstack.notCalled); + sinon.assert.calledWithExactly(reply, message, { cancelled: true }); + }); + + it('rejects unstacking without write permission', async function () { + const { panel, access } = await createPanel(); + access.resolves({ + hasWritePermission: false, + mergeMethodsAvailability: { merge: true, squash: true, rebase: true }, + viewerCanAutoMerge: false, + }); + const unstack = sinon.stub(repo, 'unstackAll'); + const reply = sinon.stub(panel as any, '_throwError').resolves(); + sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const message = { req: '3', command: 'pr.unstack-all', args: undefined }; + + await (panel as any).unstackAll(message); + + assert(unstack.notCalled); + assert.match(reply.firstCall.args[1], /do not have permission/); + }); + + it('does not offer to unstack a stack containing only merged PRs', async function () { + const { panel, stackQuery } = await createPanel(); + stackQuery.resolves({ + position: 1, size: 1, base: 'main', + pullRequests: [{ position: 1, number: 1000, title: 'First', url: '', head: 'D1', state: GithubItemStateEnum.Merged, isDraft: false, mergeable: PullRequestMergeability.Unknown }], + }); + const warning = sinon.stub(vscode.window, 'showWarningMessage').resolves(undefined); + const unstack = sinon.stub(repo, 'unstackAll'); + const reply = sinon.stub(panel as any, '_throwError').resolves(); + sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined); + + await (panel as any).unstackAll({ req: '4', command: 'pr.unstack-all', args: undefined }); + + assert(warning.notCalled); + assert(unstack.notCalled); + assert.match(reply.firstCall.args[1], /No unmerged pull requests/); + }); + }); + describe('deleteBranch', function () { it('replies with the deletion state after deletion completes', async function () { const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); diff --git a/src/test/mocks/queryProvider.ts b/src/test/mocks/queryProvider.ts index 3f9509dfb4..d2a0805873 100644 --- a/src/test/mocks/queryProvider.ts +++ b/src/test/mocks/queryProvider.ts @@ -73,9 +73,10 @@ export class QueryProvider { } } - expectOctokitRequest(accessorPath: string[], args: any[], response: R) { + expectOctokitRequest(accessorPath: string[], args: any[], response: R, status?: number) { this.getOctokitRequestStub(accessorPath).withArgs(...args).resolves({ data: response, + status, headers: { 'x-ratelimit-limit': '5000', 'x-ratelimit-remaining': '4999' }, }); } diff --git a/src/test/view/prsTree.test.ts b/src/test/view/prsTree.test.ts index 85f44fb28d..6e4de4302a 100644 --- a/src/test/view/prsTree.test.ts +++ b/src/test/view/prsTree.test.ts @@ -22,17 +22,20 @@ import { mockTreeViewWorkbench } from '../mocks/mockTreeViewWorkbench'; import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; import { PullRequestGitHelper } from '../../github/pullRequestGitHelper'; import { PullRequestModel } from '../../github/pullRequestModel'; +import { PullRequestOverviewPanel } from '../../github/pullRequestOverview'; +import { convertRESTPullRequestToRawPullRequest, parseGraphQLPullRequest } from '../../github/utils'; +import { PullRequestBuilder } from '../builders/rest/pullRequestBuilder'; +import { PRNode } from '../../view/treeNodes/pullRequestNode'; import { GitHubRemote } from '../../common/remote'; import { Protocol } from '../../common/protocol'; import { CredentialStore, GitHub } from '../../github/credentials'; -import { parseGraphQLPullRequest } from '../../github/utils'; import { GitApiImpl } from '../../api/api1'; import { RepositoriesManager } from '../../github/repositoriesManager'; import { LoggingApolloClient, LoggingOctokit, RateLogger } from '../../github/loggingOctokit'; import { AuthProvider, GitHubServerType } from '../../common/authentication'; import * as configuration from '../../authentication/configuration'; import { DataUri } from '../../common/uri'; -import { IAccount, ITeam } from '../../github/interface'; +import { GithubItemStateEnum, IAccount, ITeam, PullRequestMergeability } from '../../github/interface'; import { asPromise } from '../../common/utils'; import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; @@ -130,6 +133,52 @@ describe('GitHub Pull Requests view', function () { assert.strictEqual(options.canSelectMany, true); }); + it('refreshes selected and existing stack PR panels after adding from the tree', async function () { + const url = 'https://github.com/aaa/bbb'; + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + const repository = new MockGitHubRepository(remote, credentialStore, telemetry, sinon); + try { + const makePR = (number: number, base: string, head: string) => { + const rest = new PullRequestBuilder().number(number) + .base(ref => ref.ref(base)).head(ref => ref.ref(head)).build(); + for (const ref of [rest.base, rest.head]) { + ref.repo.owner.login = remote.owner; + ref.repo.name = remote.repositoryName; + ref.repo.clone_url = `${url}.git`; + } + return new PullRequestModel(credentialStore, telemetry, repository, remote, + convertRESTPullRequestToRawPullRequest(rest, repository)); + }; + const bottom = makePR(1, 'main', 'D1'); + const top = makePR(2, 'D1', 'D2'); + const node = (model: PullRequestModel) => Object.assign(Object.create(PRNode.prototype), { pullRequestModel: model }) as PRNode; + const selected = [node(bottom), node(top)]; + const existing = { + position: 2, size: 2, base: 'main', + pullRequests: [ + { position: 1, number: 10, title: 'Existing', url, head: 'main', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Unknown }, + { position: 2, number: 1, title: 'Bottom', url, head: 'D1', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Unknown }, + ], + }; + sinon.stub(bottom, 'getStack').resolves(existing); + sinon.stub(top, 'getStack').resolves(undefined); + sinon.stub(repository, 'getStackCandidate').resolves({ parentPullRequestNumber: 1, stackNumber: 10, size: 2, url }); + sinon.stub(repository, 'getPullRequest').callsFake(async number => number === 1 ? bottom : top); + const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); + const confirm = sinon.stub(vscode.window, 'showInformationMessage'); + confirm.onFirstCall().resolves('Add to Stack' as never); + confirm.onSecondCall().resolves(undefined); + const refresh = sinon.stub(PullRequestOverviewPanel, 'refreshStackPanels').resolves(); + + await (provider as any).addSelectedPullRequestsToStack(selected[0], selected); + + assert(add.calledOnce); + assert(refresh.calledOnceWithExactly(remote.owner, remote.repositoryName, [10, 1, 2])); + } finally { + repository.dispose(); + } + }); + it('has no children when no GitHub remotes are available', async function () { sinon .stub(vscode.workspace, 'workspaceFolders') diff --git a/src/view/prsTreeDataProvider.ts b/src/view/prsTreeDataProvider.ts index 6c5a814a06..9b3c737e8d 100644 --- a/src/view/prsTreeDataProvider.ts +++ b/src/view/prsTreeDataProvider.ts @@ -266,8 +266,14 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T if (approved !== confirmation.action) { return; } - await addPullRequestsToStack(ordered, candidate); + const existingStack = candidate.stackNumber !== undefined ? await bottom.getStack() : undefined; + if (candidate.stackNumber !== undefined && !existingStack) { + throw new Error(`Unable to load the existing stack for pull request #${bottom.number}. Refresh the view and try again.`); + } + const added = await addPullRequestsToStack(ordered, candidate); this.refreshAll(true); + await PullRequestOverviewPanel.refreshStackPanels(bottom.remote.owner, bottom.remote.repositoryName, + [...new Set([...(existingStack?.pullRequests.map(pr => pr.number) ?? []), ...added])]); void vscode.window.showInformationMessage(vscode.l10n.t('Pull requests added to the stack.')); } catch (error) { Logger.error(`Failed to add pull requests to stack: ${formatError(error)}`, PullRequestsTreeDataProvider.name); diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index 420a05da7a..1ec3651548 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -11,7 +11,7 @@ import { CloseResult, DescriptionResult, OpenCommitChangesArgs, OpenLocalFileArg import { IComment } from '../../src/common/comment'; import { EventType, ReviewEvent, SessionLinkInfo, TimelineEvent } from '../../src/common/timelineEvent'; import { IProjectItem, MergeMethod, PullRequestCheckStatus, ReadyForReview } from '../../src/github/interface'; -import { CancelCodingAgentReply, ChangeAssigneesReply, ChangeBaseReply, ConvertToDraftReply, DeleteReviewResult, FileUploadCompletedMessage, MergeArguments, MergeResult, ProjectItemsReply, PullRequest, PullRequestPreview, ReadyForReviewReply, StackMergeResult, SubmitReviewArgs, SubmitReviewReply, UploadFilesReply } from '../../src/github/views'; +import { CancelCodingAgentReply, ChangeAssigneesReply, ChangeBaseReply, ConvertToDraftReply, DeleteReviewResult, FileUploadCompletedMessage, MergeArguments, MergeResult, ProjectItemsReply, PullRequest, PullRequestPreview, ReadyForReviewReply, StackMergeResult, SubmitReviewArgs, SubmitReviewReply, UnstackAllResult, UploadFilesReply } from '../../src/github/views'; /** * Encode a {@linkcode Uint8Array} as a base64 string. Uses fixed-size chunks to @@ -104,6 +104,9 @@ export class PRContext { public mergeStack = (method: MergeMethod): Promise => this.postMessage({ command: 'pr.merge-stack', args: { method } }); + public unstackAll = (): Promise => + this.postMessage({ command: 'pr.unstack-all' }); + public openOnGitHub = () => this.postMessage({ command: 'pr.openOnGitHub', args: this.preview ? { url: this.preview.url } : undefined, diff --git a/webviews/components/pullRequestStack.tsx b/webviews/components/pullRequestStack.tsx index 0c63616719..973886ffaf 100644 --- a/webviews/components/pullRequestStack.tsx +++ b/webviews/components/pullRequestStack.tsx @@ -7,6 +7,7 @@ import * as React from 'react'; import { checkIcon, chevronDownIcon, circleFilledIcon, closeIcon, layersIcon, warningIcon } from './icon'; import { GithubItemStateEnum, PullRequestMergeability, PullRequestStack as Stack } from '../../src/github/interface'; import { PullRequest } from '../../src/github/views'; +import PullRequestContext from '../common/context'; function getReadiness(entry: Stack['pullRequests'][number]): { icon: JSX.Element; label: string; kind: string } { if (entry.state === GithubItemStateEnum.Merged) { @@ -40,17 +41,35 @@ export const StackBadge = ({ stack }: { stack?: Stack }) => stack ? ( ) : null; export const StackSection = ({ pr }: { pr: PullRequest }) => { + const { unstackAll } = React.useContext(PullRequestContext); + const [busy, setBusy] = React.useState(false); + const [error, setError] = React.useState(); const { stack } = pr; if (!stack) { return null; } const openBelow = stack.pullRequests.filter(entry => entry.position < stack.position && entry.state === GithubItemStateEnum.Open).length; + const canUnstack = pr.hasWritePermission && stack.pullRequests.some(entry => entry.state !== GithubItemStateEnum.Merged); + + const unstack = async (event: React.MouseEvent) => { + event.preventDefault(); + event.stopPropagation(); + try { + setBusy(true); + setError(undefined); + await unstackAll(); + } catch (unstackError) { + setError(unstackError instanceof Error ? unstackError.message || unstackError.name : String(unstackError)); + } finally { + setBusy(false); + } + }; return ( -
+
{layersIcon} - + Pull request stack {pr.state === GithubItemStateEnum.Open && openBelow > 0 @@ -58,8 +77,15 @@ export const StackSection = ({ pr }: { pr: PullRequest }) => { : `${stack.size} pull requests in this stack.`} + {canUnstack ? + + : null} {chevronDownIcon} + {error ?
Unable to unstack pull requests: {error}
: null}
    {[...stack.pullRequests].reverse().map(entry => { const current = entry.number === pr.number; diff --git a/webviews/editorWebview/index.css b/webviews/editorWebview/index.css index 405ba06864..9992a0ddea 100644 --- a/webviews/editorWebview/index.css +++ b/webviews/editorWebview/index.css @@ -705,6 +705,11 @@ body button .icon { scroll-margin-top: 64px; } +.stack-heading-text { + flex: 1; + min-width: 0; +} + .stack-section summary > .icon { flex-shrink: 0; } @@ -713,6 +718,10 @@ body button .icon { margin-left: auto; } +.stack-section.has-actions .stack-chevron { + margin-left: 0; +} + .stack-section:not([open]) .stack-chevron { transform: rotate(-90deg); } @@ -809,6 +818,29 @@ body button .icon { width: fit-content; } +.stack-actions { + margin-left: auto; +} + +.stack-section:not([open]) .stack-actions { + display: none; +} + +@media (max-width: 480px) { + .stack-section[open].has-actions summary { + flex-wrap: wrap; + } + + .stack-section[open].has-actions .stack-heading-text { + flex-basis: calc(100% - 24px); + } +} + +.stack-unstack-error { + padding: 8px 16px 0; + color: var(--vscode-errorForeground); +} + .subtitle .avatar, .subtitle .avatar-icon svg { margin-top: 2px; diff --git a/webviews/editorWebview/test/overview.test.tsx b/webviews/editorWebview/test/overview.test.tsx index 79702ad7da..0d0cd3c936 100644 --- a/webviews/editorWebview/test/overview.test.tsx +++ b/webviews/editorWebview/test/overview.test.tsx @@ -139,6 +139,66 @@ describe('Overview', function () { assert(out.getByText('Merge Pull Request')); }); + it('offers Unstack all for an eligible stack without showing it to users without write permission', function () { + const stack = { + position: 2, size: 2, base: 'main', + pullRequests: [ + { position: 1, number: 794, title: 'First', head: 'D1', url: 'https://example.com/794', state: GithubItemStateEnum.Merged, isDraft: false, mergeable: PullRequestMergeability.Unknown }, + { position: 2, number: 795, title: 'Second', head: 'D2', url: 'https://example.com/795', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, + ], + }; + const pr = new PullRequestBuilder().number(795).stack(stack).build(); + const context = new PRContext(pr); + const unstackAll = sinon.stub(context, 'unstackAll').resolves({ cancelled: false, remainingPullRequests: [794] }); + const out = render( + + + , + ); + const button = out.getByText('Unstack all'); + const section = button.closest('#pull-request-stack'); + assert(section); + assert.strictEqual(section.children[0].tagName, 'SUMMARY'); + assert.strictEqual(section.children[0], button.parentElement?.parentElement); + assert.strictEqual(button.parentElement?.nextElementSibling?.className, 'stack-chevron'); + assert.strictEqual(fireEvent.click(button), false); + assert(unstackAll.calledOnce); + assert(section.hasAttribute('open')); + + out.rerender( + + + , + ); + assert.strictEqual(out.queryByText('Unstack all'), null); + out.rerender( + + + , + ); + assert.strictEqual(out.queryByText('Unstack all'), null); + }); + + it('shows an inline error when unstacking fails', async function () { + const pr = new PullRequestBuilder().stack({ + position: 1, size: 1, base: 'main', + pullRequests: [ + { position: 1, number: 1234, title: 'First', head: 'D1', url: 'https://example.com/1234', state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, + ], + }).build(); + const context = new PRContext(pr); + sinon.stub(context, 'unstackAll').rejects(new Error('Stack is locked')); + const out = render( + + + , + ); + fireEvent.click(out.getByText('Unstack all')); + const alert = await waitForElement(() => out.container.querySelector('.stack-unstack-error[role="alert"]')); + assert.strictEqual(alert?.textContent, 'Unable to unstack pull requests: Stack is locked'); + assert.strictEqual((out.getByText('Unstack all') as HTMLButtonElement).disabled, false); + }); + it('shows a closed stack without suggesting it can be merged', function () { const pr = new PullRequestBuilder().state(GithubItemStateEnum.Closed).stack({ position: 1, From 90ec9ccd9ea936ad50d80b71e3d41ee0be6bb432 Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Fri, 2 Oct 2026 18:38:55 +0200 Subject: [PATCH 2/2] CCR --- src/github/githubRepository.ts | 6 ++- src/github/pullRequestOverview.ts | 9 +++-- src/test/github/pullRequestModel.test.ts | 27 +++++++++++--- src/test/github/pullRequestOverview.test.ts | 37 ++++++++++++++++++- webviews/components/pullRequestStack.tsx | 16 ++++---- webviews/editorWebview/index.css | 20 ++++++---- webviews/editorWebview/test/overview.test.tsx | 7 ++-- 7 files changed, 90 insertions(+), 32 deletions(-) diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index b2f1954da7..9661a8709a 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -909,7 +909,7 @@ export class GitHubRepository extends Disposable { } } - async unstackAll(pullRequestNumber: number): Promise { + async unstackAll(pullRequestNumber: number, expectedPullRequests: readonly number[]): Promise { const { octokit, remote } = await this.ensure(); const params = { owner: remote.owner, @@ -926,6 +926,10 @@ export class GitHubRepository extends Disposable { || !stacks[0].pull_requests.some((pr: unknown) => isObject(pr) && pr.number === pullRequestNumber)) { throw new Error(`Could not find the stack containing pull request #${pullRequestNumber}.`); } + if (stacks[0].pull_requests.length !== expectedPullRequests.length + || stacks[0].pull_requests.some((pr: unknown, index: number) => !isObject(pr) || pr.number !== expectedPullRequests[index])) { + throw new Error('The pull request stack has changed. Refresh the view and try again.'); + } const result = await octokit.call(() => octokit.api.request('POST /repos/{owner}/{repo}/stacks/{stack_number}/unstack', { ...params, stack_number: stacks[0].number, diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 06dc25f20e..916796f792 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -1125,12 +1125,13 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel pr.state !== GithubItemStateEnum.Merged)) { throw new Error(vscode.l10n.t('No unmerged pull requests are available to unstack.')); } + const expectedPullRequests = stack.pullRequests.map(pr => pr.number); const action = vscode.l10n.t('Unstack all'); const answer = await vscode.window.showWarningMessage( vscode.l10n.t('Unstack all eligible pull requests?'), { modal: true, - detail: vscode.l10n.t('Open, draft, and closed pull requests will be removed from this stack. Their base branches will not change. Merged and queued pull requests will remain in the stack.'), + detail: vscode.l10n.t('Eligible open, draft, and closed pull requests will be removed from this stack. Their base branches will not change. Merged, queued, and currently merging pull requests will remain in the stack.'), }, action, ); @@ -1138,14 +1139,14 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel pr.number)); if (remainingPullRequests.length === stack.size) { - void vscode.window.showInformationMessage(vscode.l10n.t('No pull requests were unstacked. Merged or queued pull requests remain in the stack.')); + void vscode.window.showInformationMessage(vscode.l10n.t('No pull requests were unstacked. Merged, queued, or currently merging pull requests remain in the stack.')); } else { - void vscode.window.showInformationMessage(vscode.l10n.t('Eligible pull requests unstacked. {0} merged or queued pull requests remain in the stack.', remainingPullRequests.length)); + void vscode.window.showInformationMessage(vscode.l10n.t('Eligible pull requests unstacked. {0} merged, queued, or currently merging pull requests remain in the stack.', remainingPullRequests.length)); } } catch (error) { Logger.error(`Failed to unstack pull requests: ${formatError(error)}`, PullRequestOverviewPanel.ID); diff --git a/src/test/github/pullRequestModel.test.ts b/src/test/github/pullRequestModel.test.ts index 7729c74497..98c12c3de5 100644 --- a/src/test/github/pullRequestModel.test.ts +++ b/src/test/github/pullRequestModel.test.ts @@ -626,10 +626,10 @@ describe('PullRequestModel', function () { }]); repo.queryProvider.expectOctokitRequest(['request'], unstackArgs, undefined, 204); - assert.deepStrictEqual(await repo.unstackAll(795), []); + assert.deepStrictEqual(await repo.unstackAll(795, [794, 795]), []); }); - it('reports merged or queued PRs remaining after unstacking', async function () { + it('reports locked PRs remaining after unstacking', async function () { repo.queryProvider.expectOctokitRequest(['request'], listArgs, [{ number: 12, pull_requests: [{ number: 794 }, { number: 795 }], }]); @@ -637,22 +637,37 @@ describe('PullRequestModel', function () { number: 12, pull_requests: [{ number: 794 }], }, 200); - assert.deepStrictEqual(await repo.unstackAll(795), [794]); + assert.deepStrictEqual(await repo.unstackAll(795, [794, 795]), [794]); }); it('does not unstack a different or missing stack', async function () { repo.queryProvider.expectOctokitRequest(['request'], listArgs, [{ number: 12, pull_requests: [{ number: 794 }], }]); - await assert.rejects(repo.unstackAll(795), /Could not find the stack/); + await assert.rejects(repo.unstackAll(795, [794, 795]), /Could not find the stack/); }); + for (const { description, numbers } of [ + { description: 'a different stack', numbers: [796, 795] }, + { description: 'an added PR', numbers: [794, 795, 796] }, + { description: 'a removed PR', numbers: [795] }, + { description: 'reordered PRs', numbers: [795, 794] }, + ]) { + it(`rejects ${description} after confirmation before unstacking`, async function () { + repo.queryProvider.expectOctokitRequest(['request'], listArgs, [{ + number: 13, pull_requests: numbers.map(number => ({ number })), + }]); + + await assert.rejects(repo.unstackAll(795, [794, 795]), /stack has changed/); + }); + } + it('reports an invalid successful response rather than assuming the stack dissolved', async function () { repo.queryProvider.expectOctokitRequest(['request'], listArgs, [{ number: 12, pull_requests: [{ number: 795 }], }]); repo.queryProvider.expectOctokitRequest(['request'], unstackArgs, { pull_requests: null }, 200); - await assert.rejects(repo.unstackAll(795), /invalid result/); + await assert.rejects(repo.unstackAll(795, [795]), /invalid result/); }); it('surfaces an unstack failure rather than reporting success', async function () { @@ -661,7 +676,7 @@ describe('PullRequestModel', function () { }]); repo.queryProvider.expectOctokitError(['request'], unstackArgs, new Error('Stack is locked')); - await assert.rejects(repo.unstackAll(795), /Stack is locked/); + await assert.rejects(repo.unstackAll(795, [795]), /Stack is locked/); }); }); diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index 3274ffa9cc..8bd92facac 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -830,14 +830,47 @@ describe('PullRequestOverview', function () { await (panel as any).unstackAll(message); assert.strictEqual((confirm.firstCall.args[1] as vscode.MessageOptions).modal, true); - assert.match((confirm.firstCall.args[1] as vscode.MessageOptions).detail!, /Merged and queued pull requests will remain/); - assert(unstack.calledOnceWithExactly(1000)); + assert.match((confirm.firstCall.args[1] as vscode.MessageOptions).detail!, /Eligible open, draft, and closed pull requests/); + assert.match((confirm.firstCall.args[1] as vscode.MessageOptions).detail!, /Merged, queued, and currently merging pull requests will remain/); + assert(unstack.calledOnceWithExactly(1000, [999, 1000])); sinon.assert.calledWithExactly(reply, message, { cancelled: false, remainingPullRequests: [999] }); assert(refresh.calledOnce); assert(information.calledOnce); + assert.match(information.firstCall.args[0], /1 merged, queued, or currently merging pull requests remain/); sinon.assert.callOrder(unstack, reply, refresh); }); + it('keeps the confirmed membership when stack data changes while the modal is open', async function () { + const { panel, stackQuery } = await createPanel(); + const stack = await stackQuery(); + assert(stack); + sinon.stub(vscode.window, 'showWarningMessage').callsFake(async (_message, _options, action) => { + stack.pullRequests[0].number = 998; + return action; + }); + sinon.stub(vscode.window, 'showInformationMessage').resolves(undefined); + const unstack = sinon.stub(repo, 'unstackAll').resolves([999]); + sinon.stub(panel as any, '_replyMessage').resolves(); + sinon.stub(panel, 'refreshPanel').resolves(); + + await (panel as any).unstackAll({ req: '6', command: 'pr.unstack-all', args: undefined }); + + assert(unstack.calledOnceWithExactly(1000, [999, 1000])); + }); + + it('explains that currently merging PRs may remain when nothing is unstacked', async function () { + const { panel } = await createPanel(); + sinon.stub(vscode.window, 'showWarningMessage').resolves('Unstack all' as never); + const information = sinon.stub(vscode.window, 'showInformationMessage').resolves(undefined); + sinon.stub(repo, 'unstackAll').resolves([999, 1000]); + sinon.stub(panel as any, '_replyMessage').resolves(); + sinon.stub(panel, 'refreshPanel').resolves(); + + await (panel as any).unstackAll({ req: '7', command: 'pr.unstack-all', args: undefined }); + + assert.match(information.firstCall.args[0], /No pull requests were unstacked.*currently merging pull requests remain/); + }); + it('refreshes other visible PR panels in the unstacked stack', async function () { const { panel } = await createPanel(); const siblingModel = new PullRequestModel(credentialStore, telemetry, repo, remote, diff --git a/webviews/components/pullRequestStack.tsx b/webviews/components/pullRequestStack.tsx index 973886ffaf..219e30e9a6 100644 --- a/webviews/components/pullRequestStack.tsx +++ b/webviews/components/pullRequestStack.tsx @@ -51,9 +51,7 @@ export const StackSection = ({ pr }: { pr: PullRequest }) => { const openBelow = stack.pullRequests.filter(entry => entry.position < stack.position && entry.state === GithubItemStateEnum.Open).length; const canUnstack = pr.hasWritePermission && stack.pullRequests.some(entry => entry.state !== GithubItemStateEnum.Merged); - const unstack = async (event: React.MouseEvent) => { - event.preventDefault(); - event.stopPropagation(); + const unstack = async () => { try { setBusy(true); setError(undefined); @@ -77,14 +75,14 @@ export const StackSection = ({ pr }: { pr: PullRequest }) => { : `${stack.size} pull requests in this stack.`} - {canUnstack ? - - : null} {chevronDownIcon} + {canUnstack ?
    + +
    : null} {error ?
    Unable to unstack pull requests: {error}
    : null}
      {[...stack.pullRequests].reverse().map(entry => { diff --git a/webviews/editorWebview/index.css b/webviews/editorWebview/index.css index 9992a0ddea..fa0489db81 100644 --- a/webviews/editorWebview/index.css +++ b/webviews/editorWebview/index.css @@ -702,6 +702,7 @@ body button .icon { } .stack-section { + position: relative; scroll-margin-top: 64px; } @@ -718,8 +719,8 @@ body button .icon { margin-left: auto; } -.stack-section.has-actions .stack-chevron { - margin-left: 0; +.stack-section[open].has-actions .stack-heading-text { + padding-right: 128px; } .stack-section:not([open]) .stack-chevron { @@ -819,7 +820,9 @@ body button .icon { } .stack-actions { - margin-left: auto; + position: absolute; + top: 16px; + right: 48px; } .stack-section:not([open]) .stack-actions { @@ -827,12 +830,15 @@ body button .icon { } @media (max-width: 480px) { - .stack-section[open].has-actions summary { - flex-wrap: wrap; + .stack-section[open].has-actions .stack-heading-text { + padding-right: 0; } - .stack-section[open].has-actions .stack-heading-text { - flex-basis: calc(100% - 24px); + .stack-actions { + position: static; + display: flex; + justify-content: flex-end; + padding: 0 16px 8px; } } diff --git a/webviews/editorWebview/test/overview.test.tsx b/webviews/editorWebview/test/overview.test.tsx index 0d0cd3c936..bd0a624280 100644 --- a/webviews/editorWebview/test/overview.test.tsx +++ b/webviews/editorWebview/test/overview.test.tsx @@ -159,9 +159,10 @@ describe('Overview', function () { const section = button.closest('#pull-request-stack'); assert(section); assert.strictEqual(section.children[0].tagName, 'SUMMARY'); - assert.strictEqual(section.children[0], button.parentElement?.parentElement); - assert.strictEqual(button.parentElement?.nextElementSibling?.className, 'stack-chevron'); - assert.strictEqual(fireEvent.click(button), false); + assert.strictEqual(button.closest('summary'), null); + assert.strictEqual(section.children[1], button.parentElement); + assert.strictEqual(section.children[0].lastElementChild?.className, 'stack-chevron'); + assert.strictEqual(fireEvent.click(button), true); assert(unstackAll.calledOnce); assert(section.hasAttribute('open'));