From 62dbfe1ec9654944d265c5cf5714fb9cea213ebd Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Thu, 1 Oct 2026 11:24:23 +0200 Subject: [PATCH 1/2] Add multi-select + right click to add to stack in PRs view --- package.json | 14 ++ package.nls.json | 1 + src/common/executeCommands.ts | 1 + src/github/githubRepository.ts | 11 +- src/github/pullRequestStack.ts | 88 ++++++++++++ src/test/github/pullRequestModel.test.ts | 18 +++ src/test/github/pullRequestStack.test.ts | 174 +++++++++++++++++++++++ src/test/view/prsTree.test.ts | 7 + src/view/prsTreeDataProvider.ts | 62 ++++++++ src/view/treeNodes/pullRequestNode.ts | 4 +- 10 files changed, 377 insertions(+), 3 deletions(-) create mode 100644 src/github/pullRequestStack.ts create mode 100644 src/test/github/pullRequestStack.test.ts diff --git a/package.json b/package.json index 25dc757d47..25c52d2a28 100644 --- a/package.json +++ b/package.json @@ -1134,6 +1134,11 @@ "title": "%command.pr.refreshPullRequest.title%", "category": "%command.pull.request.category%" }, + { + "command": "pr.addToStack", + "title": "%command.pr.addToStack.title%", + "category": "%command.pull.request.category%" + }, { "command": "pr.openFileOnGitHub", "title": "%command.pr.openFileOnGitHub.title%", @@ -2256,6 +2261,10 @@ "command": "pr.openPullRequestOnGitHub", "when": "(gitHubOpenRepositoryCount != 0 && github:inReviewMode) && !isSessionsWindow" }, + { + "command": "pr.addToStack", + "when": "false" + }, { "command": "pr.openAllDiffs", "when": "(gitHubOpenRepositoryCount != 0 && github:inReviewMode) && !isSessionsWindow" @@ -2986,6 +2995,11 @@ } ], "view/item/context": [ + { + "command": "pr.addToStack", + "when": "(view == pr:github && viewItem =~ /pullrequest.*:stackable/ && github:canAddToStack) && !isSessionsWindow", + "group": "0_stack@1" + }, { "command": "pr.pick", "when": "(view == pr:github && viewItem =~ /(pullrequest(:local)?:nonactive)/) && !isSessionsWindow", diff --git a/package.nls.json b/package.nls.json index d546dacf47..2004a852e7 100644 --- a/package.nls.json +++ b/package.nls.json @@ -236,6 +236,7 @@ "command.pr.openPullRequestOnGitHub.title": "Open Pull Request on GitHub", "command.pr.openAllDiffs.title": "Open All Diffs", "command.pr.refreshPullRequest.title": "Refresh Pull Request", + "command.pr.addToStack.title": "Add to Stack", "command.pr.openFileOnGitHub.title": "Open File on GitHub", "command.pr.revealFileInOS.title": "Reveal in File Explorer", "command.pr.copyCommitHash.title": "Copy Commit Hash", diff --git a/src/common/executeCommands.ts b/src/common/executeCommands.ts index 46d315300f..f755e7c8ce 100644 --- a/src/common/executeCommands.ts +++ b/src/common/executeCommands.ts @@ -16,6 +16,7 @@ export namespace contexts { export const LOADING_ISSUES_TREE = 'github:loadingIssuesTree'; export const HAS_ENTERPRISE_URIS = 'github:hasEnterpriseUris'; export const CREATE_PR_PERMISSIONS = 'github:createPrPermissions'; + export const CAN_ADD_TO_STACK = 'github:canAddToStack'; export const RESOLVING_CONFLICTS = 'github:resolvingConflicts'; export const PULL_REQUEST_DESCRIPTION_VISIBLE = 'github:pullRequestDescriptionVisible'; // Boolean indicating if the pull request description is visible export const ACTIVE_COMMENT_HAS_SUGGESTION = 'github:activeCommentHasSuggestion'; // Boolean indicating if the active comment has a suggestion diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index b6b9e3f22c..40d9bd9d8b 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -881,6 +881,13 @@ export class GitHubRepository extends Disposable { } async addPullRequestToStack(candidate: StackCandidate, number: number): Promise { + return this.addPullRequestsToStack(candidate, [number]); + } + + async addPullRequestsToStack(candidate: StackCandidate, numbers: number[]): Promise { + if (numbers.length === 0) { + throw new Error('At least one pull request is required to add to a stack.'); + } const { octokit, remote } = await this.ensure(); const params = { owner: remote.owner, @@ -892,12 +899,12 @@ export class GitHubRepository extends Disposable { await octokit.call(() => octokit.api.request('POST /repos/{owner}/{repo}/stacks/{stack_number}/add', { ...params, stack_number: stackNumber, - pull_requests: [number], + pull_requests: numbers, })); } else { await octokit.call(() => octokit.api.request('POST /repos/{owner}/{repo}/stacks', { ...params, - pull_requests: [candidate.parentPullRequestNumber, number], + pull_requests: [candidate.parentPullRequestNumber, ...numbers], })); } } diff --git a/src/github/pullRequestStack.ts b/src/github/pullRequestStack.ts new file mode 100644 index 0000000000..7342441762 --- /dev/null +++ b/src/github/pullRequestStack.ts @@ -0,0 +1,88 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { GithubItemStateEnum } from './interface'; +import { PullRequestModel } from './pullRequestModel'; +import { compareIgnoreCase } from '../common/utils'; + +function sameRepository(first: PullRequestModel, second: PullRequestModel): boolean { + return compareIgnoreCase(first.remote.owner, second.remote.owner) === 0 + && compareIgnoreCase(first.remote.repositoryName, second.remote.repositoryName) === 0 + && compareIgnoreCase(first.githubRepository.remote.normalizedHost, second.githubRepository.remote.normalizedHost) === 0; +} + +export function isStackablePullRequest(pullRequest: PullRequestModel): boolean { + const { base, head } = pullRequest; + if (pullRequest.state !== GithubItemStateEnum.Open || !head || !base || head.ref === base.ref) { + return false; + } + const repository = pullRequest.githubRepository.remote; + return [base, head].every(ref => compareIgnoreCase(ref.owner, repository.owner) === 0 + && compareIgnoreCase(ref.repositoryCloneUrl.repositoryName, repository.repositoryName) === 0 + && compareIgnoreCase(ref.repositoryCloneUrl.host, repository.gitProtocol.host) === 0); +} + +export function orderStackablePullRequests(pullRequests: readonly PullRequestModel[]): PullRequestModel[] | undefined { + if (pullRequests.length < 2 || pullRequests.some(pr => !isStackablePullRequest(pr) || !sameRepository(pr, pullRequests[0]))) { + return; + } + const byHead = new Map(); + const byBase = new Map(); + const numbers = new Set(); + for (const pr of pullRequests) { + if (byHead.has(pr.head!.ref) || byBase.has(pr.base.ref) || numbers.has(pr.number)) { + return; + } + byHead.set(pr.head!.ref, pr); + byBase.set(pr.base.ref, pr); + numbers.add(pr.number); + } + const bottoms = pullRequests.filter(pr => !byHead.has(pr.base.ref)); + if (bottoms.length !== 1) { + return; + } + const ordered: PullRequestModel[] = []; + let current: PullRequestModel | undefined = bottoms[0]; + while (current && ordered.length < pullRequests.length) { + ordered.push(current); + current = byBase.get(current.head!.ref); + } + return ordered.length === pullRequests.length && !current ? ordered : undefined; +} + +export async function addPullRequestsToStack(pullRequests: readonly PullRequestModel[]): Promise { + const initial = orderStackablePullRequests(pullRequests); + if (!initial) { + throw new Error('Select two or more open pull requests whose head and base branches form a chain in the same repository.'); + } + const selectedBranches = initial.map(pr => ({ number: pr.number, base: pr.base.ref, head: pr.head!.ref })); + const repository = initial[0].githubRepository; + const refreshed = await Promise.all(initial.map(async pr => { + const current = await repository.getPullRequest(pr.number, 'addPullRequestsToStack'); + if (!current) { + throw new Error(`Unable to refresh pull request #${pr.number} before creating a stack.`); + } + return current; + })); + const ordered = orderStackablePullRequests(refreshed); + if (!ordered || ordered.some((pr, index) => + pr.number !== selectedBranches[index].number + || pr.base.ref !== selectedBranches[index].base + || pr.head?.ref !== selectedBranches[index].head)) { + throw new Error('The selected pull request branches have changed. Refresh the view and try again.'); + } + const bottom = ordered[0]; + const candidate = await repository.getStackCandidate(bottom.head!.ref); + if (!candidate || candidate.parentPullRequestNumber !== bottom.number) { + throw new Error(`Pull request #${bottom.number} is no longer eligible to start or extend a stack.`); + } + for (const pr of ordered.slice(1)) { + if (await pr.getStack()) { + throw new Error(`Pull request #${pr.number} is already in a stack.`); + } + } + await repository.addPullRequestsToStack(candidate, ordered.slice(1).map(pr => pr.number)); + return ordered.map(pr => pr.number); +} diff --git a/src/test/github/pullRequestModel.test.ts b/src/test/github/pullRequestModel.test.ts index 430a8d2bca..c040a344b0 100644 --- a/src/test/github/pullRequestModel.test.ts +++ b/src/test/github/pullRequestModel.test.ts @@ -467,6 +467,24 @@ describe('PullRequestModel', function () { await repo.addPullRequestToStack(candidate, 796); }); + it('creates a stack from multiple existing pull requests in branch order', async function () { + const candidate = { parentPullRequestNumber: 795, size: 1, url: 'https://github.com/github/test/pull/795' }; + repo.queryProvider.expectOctokitRequest(['request'], ['POST /repos/{owner}/{repo}/stacks', { + owner: 'github', repo: 'test', headers: listParams.headers, pull_requests: [795, 796, 797], + }], {}); + + await repo.addPullRequestsToStack(candidate, [796, 797]); + }); + + it('extends a stack with multiple existing pull requests in branch order', async function () { + const candidate = { parentPullRequestNumber: 795, stackNumber: 12, size: 2, url: 'https://github.com/github/test/pull/795' }; + repo.queryProvider.expectOctokitRequest(['request'], ['POST /repos/{owner}/{repo}/stacks/{stack_number}/add', { + owner: 'github', repo: 'test', headers: listParams.headers, stack_number: 12, pull_requests: [796, 797], + }], {}); + + await repo.addPullRequestsToStack(candidate, [796, 797]); + }); + it('finds the parent from its GraphQL head branch before offering a stack', async function () { const parent = new GraphQLPullRequestBuilder().build().repository!.pullRequest; parent.number = 795; diff --git a/src/test/github/pullRequestStack.test.ts b/src/test/github/pullRequestStack.test.ts new file mode 100644 index 0000000000..f8df35ea96 --- /dev/null +++ b/src/test/github/pullRequestStack.test.ts @@ -0,0 +1,174 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { default as assert } from 'assert'; +import { createSandbox, SinonSandbox } from 'sinon'; +import { Protocol } from '../../common/protocol'; +import { GitHubServerType } from '../../common/authentication'; +import { GitHubRemote } from '../../common/remote'; +import { CredentialStore } from '../../github/credentials'; +import { GithubItemStateEnum } from '../../github/interface'; +import { PullRequestModel } from '../../github/pullRequestModel'; +import { addPullRequestsToStack, isStackablePullRequest, orderStackablePullRequests } from '../../github/pullRequestStack'; +import { convertRESTPullRequestToRawPullRequest } from '../../github/utils'; +import { PullRequestBuilder } from '../builders/rest/pullRequestBuilder'; +import { getAddToStackConfirmation } from '../../view/prsTreeDataProvider'; +import { MockCommandRegistry } from '../mocks/mockCommandRegistry'; +import { MockExtensionContext } from '../mocks/mockExtensionContext'; +import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; +import { MockTelemetry } from '../mocks/mockTelemetry'; + +describe('Pull request stack selection', function () { + let sinon: SinonSandbox; + let context: MockExtensionContext; + let credentials: CredentialStore; + let repository: MockGitHubRepository; + let remote: GitHubRemote; + let telemetry: MockTelemetry; + + beforeEach(function () { + sinon = createSandbox(); + MockCommandRegistry.install(sinon); + context = new MockExtensionContext(); + telemetry = new MockTelemetry(); + credentials = new CredentialStore(telemetry, context); + const url = 'https://github.com/owner/repo'; + remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + repository = new MockGitHubRepository(remote, credentials, telemetry, sinon); + }); + + afterEach(function () { + repository.dispose(); + credentials.dispose(); + context.dispose(); + sinon.restore(); + }); + + function pullRequest(number: number, base: string, head: string, state: 'open' | 'closed' = 'open', headOwner: string = remote.owner): PullRequestModel { + const rest = new PullRequestBuilder().number(number).state(state) + .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 = `https://github.com/${remote.owner}/${remote.repositoryName}.git`; + } + rest.head.repo.owner.login = headOwner; + rest.head.repo.clone_url = `https://github.com/${headOwner}/${remote.repositoryName}.git`; + return new PullRequestModel(credentials, telemetry, repository, remote, convertRESTPullRequestToRawPullRequest(rest, repository)); + } + + it('orders a branch chain from the target base toward the top', function () { + const bottom = pullRequest(1, 'main', 'D1'); + const middle = pullRequest(2, 'D1', 'D2'); + const top = pullRequest(3, 'D2', 'D3'); + assert.deepStrictEqual(orderStackablePullRequests([top, bottom, middle]), [bottom, middle, top]); + }); + + it('distinguishes creating a stack from adding PRs to an existing stack', function () { + const bottom = pullRequest(1, 'main', 'D1'); + const middle = pullRequest(2, 'D1', 'D2'); + const top = pullRequest(3, 'D2', 'D3'); + assert.deepStrictEqual(getAddToStackConfirmation([bottom, middle], { + parentPullRequestNumber: 1, size: 1, url: bottom.html_url, + }), { + message: 'Create a stack with 2 pull requests?', + detail: '#1 New feature\n#2 New feature', + action: 'Create Stack', + }); + assert.deepStrictEqual(getAddToStackConfirmation([bottom, middle], { + parentPullRequestNumber: 1, stackNumber: 10, size: 3, url: bottom.html_url, + }), { + message: 'Add 1 pull request to an existing stack?', + detail: 'Adding #2 New feature\nto #1 New feature', + action: 'Add to Stack', + }); + assert.deepStrictEqual(getAddToStackConfirmation([bottom, middle, top], { + parentPullRequestNumber: 1, stackNumber: 10, size: 3, url: bottom.html_url, + }), { + message: 'Add 2 pull requests to an existing stack?', + detail: 'Adding #2 New feature\n#3 New feature\nto #1 New feature', + action: 'Add to Stack', + }); + }); + + it('hides the action for unrelated, duplicate, closed and forked PRs', function () { + const bottom = pullRequest(1, 'main', 'D1'); + const top = pullRequest(2, 'D1', 'D2'); + assert.strictEqual(orderStackablePullRequests([bottom]) === undefined, true, 'one PR is insufficient'); + assert.strictEqual(orderStackablePullRequests([bottom, bottom]) === undefined, true, 'duplicate PRs are invalid'); + assert.strictEqual(orderStackablePullRequests([bottom, pullRequest(3, 'other', 'D3')]) === undefined, true, 'disconnected branches are invalid'); + assert.strictEqual(orderStackablePullRequests([bottom, top, pullRequest(3, 'D1', 'D2')]) === undefined, true, 'duplicate head branches are invalid'); + assert.strictEqual(orderStackablePullRequests([bottom, pullRequest(3, 'D1', 'D3', 'closed')]) === undefined, true, 'closed PRs are invalid'); + const forked = pullRequest(4, 'D1', 'D4', 'open', 'another'); + assert.strictEqual(isStackablePullRequest(forked), false); + assert.strictEqual(orderStackablePullRequests([bottom, forked]) === undefined, true, 'forked heads are invalid'); + assert.strictEqual(orderStackablePullRequests([bottom, top])?.map(pr => pr.number).join(','), '1,2'); + }); + + it('creates a new stack with selected PRs in branch order after refreshing them', async function () { + const bottom = pullRequest(1, 'main', 'D1'); + const middle = pullRequest(2, 'D1', 'D2'); + const top = pullRequest(3, 'D2', 'D3'); + const refresh = sinon.stub(repository, 'getPullRequest').callsFake(async number => [bottom, middle, top].find(pr => pr.number === number)); + sinon.stub(repository, 'getStackCandidate').resolves({ parentPullRequestNumber: 1, size: 1, url: bottom.html_url }); + sinon.stub(middle, 'getStack').resolves(undefined); + sinon.stub(top, 'getStack').resolves(undefined); + const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); + + assert.deepStrictEqual(await addPullRequestsToStack([top, bottom, middle]), [1, 2, 3]); + assert(refresh.calledThrice); + assert(add.calledOnceWithExactly({ parentPullRequestNumber: 1, size: 1, url: bottom.html_url }, [2, 3])); + }); + + it('appends to an existing stack only when the bottom PR is its top', async function () { + const bottom = pullRequest(1, 'main', 'D1'); + const top = pullRequest(2, 'D1', 'D2'); + sinon.stub(repository, 'getPullRequest').callsFake(async number => [bottom, top].find(pr => pr.number === number)); + const candidate = { parentPullRequestNumber: 1, stackNumber: 10, size: 3, url: bottom.html_url }; + sinon.stub(repository, 'getStackCandidate').resolves(candidate); + sinon.stub(top, 'getStack').resolves(undefined); + const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); + + assert.deepStrictEqual(await addPullRequestsToStack([top, bottom]), [1, 2]); + assert(add.calledOnceWithExactly(candidate, [2])); + }); + + it('rejects stale branch chains and PRs already in another stack before writing', async function () { + const bottom = pullRequest(1, 'main', 'D1'); + const top = pullRequest(2, 'D1', 'D2'); + const moved = pullRequest(2, 'other', 'D2'); + const refresh = sinon.stub(repository, 'getPullRequest').callsFake(async number => number === 1 ? bottom : moved); + const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); + await assert.rejects(addPullRequestsToStack([top, bottom]), /branches have changed/); + assert(add.notCalled); + + refresh.callsFake(async number => number === 1 ? bottom : top); + sinon.stub(repository, 'getStackCandidate').resolves({ parentPullRequestNumber: 1, size: 1, url: bottom.html_url }); + sinon.stub(top, 'getStack').resolves({ + position: 1, size: 1, base: 'main', pullRequests: [{ + position: 1, number: 2, title: top.title, url: top.html_url, head: 'D2', + state: GithubItemStateEnum.Open, isDraft: false, mergeable: top.item.mergeable!, + }], + }); + await assert.rejects(addPullRequestsToStack([bottom, top]), /already in a stack/); + assert(add.notCalled); + }); + + it('rejects a changed head branch when refreshing reuses the selected model', async function () { + const bottom = pullRequest(1, 'main', 'D1'); + const top = pullRequest(2, 'D1', 'D2'); + sinon.stub(repository, 'getPullRequest').callsFake(async number => { + if (number === top.number) { + top.head!.ref = 'D3'; + } + return number === bottom.number ? bottom : top; + }); + const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); + + await assert.rejects(addPullRequestsToStack([bottom, top]), /branches have changed/); + assert(add.notCalled); + }); +}); diff --git a/src/test/view/prsTree.test.ts b/src/test/view/prsTree.test.ts index e9794e7d0b..85f44fb28d 100644 --- a/src/test/view/prsTree.test.ts +++ b/src/test/view/prsTree.test.ts @@ -123,6 +123,13 @@ describe('GitHub Pull Requests view', function () { assert.strictEqual(rootNodes.length, 0); }); + it('allows selecting multiple pull requests to create a stack', function () { + const tree = createTreeView.getCalls().find(call => call.args[0] === 'pr:github'); + assert(tree); + const options = tree.args[1] as { canSelectMany?: boolean }; + assert.strictEqual(options.canSelectMany, true); + }); + 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 a48adf744d..2fe3fb3201 100644 --- a/src/view/prsTreeDataProvider.ts +++ b/src/view/prsTreeDataProvider.ts @@ -7,6 +7,7 @@ import * as vscode from 'vscode'; import { PRStatusDecorationProvider } from './prStatusDecorationProvider'; import { PrsTreeModel } from './prsTreeModel'; import { ReviewModel } from './reviewModel'; +import { StackCandidate } from '../../common/views'; import { getEnterpriseUris } from '../authentication/configuration'; import { AuthProvider } from '../common/authentication'; import { commands, contexts } from '../common/executeCommands'; @@ -24,6 +25,7 @@ import { PRType } from '../github/interface'; import { escapeMarkdownText, issueMarkdown } from '../github/markdownUtils'; import { PullRequestModel } from '../github/pullRequestModel'; import { PullRequestOverviewPanel } from '../github/pullRequestOverview'; +import { addPullRequestsToStack, orderStackablePullRequests } from '../github/pullRequestStack'; import { RepositoriesManager } from '../github/repositoriesManager'; import { CategoryTreeNode, PRCategoryActionNode, PRCategoryActionType } from './treeNodes/categoryNode'; import { InMemFileChangeNode } from './treeNodes/fileChangeNode'; @@ -70,6 +72,20 @@ function enterpriseSettingsMessage(text: string | undefined, actions: { github: return message; } +export function getAddToStackConfirmation(ordered: readonly PullRequestModel[], candidate: StackCandidate): { message: string; detail: string; action: string } { + const existing = candidate.stackNumber !== undefined; + const additions = ordered.slice(1); + const message = existing + ? additions.length === 1 + ? vscode.l10n.t('Add 1 pull request to an existing stack?') + : vscode.l10n.t('Add {0} pull requests to an existing stack?', additions.length) + : vscode.l10n.t('Create a stack with {0} pull requests?', ordered.length); + const detail = existing + ? vscode.l10n.t('Adding {0}\nto #{1}', [additions.map(pr => `#${pr.number} ${pr.title}`).join('\n'), `${ordered[0].number} ${ordered[0].title}`]) + : ordered.map(pr => `#${pr.number} ${pr.title}`).join('\n'); + return { message, detail, action: existing ? vscode.l10n.t('Add to Stack') : vscode.l10n.t('Create Stack') }; +} + export class PullRequestsTreeDataProvider extends Disposable implements vscode.TreeDataProvider, BaseTreeNode { private _onDidChangeTreeData = new vscode.EventEmitter(); readonly onDidChangeTreeData = this._onDidChangeTreeData.event; @@ -115,6 +131,7 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T this._view = this._register(vscode.window.createTreeView('pr:github', { treeDataProvider: this, showCollapseAll: true, + canSelectMany: true, manageCheckboxStateManually: true })); this._loginView = this._register(vscode.window.createTreeView('github:login', { @@ -128,6 +145,17 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T }, })); + void commands.setContext(contexts.CAN_ADD_TO_STACK, false); + this._register(this._view.onDidChangeSelection(e => { + const selectedPRs = e.selection.filter((node): node is PRNode => node instanceof PRNode); + const stackable = selectedPRs.length === e.selection.length + && !!orderStackablePullRequests(selectedPRs.map(node => node.pullRequestModel)); + void commands.setContext(contexts.CAN_ADD_TO_STACK, stackable); + })); + this._register({ dispose: () => { void commands.setContext(contexts.CAN_ADD_TO_STACK, false); } }); + this._register(vscode.commands.registerCommand('pr.addToStack', + (clicked: PRNode, selected: TreeNode[]) => this.addSelectedPullRequestsToStack(clicked, selected))); + this._register(this._view.onDidChangeVisibility(e => { if (e.visible) { // Sync with currently active PR when view becomes visible @@ -213,6 +241,40 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T })); } + private async addSelectedPullRequestsToStack(clicked: PRNode, selected: TreeNode[] | undefined): Promise { + const selection = selected ?? this._view.selection; + if (!(clicked instanceof PRNode) || !Array.isArray(selection) || selection.length < 2 + || !selection.includes(clicked) || !selection.every(node => node instanceof PRNode)) { + void vscode.window.showErrorMessage(vscode.l10n.t('Select at least two pull requests in the Pull Requests view to add them to a stack.')); + return; + } + const ordered = orderStackablePullRequests(selection.map(node => (node as PRNode).pullRequestModel)); + if (!ordered) { + void vscode.window.showErrorMessage(vscode.l10n.t('Selected pull requests must be open and have matching head and base branches in the same repository.')); + return; + } + try { + const bottom = ordered[0]; + const candidate = await bottom.githubRepository.getStackCandidate(bottom.head!.ref); + if (!candidate || candidate.parentPullRequestNumber !== bottom.number) { + throw new Error(`Pull request #${bottom.number} is no longer eligible to start or extend a stack.`); + } + const confirmation = getAddToStackConfirmation(ordered, candidate); + const approved = await vscode.window.showInformationMessage( + confirmation.message, { modal: true, detail: confirmation.detail }, confirmation.action, + ); + if (approved !== confirmation.action) { + return; + } + await addPullRequestsToStack(ordered); + this.refreshAll(true); + 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); + void vscode.window.showErrorMessage(vscode.l10n.t('Unable to add pull requests to stack: {0}', formatError(error))); + } + } + private filterNotificationsToKnown(notifications: PullRequestModel[]): PullRequestModel[] { return notifications.filter(notification => { if (!this.prsTreeModel.hasPullRequest(notification)) { diff --git a/src/view/treeNodes/pullRequestNode.ts b/src/view/treeNodes/pullRequestNode.ts index f9ddf00328..5c0a9f3e2f 100644 --- a/src/view/treeNodes/pullRequestNode.ts +++ b/src/view/treeNodes/pullRequestNode.ts @@ -16,6 +16,7 @@ import { FolderRepositoryManager } from '../../github/folderRepositoryManager'; import { CopilotWorkingStatus } from '../../github/githubRepository'; import { GithubItemStateEnum } from '../../github/interface'; import { IResolvedPullRequestModel, PullRequestModel } from '../../github/pullRequestModel'; +import { isStackablePullRequest } from '../../github/pullRequestStack'; import { InMemFileChangeModel, RemoteFileChangeModel } from '../fileChangeModel'; import { getInMemPRFileSystemProvider, provideDocumentContentForChangeModel } from '../inMemPRContentProvider'; import { getIconForeground, getListErrorForeground, getListWarningForeground, getNotebookStatusSuccessIconForeground } from '../theme'; @@ -388,7 +389,8 @@ export class PRNode extends TreeNode implements vscode.CommentingRangeProvider2 (this._isLocal ? ':local' : '') + (currentBranchIsForThisPR ? ':active' : ':nonactive') + (hasNotification ? ':notification' : '') + - (((this.pullRequestModel.item.isRemoteHeadDeleted && !this._isLocal) || !this._folderReposManager.isPullRequestAssociatedWithOpenRepository(this.pullRequestModel)) ? '' : ':hasHeadRef'), + (((this.pullRequestModel.item.isRemoteHeadDeleted && !this._isLocal) || !this._folderReposManager.isPullRequestAssociatedWithOpenRepository(this.pullRequestModel)) ? '' : ':hasHeadRef') + + (isStackablePullRequest(this.pullRequestModel) ? ':stackable' : ''), iconPath: await this._getIcon(), accessibilityInformation: { label: `${isDraft ? 'Draft ' : ''}Pull request number ${number}: ${title} by ${login}` From 3fcf10f2dbd4265bf5b6007955a1556e2ff52bdc Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Fri, 2 Oct 2026 17:56:22 +0200 Subject: [PATCH 2/2] CCR --- src/github/pullRequestStack.ts | 10 +++- src/test/github/pullRequestStack.test.ts | 74 +++++++++++++++++++++--- src/view/prsTreeDataProvider.ts | 2 +- 3 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/github/pullRequestStack.ts b/src/github/pullRequestStack.ts index 7342441762..b6670374c4 100644 --- a/src/github/pullRequestStack.ts +++ b/src/github/pullRequestStack.ts @@ -5,6 +5,7 @@ import { GithubItemStateEnum } from './interface'; import { PullRequestModel } from './pullRequestModel'; +import { StackCandidate } from '../../common/views'; import { compareIgnoreCase } from '../common/utils'; function sameRepository(first: PullRequestModel, second: PullRequestModel): boolean { @@ -15,7 +16,8 @@ function sameRepository(first: PullRequestModel, second: PullRequestModel): bool export function isStackablePullRequest(pullRequest: PullRequestModel): boolean { const { base, head } = pullRequest; - if (pullRequest.state !== GithubItemStateEnum.Open || !head || !base || head.ref === base.ref) { + if (pullRequest.state !== GithubItemStateEnum.Open || pullRequest.isRemoteHeadDeleted || pullRequest.isRemoteBaseDeleted + || !head || !base || head.ref === base.ref) { return false; } const repository = pullRequest.githubRepository.remote; @@ -52,7 +54,7 @@ export function orderStackablePullRequests(pullRequests: readonly PullRequestMod return ordered.length === pullRequests.length && !current ? ordered : undefined; } -export async function addPullRequestsToStack(pullRequests: readonly PullRequestModel[]): Promise { +export async function addPullRequestsToStack(pullRequests: readonly PullRequestModel[], confirmedCandidate: StackCandidate): Promise { const initial = orderStackablePullRequests(pullRequests); if (!initial) { throw new Error('Select two or more open pull requests whose head and base branches form a chain in the same repository.'); @@ -78,6 +80,10 @@ export async function addPullRequestsToStack(pullRequests: readonly PullRequestM if (!candidate || candidate.parentPullRequestNumber !== bottom.number) { throw new Error(`Pull request #${bottom.number} is no longer eligible to start or extend a stack.`); } + if (candidate.parentPullRequestNumber !== confirmedCandidate.parentPullRequestNumber + || candidate.stackNumber !== confirmedCandidate.stackNumber) { + throw new Error('The selected pull request stack has changed. Refresh the view and try again.'); + } for (const pr of ordered.slice(1)) { if (await pr.getStack()) { throw new Error(`Pull request #${pr.number} is already in a stack.`); diff --git a/src/test/github/pullRequestStack.test.ts b/src/test/github/pullRequestStack.test.ts index f8df35ea96..4435213f33 100644 --- a/src/test/github/pullRequestStack.test.ts +++ b/src/test/github/pullRequestStack.test.ts @@ -5,6 +5,7 @@ import { default as assert } from 'assert'; import { createSandbox, SinonSandbox } from 'sinon'; +import { StackCandidate } from '../../../common/views'; import { Protocol } from '../../common/protocol'; import { GitHubServerType } from '../../common/authentication'; import { GitHubRemote } from '../../common/remote'; @@ -108,17 +109,54 @@ describe('Pull request stack selection', function () { assert.strictEqual(orderStackablePullRequests([bottom, top])?.map(pr => pr.number).join(','), '1,2'); }); + for (const flag of ['isRemoteHeadDeleted', 'isRemoteBaseDeleted'] as const) { + it(`hides the action when ${flag} is set despite retained ref metadata`, function () { + const bottom = pullRequest(1, 'main', 'D1'); + const top = pullRequest(2, 'D1', 'D2'); + for (const pr of [bottom, top]) { + pr[flag] = true; + assert(pr.head && pr.base); + assert.strictEqual(isStackablePullRequest(pr), false); + assert.strictEqual(orderStackablePullRequests([bottom, top]), undefined); + pr[flag] = false; + } + assert.deepStrictEqual(orderStackablePullRequests([top, bottom]), [bottom, top]); + }); + + for (const number of [1, 2]) { + it(`rejects PR #${number} when ${flag} is set during refresh before writing`, async function () { + const bottom = pullRequest(1, 'main', 'D1'); + const top = pullRequest(2, 'D1', 'D2'); + sinon.stub(repository, 'getPullRequest').callsFake(async prNumber => { + const pr = prNumber === bottom.number ? bottom : top; + if (prNumber === number) { + pr[flag] = true; + } + return pr; + }); + const candidate = { parentPullRequestNumber: 1, size: 1, url: bottom.html_url }; + const findCandidate = sinon.stub(repository, 'getStackCandidate').resolves(candidate); + const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); + + await assert.rejects(addPullRequestsToStack([bottom, top], candidate), /branches have changed/); + assert(findCandidate.notCalled); + assert(add.notCalled); + }); + } + } + it('creates a new stack with selected PRs in branch order after refreshing them', async function () { const bottom = pullRequest(1, 'main', 'D1'); const middle = pullRequest(2, 'D1', 'D2'); const top = pullRequest(3, 'D2', 'D3'); const refresh = sinon.stub(repository, 'getPullRequest').callsFake(async number => [bottom, middle, top].find(pr => pr.number === number)); - sinon.stub(repository, 'getStackCandidate').resolves({ parentPullRequestNumber: 1, size: 1, url: bottom.html_url }); + const candidate = { parentPullRequestNumber: 1, size: 1, url: bottom.html_url }; + sinon.stub(repository, 'getStackCandidate').resolves(candidate); sinon.stub(middle, 'getStack').resolves(undefined); sinon.stub(top, 'getStack').resolves(undefined); const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); - assert.deepStrictEqual(await addPullRequestsToStack([top, bottom, middle]), [1, 2, 3]); + assert.deepStrictEqual(await addPullRequestsToStack([top, bottom, middle], candidate), [1, 2, 3]); assert(refresh.calledThrice); assert(add.calledOnceWithExactly({ parentPullRequestNumber: 1, size: 1, url: bottom.html_url }, [2, 3])); }); @@ -132,28 +170,49 @@ describe('Pull request stack selection', function () { sinon.stub(top, 'getStack').resolves(undefined); const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); - assert.deepStrictEqual(await addPullRequestsToStack([top, bottom]), [1, 2]); + assert.deepStrictEqual(await addPullRequestsToStack([top, bottom], candidate), [1, 2]); assert(add.calledOnceWithExactly(candidate, [2])); }); + for (const { confirmedStackNumber, currentStackNumber } of [ + { confirmedStackNumber: undefined, currentStackNumber: 10 }, + { confirmedStackNumber: 10, currentStackNumber: undefined }, + { confirmedStackNumber: 10, currentStackNumber: 11 }, + ]) { + it(`rejects a stack changing from ${confirmedStackNumber} to ${currentStackNumber} after confirmation`, async function () { + const bottom = pullRequest(1, 'main', 'D1'); + const top = pullRequest(2, 'D1', 'D2'); + sinon.stub(repository, 'getPullRequest').callsFake(async number => number === bottom.number ? bottom : top); + const confirmedCandidate: StackCandidate = { + parentPullRequestNumber: bottom.number, stackNumber: confirmedStackNumber, size: 1, url: bottom.html_url, + }; + sinon.stub(repository, 'getStackCandidate').resolves({ ...confirmedCandidate, stackNumber: currentStackNumber }); + const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); + + await assert.rejects(addPullRequestsToStack([bottom, top], confirmedCandidate), /stack has changed/); + assert(add.notCalled); + }); + } + it('rejects stale branch chains and PRs already in another stack before writing', async function () { const bottom = pullRequest(1, 'main', 'D1'); const top = pullRequest(2, 'D1', 'D2'); const moved = pullRequest(2, 'other', 'D2'); const refresh = sinon.stub(repository, 'getPullRequest').callsFake(async number => number === 1 ? bottom : moved); const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); - await assert.rejects(addPullRequestsToStack([top, bottom]), /branches have changed/); + const candidate = { parentPullRequestNumber: 1, size: 1, url: bottom.html_url }; + await assert.rejects(addPullRequestsToStack([top, bottom], candidate), /branches have changed/); assert(add.notCalled); refresh.callsFake(async number => number === 1 ? bottom : top); - sinon.stub(repository, 'getStackCandidate').resolves({ parentPullRequestNumber: 1, size: 1, url: bottom.html_url }); + sinon.stub(repository, 'getStackCandidate').resolves(candidate); sinon.stub(top, 'getStack').resolves({ position: 1, size: 1, base: 'main', pullRequests: [{ position: 1, number: 2, title: top.title, url: top.html_url, head: 'D2', state: GithubItemStateEnum.Open, isDraft: false, mergeable: top.item.mergeable!, }], }); - await assert.rejects(addPullRequestsToStack([bottom, top]), /already in a stack/); + await assert.rejects(addPullRequestsToStack([bottom, top], candidate), /already in a stack/); assert(add.notCalled); }); @@ -167,8 +226,9 @@ describe('Pull request stack selection', function () { return number === bottom.number ? bottom : top; }); const add = sinon.stub(repository, 'addPullRequestsToStack').resolves(); + const candidate = { parentPullRequestNumber: 1, size: 1, url: bottom.html_url }; - await assert.rejects(addPullRequestsToStack([bottom, top]), /branches have changed/); + await assert.rejects(addPullRequestsToStack([bottom, top], candidate), /branches have changed/); assert(add.notCalled); }); }); diff --git a/src/view/prsTreeDataProvider.ts b/src/view/prsTreeDataProvider.ts index 2fe3fb3201..6c5a814a06 100644 --- a/src/view/prsTreeDataProvider.ts +++ b/src/view/prsTreeDataProvider.ts @@ -266,7 +266,7 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T if (approved !== confirmation.action) { return; } - await addPullRequestsToStack(ordered); + await addPullRequestsToStack(ordered, candidate); this.refreshAll(true); void vscode.window.showInformationMessage(vscode.l10n.t('Pull requests added to the stack.')); } catch (error) {