From 072ff44e2d74a5919e7c21bf4e08a7a9846601fd Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:55:08 +0200 Subject: [PATCH 1/3] Add "create/add to stack" option when creating PR --- common/views.ts | 14 + src/github/createPRViewProvider.ts | 145 ++++++++- src/github/githubRepository.ts | 79 ++++- src/test/github/createPRViewProvider.test.ts | 304 ++++++++++++++++++ src/test/github/githubRepository.test.ts | 13 + src/test/github/pullRequestModel.test.ts | 128 ++++++++ webviews/common/createContextNew.ts | 72 ++++- webviews/createPullRequestViewNew/app.tsx | 89 +++-- webviews/createPullRequestViewNew/index.css | 39 +++ .../test/app.test.tsx | 215 +++++++++++++ 10 files changed, 1038 insertions(+), 60 deletions(-) create mode 100644 src/test/github/createPRViewProvider.test.ts create mode 100644 webviews/createPullRequestViewNew/test/app.test.tsx diff --git a/common/views.ts b/common/views.ts index 0c70a2f7c8..bdec0fa4c7 100644 --- a/common/views.ts +++ b/common/views.ts @@ -76,6 +76,9 @@ export interface CreatePullRequestNew { draft: boolean; autoMerge: boolean; autoMergeMethod?: MergeMethod; + addToStack?: boolean; + stackParentPullRequest?: number; + stackNumber?: number; labels: ILabel[]; projects: IProject[]; assignees: IAccount[]; @@ -131,6 +134,8 @@ export interface CreateParamsNew { defaultMergeMethod?: MergeMethod; mergeMethodsAvailability?: MergeMethodsAvailability; baseHasMergeQueue: boolean; + stackCandidate?: StackCandidate; + addToStack?: boolean; creating: boolean; reviewing: boolean; @@ -151,6 +156,7 @@ export interface ChooseBaseRemoteAndBranchResult { mergeMethodsAvailability: MergeMethodsAvailability; autoMergeDefault: boolean; baseHasMergeQueue: boolean; + stackCandidate?: StackCandidate; defaultTitle: string; defaultDescription: string; } @@ -159,6 +165,14 @@ export interface ChooseCompareRemoteAndBranchResult { compareRemote: RemoteInfo; compareBranch: string; defaultCompareBranch: string; + stackCandidate?: StackCandidate; +} + +export interface StackCandidate { + parentPullRequestNumber: number; + stackNumber?: number; + size: number; + url: string; } export interface TitleAndDescriptionArgs { diff --git a/src/github/createPRViewProvider.ts b/src/github/createPRViewProvider.ts index ed7ed6fa37..02975b2f49 100644 --- a/src/github/createPRViewProvider.ts +++ b/src/github/createPRViewProvider.ts @@ -19,7 +19,7 @@ import { branchPicks, cachedBranchPicks, getAssigneesQuickPickItems, getLabelOpt import { ISSUE_EXPRESSION, parseIssueExpressionOutput, variableSubstitution } from './utils'; import { ChangeTemplateReply, DisplayLabel, PreReviewState } from './views'; import { RemoteInfo } from '../../common/types'; -import { CancelCreatePullRequestNew, ChooseBaseRemoteAndBranchResult, ChooseCompareRemoteAndBranchResult, ChooseRemoteAndBranchArgs, CreateParamsNew, CreatePullRequestNew, TitleAndDescriptionArgs } from '../../common/views'; +import { CancelCreatePullRequestNew, ChooseBaseRemoteAndBranchResult, ChooseCompareRemoteAndBranchResult, ChooseRemoteAndBranchArgs, CreateParamsNew, CreatePullRequestNew, StackCandidate, TitleAndDescriptionArgs } from '../../common/views'; import type { Branch } from '../api/api'; import { debounce } from '../common/async'; import { GitHubServerType } from '../common/authentication'; @@ -39,6 +39,7 @@ import { SHOW_CREATE_PULL_REQUEST_CANCEL_CONFIRMATION } from '../common/settingKeys'; import { ITelemetry } from '../common/telemetry'; +import { toOpenPullRequestWebviewUri } from '../common/uri'; import { asPromise, compareIgnoreCase, formatError, promiseWithTimeout } from '../common/utils'; import { generateUuid } from '../common/uuid'; import { IRequestMessage, WebviewViewBase } from '../common/webview'; @@ -673,6 +674,7 @@ function serializeRemoteInfo(remote: { owner: string, repositoryName: string }) export class CreatePullRequestViewProvider extends BaseCreatePullRequestViewProvider implements vscode.WebviewViewProvider { public override readonly viewType = 'github:createPullRequestWebview'; + private _stackCandidateSequence = 0; constructor( telemetry: ITelemetry, @@ -684,6 +686,7 @@ export class CreatePullRequestViewProvider extends BaseCreatePullRequestViewProv super(telemetry, model, extensionUri, folderRepositoryManager, pullRequestDefaults, model.compareBranch); this._register(this.model.onDidChange(async (e) => { + const stackCandidateSequence = ++this._stackCandidateSequence; let baseRemote: RemoteInfo | undefined; let baseBranch: string | undefined; if (e.baseOwner) { @@ -710,7 +713,16 @@ export class CreatePullRequestViewProvider extends BaseCreatePullRequestViewProv compareRemote, compareBranch, warning: await this.existingPRMessage(), + stackCandidate: await this.getStackCandidateForView( + { owner: this.model.baseOwner, repositoryName: this.model.repositoryName }, + this.model.baseBranch, + { owner: this.model.compareOwner, repositoryName: this.model.repositoryName }, + this.model.compareBranch + ), }; + if (stackCandidateSequence !== this._stackCandidateSequence) { + return; + } // TODO: consider updating title and description return this._postMessage({ command: 'pr.initialize', @@ -737,12 +749,21 @@ export class CreatePullRequestViewProvider extends BaseCreatePullRequestViewProv this._defaultCompareBranch = compareBranch.name; this.model.setCompareBranch(compareBranch.name); this.changeBranch(compareBranch.name, false).then(async titleAndDescription => { + if (compareBranch.name !== this.model.compareBranch) { + return; + } const params: Partial = { defaultTitle: titleAndDescription.title, defaultDescription: titleAndDescription.description, compareBranch: compareBranch.name, defaultCompareBranch: compareBranch.name, warning: await this.existingPRMessage(), + stackCandidate: await this.getStackCandidateForView( + { owner: this.model.baseOwner, repositoryName: this.model.repositoryName }, + this.model.baseBranch, + { owner: this.model.compareOwner, repositoryName: this.model.repositoryName }, + this.model.compareBranch + ), }; return this._postMessage({ command: 'pr.initialize', @@ -1058,9 +1079,43 @@ Don't forget to commit your template file to the repository so that it can be us const params = await super.getCreateParams(); // Pre-fetch branches so they're cached when the user opens the branch picker this.prefetchBranches(params.defaultBaseRemote!); + params.stackCandidate = await this.getStackCandidateForView(params.defaultBaseRemote, params.defaultBaseBranch, params.defaultCompareRemote, params.defaultCompareBranch); return params; } + private async getStackCandidate(baseRemote: RemoteInfo | undefined, baseBranch: string | undefined, compareRemote: RemoteInfo | undefined, compareBranch: string | undefined): Promise { + if (!baseRemote || !baseBranch || !compareRemote || !compareBranch || + (compareIgnoreCase(baseRemote.owner, compareRemote.owner) === 0 && + compareIgnoreCase(baseRemote.repositoryName, compareRemote.repositoryName) === 0 && + baseBranch === compareBranch)) { + return; + } + const repo = await this._folderRepositoryManager.createGitHubRepositoryFromOwnerName(baseRemote.owner, baseRemote.repositoryName); + if (!repo) { + throw new Error(vscode.l10n.t('Unable to find the selected base repository.')); + } + return repo.getStackCandidate(baseBranch); + } + + protected async getStackCandidateForView(baseRemote: RemoteInfo | undefined, baseBranch: string | undefined, compareRemote: RemoteInfo | undefined, compareBranch: string | undefined): Promise { + try { + const candidate = await this.getStackCandidate(baseRemote, baseBranch, compareRemote, compareBranch); + if (!candidate || !baseRemote) { + return candidate; + } + const url = await toOpenPullRequestWebviewUri({ + owner: baseRemote.owner, + repo: baseRemote.repositoryName, + pullRequestNumber: candidate.parentPullRequestNumber, + }); + return { ...candidate, url: url.toString() }; + } catch (error) { + Logger.error(`Failed to check stack eligibility: ${formatError(error)}`, CreatePullRequestViewProvider.ID); + void vscode.window.showWarningMessage(vscode.l10n.t('Unable to check pull request stack eligibility: {0}', formatError(error))); + return; + } + } + private prefetchBranches(baseRemote: RemoteInfo): void { const githubRepository = this._folderRepositoryManager.findRepo( repo => repo.remote.owner === baseRemote.owner && repo.remote.repositoryName === baseRemote.repositoryName, @@ -1101,9 +1156,9 @@ Don't forget to commit your template file to the repository so that it can be us } const defaultBranch = await githubRepository.getDefaultBranch(); - commands.setContext(contexts.CREATE_PR_PERMISSIONS, viewerPermission); let chooseResult: ChooseBaseRemoteAndBranchResult | ChooseCompareRemoteAndBranchResult; if (isBase) { + commands.setContext(contexts.CREATE_PR_PERMISSIONS, viewerPermission); const baseRemoteChanged = this.model.baseOwner !== result.remote.owner; const baseBranchChanged = baseRemoteChanged || this.model.baseBranch !== result.branch; this.model.baseOwner = result.remote.owner; @@ -1131,6 +1186,8 @@ Don't forget to commit your template file to the repository so that it can be us defaultMergeMethod: mergeConfiguration ? getDefaultMergeMethod(mergeConfiguration.mergeMethodsAvailability) : 'merge' as MergeMethod, allowAutoMerge: mergeConfiguration?.viewerCanAutoMerge ?? false, baseHasMergeQueue: !!mergeQueueMethodForBranch, + stackCandidate: await this.getStackCandidateForView(result.remote, result.branch, + { owner: this.model.compareOwner, repositoryName: this.model.repositoryName }, this.model.compareBranch), mergeMethodsAvailability: mergeConfiguration?.mergeMethodsAvailability ?? { merge: true, squash: true, rebase: true }, autoMergeDefault, defaultTitle: titleAndDescription.title, @@ -1149,12 +1206,30 @@ Don't forget to commit your template file to the repository so that it can be us this._folderRepositoryManager.telemetry.sendTelemetryEvent('pr.create.changedBaseBranch'); } } else { - await this.changeBranch(result.branch, false); - chooseResult = { - compareRemote: result.remote, - compareBranch: result.branch, - defaultCompareBranch: defaultBranch - }; + const compareBranch = await this._folderRepositoryManager.repository.getBranch(result.branch); + const previousOwner = this.model.compareOwner; + const previousBranch = this.model.compareBranch; + try { + this.model.compareOwner = result.remote.owner; + await this.model.setCompareBranch(result.branch); + await this.getTitleAndDescription(compareBranch, this.model.baseBranch); + chooseResult = { + compareRemote: result.remote, + compareBranch: result.branch, + defaultCompareBranch: defaultBranch, + stackCandidate: await this.getStackCandidateForView( + { owner: this.model.baseOwner, repositoryName: this.model.repositoryName }, + this.model.baseBranch, result.remote, result.branch) + }; + } catch (error) { + const ownerChanged = this.model.compareOwner !== previousOwner; + this.model.compareOwner = previousOwner; + if (ownerChanged || this.model.compareBranch !== previousBranch) { + await this.model.setCompareBranch(previousBranch); + } + throw error; + } + commands.setContext(contexts.CREATE_PR_PERMISSIONS, viewerPermission); /* __GDPR__ "pr.create.changedCompare" : {} */ @@ -1252,12 +1327,17 @@ Don't forget to commit your template file to the repository so that it can be us } quickPick.busy = true; - const chooseResult = await this.processRemoteAndBranchResult(githubRepository, result, isBase); - - quickPick.hide(); - quickPick.dispose(); - onDidChangeValueDisposable?.dispose(); - return this._replyMessage(message, chooseResult); + try { + const chooseResult = await this.processRemoteAndBranchResult(githubRepository, result, isBase); + await this._replyMessage(message, chooseResult); + } catch (error) { + Logger.error(`Failed to change pull request branch: ${formatError(error)}`, CreatePullRequestViewProvider.ID); + await this._throwError(message, formatError(error)); + } finally { + quickPick.hide(); + quickPick.dispose(); + onDidChangeValueDisposable?.dispose(); + } } private async getCommitsAndPatches(): Promise<{ commitMessages: string[], patches: { patch: string, fileUri: string, previousFileUri?: string }[] }> { @@ -1439,7 +1519,21 @@ Don't forget to commit your template file to the repository so that it can be us let totalIncrement = 0; progress.report({ message: vscode.l10n.t('Checking for upstream branch'), increment: totalIncrement }); let createdPR: PullRequestModel | undefined = undefined; + let stackAdditionFailed = false; try { + let stackCandidate: StackCandidate | undefined; + if (message.args.addToStack) { + if (message.args.autoMerge) { + throw new Error(vscode.l10n.t('Auto-merge is not available for stacked pull requests.')); + } + stackCandidate = await this.getStackCandidate( + { owner: message.args.owner, repositoryName: message.args.repo }, message.args.base, + { owner: message.args.compareOwner, repositoryName: message.args.compareRepo }, message.args.compareBranch); + if (!stackCandidate || stackCandidate.parentPullRequestNumber !== message.args.stackParentPullRequest + || stackCandidate.stackNumber !== message.args.stackNumber) { + throw new Error(vscode.l10n.t('The selected base branch is no longer at the top of a pull request stack. Refresh and try again.')); + } + } const compareOwner = message.args.compareOwner; const compareRepositoryName = message.args.compareRepo; const compareBranchName = message.args.compareBranch; @@ -1540,7 +1634,24 @@ Don't forget to commit your template file to the repository so that it can be us } else { // Save the base branch to recently used branches after successful PR creation this.saveRecentlyUsedBranch(message.args.owner, message.args.repo, message.args.base); - await this.postCreate(message, createdPR); + let postCreateError: unknown; + try { + await this.postCreate(message, createdPR); + } catch (error) { + postCreateError = error; + Logger.error(`Failed to set pull request details: ${formatError(error)}`, CreatePullRequestViewProvider.ID); + } + if (stackCandidate) { + try { + await createdPR.githubRepository.addPullRequestToStack(stackCandidate, createdPR.number); + } catch (error) { + stackAdditionFailed = true; + throw error; + } + } + if (postCreateError) { + throw postCreateError; + } } } catch (e) { if (!createdPR) { @@ -1555,7 +1666,9 @@ Don't forget to commit your template file to the repository so that it can be us await this.postCreate(message, createdPR); } // All of these errors occur after the PR is created, so the error is not critical. - vscode.window.showErrorMessage(vscode.l10n.t('There was an error creating the pull request: {0}', (e as Error).message)); + vscode.window.showErrorMessage(stackAdditionFailed + ? vscode.l10n.t('Pull request #{0} was created but could not be added to its stack: {1}', createdPR.number, formatError(e)) + : vscode.l10n.t('There was an error creating the pull request: {0}', (e as Error).message)); } } finally { commands.setContext(contexts.CREATING, false); diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index cafd947a77..ebf0583dc8 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -42,6 +42,7 @@ import { } from './graphql'; import { CheckState, + GithubItemStateEnum, IAccount, IMilestone, IProject, @@ -76,6 +77,7 @@ import { parseMilestone, restPaginate, } from './utils'; +import { StackCandidate } from '../../common/views'; import { AuthenticationError, AuthProvider, GitHubServerType, isSamlError } from '../common/authentication'; import { Disposable, disposeAll } from '../common/lifecycle'; @@ -86,6 +88,7 @@ import { GitHubRemote, parseRemote } from '../common/remote'; import { BRANCH_LIST_TIMEOUT, PR_SETTINGS_NAMESPACE } from '../common/settingKeys'; import { ITelemetry } from '../common/telemetry'; +import { compareIgnoreCase, isObject } from '../common/utils'; import { PullRequestCommentController } from '../view/pullRequestCommentController'; @@ -778,7 +781,7 @@ export class GitHubRepository extends Disposable { return undefined; } - async getPullRequestForBranch(branch: string, headOwner: string): Promise { + async getPullRequestForBranch(branch: string, headOwner: string, throwOnError: boolean = false): Promise { let remote: GitHubRemote | undefined; try { Logger.debug(`Fetch pull requests for branch - enter`, this.id); @@ -798,7 +801,7 @@ export class GitHubRepository extends Disposable { if (data?.repository) { const nodes = [...data.repository.openPullRequests.nodes, ...data.repository.pullRequests.nodes] .filter((pullRequest, index, pullRequests) => pullRequests.findIndex(candidate => candidate.number === pullRequest.number) === index); - const prs = (await Promise.all(nodes.map(node => parseGraphQLPullRequest(node, this)))).filter(pr => pr.head?.repo.owner === headOwner); + const prs = (await Promise.all(nodes.map(node => parseGraphQLPullRequest(node, this)))).filter(pr => pr.head && compareIgnoreCase(pr.head.repo.owner, headOwner) === 0); if (prs.length === 0) { return undefined; } @@ -807,6 +810,9 @@ export class GitHubRepository extends Disposable { } } catch (e) { Logger.error(`Fetching pull request for branch failed: ${e}`, this.id); + if (throwOnError) { + throw e; + } if (e.status === 404) { // not found vscode.window.showWarningMessage( @@ -817,6 +823,75 @@ export class GitHubRepository extends Disposable { return undefined; } + async getStackCandidate(baseBranch: string): Promise { + const parent = await this.getPullRequestForBranch(baseBranch, this.remote.owner, true); + if (!parent || parent.state !== GithubItemStateEnum.Open || parent.head?.ref !== baseBranch + || parent.head.owner.toLowerCase() !== this.remote.owner.toLowerCase() + || parent.head.repositoryCloneUrl.repositoryName.toLowerCase() !== this.remote.repositoryName.toLowerCase()) { + return; + } + + const { octokit, remote } = await this.ensure(); + let data: unknown; + try { + ({ data } = await octokit.call(() => octokit.api.request('GET /repos/{owner}/{repo}/stacks', { + owner: remote.owner, + repo: remote.repositoryName, + pull_request: parent.number, + per_page: 1, + headers: { 'X-GitHub-Api-Version': '2026-03-10' }, + }))); + } catch (error) { + if (isObject(error) && error.status === 404) { + Logger.debug('Pull request stacks are not supported by this GitHub server.', this.id); + return; + } + throw error; + } + + if (!Array.isArray(data) || data.length > 1) { + throw new Error('GitHub returned an invalid pull request stack list.'); + } + if (data.length === 0) { + return { parentPullRequestNumber: parent.number, size: 1, url: parent.html_url }; + } + const stack: unknown = data[0]; + if (!isObject(stack) || typeof stack.number !== 'number' || !Array.isArray(stack.pull_requests) + || stack.pull_requests.length === 0) { + throw new Error('GitHub returned an invalid pull request stack.'); + } + const top: unknown = stack.pull_requests[stack.pull_requests.length - 1]; + if (!isObject(top) || typeof top.number !== 'number') { + throw new Error('GitHub returned an invalid pull request stack entry.'); + } + if (top.number !== parent.number) { + return; + } + return { parentPullRequestNumber: parent.number, stackNumber: stack.number, size: stack.pull_requests.length, url: parent.html_url }; + } + + async addPullRequestToStack(candidate: StackCandidate, number: 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 stackNumber = candidate.stackNumber; + if (stackNumber !== undefined) { + await octokit.call(() => octokit.api.request('POST /repos/{owner}/{repo}/stacks/{stack_number}/add', { + ...params, + stack_number: stackNumber, + pull_requests: [number], + })); + } else { + await octokit.call(() => octokit.api.request('POST /repos/{owner}/{repo}/stacks', { + ...params, + pull_requests: [candidate.parentPullRequestNumber, number], + })); + } + } + async canGetProjectsNow(): Promise { let { schema } = await this.ensure(); if (schema.GetRepoProjects && schema.GetOrgProjects) { diff --git a/src/test/github/createPRViewProvider.test.ts b/src/test/github/createPRViewProvider.test.ts new file mode 100644 index 0000000000..cc61f733e3 --- /dev/null +++ b/src/test/github/createPRViewProvider.test.ts @@ -0,0 +1,304 @@ +/*--------------------------------------------------------------------------------------------- + * 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 * as vscode from 'vscode'; +import { createSandbox, SinonSandbox } from 'sinon'; +import { CreatePullRequestNew, StackCandidate } from '../../../common/views'; +import { RemoteInfo } from '../../../common/types'; +import { GitApiImpl } from '../../api/api1'; +import { Branch } from '../../api/api'; +import { GitHubServerType } from '../../common/authentication'; +import { Protocol } from '../../common/protocol'; +import { GitHubRemote } from '../../common/remote'; +import { fromOpenOrCheckoutPullRequestWebviewUri } from '../../common/uri'; +import { asPromise } from '../../common/utils'; +import { IRequestMessage } from '../../common/webview'; +import { CreatePullRequestViewProvider } from '../../github/createPRViewProvider'; +import { CredentialStore } from '../../github/credentials'; +import { FolderRepositoryManager } from '../../github/folderRepositoryManager'; +import { ViewerPermission } from '../../github/githubRepository'; +import { PullRequestModel } from '../../github/pullRequestModel'; +import { RepositoriesManager } from '../../github/repositoriesManager'; +import { convertRESTPullRequestToRawPullRequest } from '../../github/utils'; +import { CreatePullRequestDataModel } from '../../view/createPullRequestDataModel'; +import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; +import { PullRequestBuilder } from '../builders/rest/pullRequestBuilder'; +import { MockCommandRegistry } from '../mocks/mockCommandRegistry'; +import { MockExtensionContext } from '../mocks/mockExtensionContext'; +import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; +import { MockRepository } from '../mocks/mockRepository'; +import { MockTelemetry } from '../mocks/mockTelemetry'; +import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; + +class TestCreatePullRequestViewProvider extends CreatePullRequestViewProvider { + failTitleAndDescription = false; + + protected override async getTitleAndDescription(_compareBranch: Branch, _baseBranch: string) { + if (this.failTitleAndDescription) { + throw new Error('Unable to compute the title'); + } + return { title: '', description: '' }; + } + + getStackCandidateForTest(baseRemote: RemoteInfo, baseBranch: string, compareRemote: RemoteInfo, compareBranch: string) { + return this.getStackCandidateForView(baseRemote, baseBranch, compareRemote, compareBranch); + } + + public override _replyMessage(message: IRequestMessage, response: any) { + return super._replyMessage(message, response); + } + + public override _throwError(message: IRequestMessage | undefined, error: string) { + return super._throwError(message, error); + } + + public override postCreate(message: IRequestMessage, createdPR: PullRequestModel) { + return super.postCreate(message, createdPR); + } + + createForTest(message: { command: string; args: CreatePullRequestNew; req: string }) { + return this.create(message); + } +} + +describe('Create pull request stack', function () { + let sinon: SinonSandbox; + let context: MockExtensionContext; + let credentials: CredentialStore; + let folderManager: FolderRepositoryManager; + let repository: MockRepository; + let githubRepository: MockGitHubRepository; + let provider: TestCreatePullRequestViewProvider; + let model: CreatePullRequestDataModel; + + beforeEach(async function () { + sinon = createSandbox(); + MockCommandRegistry.install(sinon); + context = new MockExtensionContext(); + const telemetry = new MockTelemetry(); + credentials = new CredentialStore(telemetry, context); + repository = new MockRepository(); + const manager = new RepositoriesManager(credentials, telemetry); + folderManager = new FolderRepositoryManager(0, context, repository, telemetry, new GitApiImpl(manager), credentials, new CreatePullRequestHelper(), new MockThemeWatcher()); + const url = 'https://github.com/github/test'; + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + githubRepository = new MockGitHubRepository(remote, credentials, telemetry, sinon); + sinon.stub(folderManager, 'gitHubRepositories').get(() => [githubRepository]); + sinon.stub(folderManager, 'findRepo').callsFake(predicate => predicate(githubRepository) ? githubRepository : undefined); + await repository.createBranch('D4', false, 'commit-sha'); + await repository.setBranchUpstream('D4', 'refs/remotes/origin/D4'); + sinon.stub(githubRepository, 'hasBranch').resolves('commit-sha'); + model = new CreatePullRequestDataModel(folderManager, 'github', 'D3', 'github', 'D4', 'test'); + provider = new TestCreatePullRequestViewProvider(telemetry, model, vscode.Uri.file(__dirname), folderManager, { owner: 'github', repo: 'test', base: 'main' }); + }); + + afterEach(function () { + model.dispose(); + folderManager.dispose(); + githubRepository.dispose(); + credentials.dispose(); + context.dispose(); + sinon.restore(); + }); + + it('links to the parent PR webview from the stack option', async function () { + sinon.stub(folderManager, 'createGitHubRepositoryFromOwnerName').resolves(githubRepository); + sinon.stub(githubRepository, 'getStackCandidate').resolves({ + parentPullRequestNumber: 795, + stackNumber: 12, + size: 3, + url: 'https://github.com/github/test/pull/795', + }); + + const candidate = await provider.getStackCandidateForTest( + { owner: 'github', repositoryName: 'test' }, 'D3', + { owner: 'github', repositoryName: 'test' }, 'D4', + ); + assert(candidate); + const link = vscode.Uri.parse(candidate.url); + assert.strictEqual(link.path, '/open-pull-request-webview'); + assert.deepStrictEqual(fromOpenOrCheckoutPullRequestWebviewUri(link), { owner: 'github', repo: 'test', pullRequestNumber: 795 }); + }); + + it('updates compare owner before detecting stacks after switching to a fork with the same branch name', async function () { + await repository.createBranch('D3', false, 'new-commit-sha'); + sinon.stub(githubRepository, 'getViewerPermission').resolves(ViewerPermission.Write); + sinon.stub(githubRepository, 'getDefaultBranch').resolves('main'); + sinon.stub(folderManager, 'createGitHubRepositoryFromOwnerName').resolves(githubRepository); + const getCandidate = sinon.stub(githubRepository, 'getStackCandidate').resolves({ + parentPullRequestNumber: 795, stackNumber: 12, size: 3, url: 'https://github.com/github/test/pull/795', + }); + + const result = await provider['processRemoteAndBranchResult'](githubRepository, { + remote: { owner: 'fork', repositoryName: 'test' }, + branch: 'D3', + }, false); + + assert.strictEqual(model.compareOwner, 'fork'); + assert.strictEqual(result.stackCandidate?.parentPullRequestNumber, 795); + assert(getCandidate.calledWithExactly('D3')); + }); + + it('keeps the old owner when the selected compare branch is not available locally', async function () { + sinon.stub(githubRepository, 'getViewerPermission').resolves(ViewerPermission.Write); + sinon.stub(githubRepository, 'getDefaultBranch').resolves('main'); + + await assert.rejects(provider['processRemoteAndBranchResult'](githubRepository, { + remote: { owner: 'fork', repositoryName: 'test' }, + branch: 'not-local', + }, false), /unrecognized name/); + + assert.strictEqual(model.compareOwner, 'github'); + assert.strictEqual(model.compareBranch, 'D4'); + }); + + it('rolls back both owner and branch when a later compare update fails', async function () { + await repository.createBranch('D3', false, 'new-commit-sha'); + sinon.stub(githubRepository, 'getViewerPermission').resolves(ViewerPermission.Write); + sinon.stub(githubRepository, 'getDefaultBranch').resolves('main'); + provider.failTitleAndDescription = true; + + await assert.rejects(provider['processRemoteAndBranchResult'](githubRepository, { + remote: { owner: 'fork', repositoryName: 'test' }, + branch: 'D3', + }, false), /Unable to compute the title/); + + assert.strictEqual(model.compareOwner, 'github'); + assert.strictEqual(model.compareBranch, 'D4'); + }); + + it('restores upstream state when switching repositories without changing the branch name fails', async function () { + assert.strictEqual(await model.getCompareHasUpstream(), true); + sinon.stub(githubRepository, 'getViewerPermission').resolves(ViewerPermission.Write); + sinon.stub(githubRepository, 'getDefaultBranch').resolves('main'); + provider.failTitleAndDescription = true; + + await assert.rejects(provider['processRemoteAndBranchResult'](githubRepository, { + remote: { owner: 'fork', repositoryName: 'test' }, + branch: 'D4', + }, false), /Unable to compute the title/); + + assert.strictEqual(model.compareOwner, 'github'); + assert.strictEqual(model.compareBranch, 'D4'); + assert.strictEqual(await model.getCompareHasUpstream(), true); + }); + + it('notifies the user when stack eligibility cannot be checked', async function () { + const error = new Error('GraphQL unavailable'); + sinon.stub(folderManager, 'createGitHubRepositoryFromOwnerName').resolves(githubRepository); + sinon.stub(githubRepository, 'getStackCandidate').rejects(error); + const warn = sinon.stub(vscode.window, 'showWarningMessage').resolves(undefined); + + const result = await provider.getStackCandidateForTest( + { owner: 'github', repositoryName: 'test' }, 'D3', + { owner: 'github', repositoryName: 'test' }, 'D4', + ); + + assert.strictEqual(result, undefined); + assert(warn.calledOnce); + assert.match(warn.firstCall.args[0], /GraphQL unavailable/); + }); + + it('revalidates the parent and adds the new PR to its stack', async function () { + this.timeout(10000); + const cancellation = new vscode.CancellationTokenSource(); + sinon.stub(vscode.window, 'withProgress').callsFake((_options, task) => task({ report: () => undefined }, cancellation.token)); + const candidate: StackCandidate = { parentPullRequestNumber: 795, stackNumber: 12, size: 3, url: 'https://github.com/github/test/pull/795' }; + const getCandidate = sinon.stub(githubRepository, 'getStackCandidate').resolves(candidate); + sinon.stub(folderManager, 'createGitHubRepositoryFromOwnerName').resolves(githubRepository); + sinon.stub(model, 'filesHaveChanges').resolves(false); + repository.expectFetch('origin', 'D4'); + const createdPR = new PullRequestModel(credentials, new MockTelemetry(), githubRepository, githubRepository.remote, + convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(796).build(), githubRepository)); + const create = sinon.stub(folderManager, 'createPullRequest').resolves(createdPR); + const addToStack = sinon.stub(githubRepository, 'addPullRequestToStack').resolves(); + sinon.stub(provider, 'postCreate').resolves(); + sinon.stub(provider, '_replyMessage').resolves(); + const throwError = sinon.stub(provider, '_throwError').resolves(); + const done = asPromise(provider.onDone); + + await provider.createForTest({ + command: 'pr.create', req: '1', + args: { + title: 'Fourth change', body: '', owner: 'github', repo: 'test', base: 'D3', + compareOwner: 'github', compareRepo: 'test', compareBranch: 'D4', + draft: false, autoMerge: false, labels: [], projects: [], assignees: [], reviewers: [], + addToStack: true, stackParentPullRequest: 795, stackNumber: 12, + }, + }); + const result = await Promise.race([ + done, + new Promise(resolve => setTimeout(() => resolve(undefined), 2000)), + ]); + assert(result === createdPR, throwError.firstCall?.args[1] ?? 'Pull request creation did not complete.'); + assert(getCandidate.calledWithExactly('D3')); + assert(create.calledOnce); + assert(addToStack.calledOnceWithExactly(candidate, 796)); + sinon.assert.callOrder(getCandidate, create, addToStack); + cancellation.dispose(); + }); + + it('does not create a pull request when the selected stack parent has changed', async function () { + const cancellation = new vscode.CancellationTokenSource(); + sinon.stub(vscode.window, 'withProgress').callsFake((_options, task) => task({ report: () => undefined }, cancellation.token)); + sinon.stub(githubRepository, 'getStackCandidate').resolves({ parentPullRequestNumber: 795, stackNumber: 12, size: 3, url: 'https://github.com/github/test/pull/795' }); + sinon.stub(folderManager, 'createGitHubRepositoryFromOwnerName').resolves(githubRepository); + sinon.stub(model, 'filesHaveChanges').resolves(false); + const create = sinon.stub(folderManager, 'createPullRequest'); + const throwError = sinon.stub(provider, '_throwError').resolves(); + let finish!: () => void; + const finished = new Promise(resolve => { finish = resolve; }); + sinon.stub(provider, '_replyMessage').callsFake(async () => { finish(); }); + + await provider.createForTest({ + command: 'pr.create', req: '2', + args: { + title: 'Fourth change', body: '', owner: 'github', repo: 'test', base: 'D3', + compareOwner: 'github', compareRepo: 'test', compareBranch: 'D4', + draft: false, autoMerge: false, labels: [], projects: [], assignees: [], reviewers: [], + addToStack: true, stackParentPullRequest: 794, stackNumber: 12, + }, + }); + await finished; + + assert(create.notCalled); + assert(throwError.calledOnce); + assert.match(throwError.firstCall.args[1], /no longer at the top/); + cancellation.dispose(); + }); + + it('reports a failed stack addition while still applying PR details', async function () { + const cancellation = new vscode.CancellationTokenSource(); + sinon.stub(vscode.window, 'withProgress').callsFake((_options, task) => task({ report: () => undefined }, cancellation.token)); + const candidate: StackCandidate = { parentPullRequestNumber: 795, stackNumber: 12, size: 3, url: 'https://github.com/github/test/pull/795' }; + sinon.stub(githubRepository, 'getStackCandidate').resolves(candidate); + sinon.stub(folderManager, 'createGitHubRepositoryFromOwnerName').resolves(githubRepository); + sinon.stub(model, 'filesHaveChanges').resolves(false); + repository.expectFetch('origin', 'D4'); + const createdPR = new PullRequestModel(credentials, new MockTelemetry(), githubRepository, githubRepository.remote, + convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(796).build(), githubRepository)); + sinon.stub(folderManager, 'createPullRequest').resolves(createdPR); + const setDetails = sinon.stub(provider, 'postCreate').resolves(); + sinon.stub(githubRepository, 'addPullRequestToStack').rejects(new Error('Stack is locked')); + const showError = sinon.stub(vscode.window, 'showErrorMessage').resolves(undefined); + const done = asPromise(provider.onDone); + + await provider.createForTest({ + command: 'pr.create', req: '3', + args: { + title: 'Fourth change', body: '', owner: 'github', repo: 'test', base: 'D3', + compareOwner: 'github', compareRepo: 'test', compareBranch: 'D4', + draft: false, autoMerge: false, labels: [], projects: [], assignees: [], reviewers: [], + addToStack: true, stackParentPullRequest: 795, stackNumber: 12, + }, + }); + assert((await done) === createdPR); + assert(setDetails.calledOnce); + assert(showError.calledOnce); + assert.match(showError.firstCall.args[0], /#796 was created but could not be added to its stack: Stack is locked/); + cancellation.dispose(); + }); +}); diff --git a/src/test/github/githubRepository.test.ts b/src/test/github/githubRepository.test.ts index 55e0744c23..080c1def1d 100644 --- a/src/test/github/githubRepository.test.ts +++ b/src/test/github/githubRepository.test.ts @@ -283,6 +283,19 @@ describe('GitHubRepository', function () { const pullRequest = await repo.getPullRequestForBranch('feature', 'me'); assert.strictEqual(pullRequest?.number, 7231); + assert.strictEqual((await repo.getPullRequestForBranch('feature', 'ME'))?.number, 7231); + }); + + it('preserves legacy behavior on lookup errors unless requested by the caller', async function () { + const url = 'https://github.com/some/repo'; + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + const repo = new GitHubRepository(1, remote, Uri.file('/workspaces/repo'), credentialStore, telemetry, true); + const error = new Error('GraphQL unavailable'); + sinon.stub(repo, 'ensure').resolves(repo); + sinon.stub(repo, 'query').rejects(error); + + assert.strictEqual(await repo.getPullRequestForBranch('feature', 'some'), undefined); + await assert.rejects(repo.getPullRequestForBranch('feature', 'some', true), candidate => candidate === error); }); }); diff --git a/src/test/github/pullRequestModel.test.ts b/src/test/github/pullRequestModel.test.ts index 408ea3f05d..46014956bd 100644 --- a/src/test/github/pullRequestModel.test.ts +++ b/src/test/github/pullRequestModel.test.ts @@ -17,6 +17,7 @@ import { GitHubRemote, Remote } from '../../common/remote'; import { convertRESTPullRequestToRawPullRequest } from '../../github/utils'; import { SinonSandbox, createSandbox } from 'sinon'; import { PullRequestBuilder } from '../builders/rest/pullRequestBuilder'; +import { PullRequestBuilder as GraphQLPullRequestBuilder } from '../builders/graphql/pullRequestBuilder'; import { MockTelemetry } from '../mocks/mockTelemetry'; import { MockGitHubRepository } from '../mocks/mockGitHubRepository'; import { MockRepository } from '../mocks/mockRepository'; @@ -396,6 +397,133 @@ describe('PullRequestModel', function () { }); }); + describe('stack creation', function () { + function parentModel() { + const pr = new PullRequestBuilder().number(795).head(head => head.ref('D3')).build(); + pr.html_url = 'https://github.com/github/test/pull/795'; + pr.head.repo.name = 'test'; + pr.head.repo.owner.login = 'github'; + pr.head.repo.clone_url = 'https://github.com/github/test.git'; + return new PullRequestModel(credentials, telemetry, repo, remote, convertRESTPullRequestToRawPullRequest(pr, repo)); + } + + const listRoute = 'GET /repos/{owner}/{repo}/stacks'; + const listParams = { + owner: 'github', + repo: 'test', + pull_request: 795, + per_page: 1, + headers: { 'X-GitHub-Api-Version': '2026-03-10' }, + }; + + it('detects an unstacked parent and creates a new stack with both PRs', async function () { + sinon.stub(repo, 'getPullRequestForBranch').resolves(parentModel()); + repo.queryProvider.expectOctokitRequest(['request'], [listRoute, listParams], []); + repo.queryProvider.expectOctokitRequest(['request'], ['POST /repos/{owner}/{repo}/stacks', { + owner: 'github', + repo: 'test', + headers: listParams.headers, + pull_requests: [795, 796], + }], {}); + + const candidate = await repo.getStackCandidate('D3'); + assert.deepStrictEqual(candidate, { parentPullRequestNumber: 795, size: 1, url: 'https://github.com/github/test/pull/795' }); + assert(candidate); + await repo.addPullRequestToStack(candidate, 796); + }); + + 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; + parent.url = 'https://github.com/github/test/pull/795'; + parent.headRefName = 'D3'; + parent.headRef!.name = 'D3'; + parent.headRef!.repository.owner.login = 'github'; + parent.headRef!.repository.url = 'https://github.com/github/test'; + parent.headRepository!.owner.login = 'github'; + parent.headRepository!.url = 'https://github.com/github/test'; + repo.queryProvider.expectGraphQLQuery({ + query: queries.PullRequestForHead, + variables: { owner: 'github', name: 'test', headRefName: 'D3' }, + }, { + data: { + repository: { + openPullRequests: { nodes: [parent] }, + pullRequests: { nodes: [] }, + }, + }, + loading: false, + stale: false, + networkStatus: NetworkStatus.ready, + }); + repo.queryProvider.expectOctokitRequest(['request'], [listRoute, listParams], []); + + assert.deepStrictEqual(await repo.getStackCandidate('D3'), { parentPullRequestNumber: 795, size: 1, url: 'https://github.com/github/test/pull/795' }); + }); + + it('surfaces a failed GraphQL parent lookup rather than treating the branch as unstackable', async function () { + const error = new Error('GraphQL unavailable'); + sinon.stub(repo, 'query').rejects(error); + + await assert.rejects(repo.getStackCandidate('D3'), candidate => candidate === error); + }); + + it('only offers an existing stack when the parent is at its top', async function () { + sinon.stub(repo, 'getPullRequestForBranch').resolves(parentModel()); + repo.queryProvider.expectOctokitRequest(['request'], [listRoute, listParams], [{ + number: 12, + pull_requests: [{ number: 793 }, { number: 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], + }], {}); + + const candidate = await repo.getStackCandidate('D3'); + assert.deepStrictEqual(candidate, { parentPullRequestNumber: 795, stackNumber: 12, size: 2, url: 'https://github.com/github/test/pull/795' }); + assert(candidate); + await repo.addPullRequestToStack(candidate, 796); + }); + + it('does not offer a stack when the matching PR is not the top', async function () { + sinon.stub(repo, 'getPullRequestForBranch').resolves(parentModel()); + repo.queryProvider.expectOctokitRequest(['request'], [listRoute, listParams], [{ + number: 12, + pull_requests: [{ number: 795 }, { number: 797 }], + }]); + + assert.strictEqual(await repo.getStackCandidate('D3'), undefined); + }); + + it('does not offer stack creation on servers without the Stacks API', async function () { + sinon.stub(repo, 'getPullRequestForBranch').resolves(parentModel()); + repo.queryProvider.expectOctokitError(['request'], [listRoute, listParams], Object.assign(new Error('Not Found'), { status: 404 })); + + assert.strictEqual(await repo.getStackCandidate('D3'), undefined); + }); + + it('reports failures other than unsupported Stacks API', async function () { + sinon.stub(repo, 'getPullRequestForBranch').resolves(parentModel()); + repo.queryProvider.expectOctokitError(['request'], [listRoute, listParams], Object.assign(new Error('Forbidden'), { status: 403 })); + + await assert.rejects(repo.getStackCandidate('D3'), /Forbidden/); + }); + + it('does not mistake a pull request from another repository for the base head', async function () { + const pr = new PullRequestBuilder().number(795).head(head => head.ref('D3')).build(); + pr.head.repo.name = 'other'; + pr.head.repo.owner.login = 'github'; + pr.head.repo.clone_url = 'https://github.com/github/other.git'; + const parent = new PullRequestModel(credentials, telemetry, repo, remote, convertRESTPullRequestToRawPullRequest(pr, repo)); + sinon.stub(repo, 'getPullRequestForBranch').resolves(parent); + + assert.strictEqual(await repo.getStackCandidate('D3'), undefined); + }); + }); + describe('openReadonlyChanges', function () { const baseCommit = '1111111111111111111111111111111111111111'; const mergeBase = '2222222222222222222222222222222222222222'; diff --git a/webviews/common/createContextNew.ts b/webviews/common/createContextNew.ts index b220858f27..bce5f17fc8 100644 --- a/webviews/common/createContextNew.ts +++ b/webviews/common/createContextNew.ts @@ -6,7 +6,7 @@ import { createContext } from 'react'; import { getMessageHandler, MessageHandler, vscode } from './message'; import { RemoteInfo } from '../../common/types'; -import { CancelCreatePullRequestNew, ChooseBaseRemoteAndBranchResult, ChooseCompareRemoteAndBranchResult, ChooseRemoteAndBranchArgs, CreateParamsNew, CreatePullRequestNew, ScrollPosition, TitleAndDescriptionArgs, TitleAndDescriptionResult } from '../../common/views'; +import { CancelCreatePullRequestNew, ChooseBaseRemoteAndBranchResult, ChooseCompareRemoteAndBranchResult, ChooseRemoteAndBranchArgs, CreateParamsNew, CreatePullRequestNew, ScrollPosition, StackCandidate, TitleAndDescriptionArgs, TitleAndDescriptionResult } from '../../common/views'; import { compareIgnoreCase } from '../../src/common/utils'; import { PreReviewState } from '../../src/github/views'; @@ -32,12 +32,25 @@ const defaultCreateParams: CreateParamsNew = { generateTitleAndDescriptionTitle: undefined, initializeWithGeneratedTitleAndDescription: false, baseHasMergeQueue: false, + addToStack: false, preReviewState: PreReviewState.None, preReviewer: undefined, reviewing: false, usingTemplate: false }; +function remoteChanged(next: RemoteInfo | undefined, current: RemoteInfo | undefined): boolean { + return next !== undefined && (next.owner !== current?.owner || next.repositoryName !== current?.repositoryName); +} + +function stackCandidateChanged(next: Partial, current: StackCandidate | undefined): boolean { + if (!Object.prototype.hasOwnProperty.call(next, 'stackCandidate')) { + return false; + } + return next.stackCandidate?.parentPullRequestNumber !== current?.parentPullRequestNumber + || next.stackCandidate?.stackNumber !== current?.stackNumber; +} + export class CreatePRContextNew { public createParams: CreateParamsNew; private _titleStack: string[] = []; @@ -134,14 +147,22 @@ export class CreatePRContextNew { }; const startingBaseOwner = this.createParams.baseRemote?.owner; const startingBaseRepo = this.createParams.baseRemote?.repositoryName; - const response: ChooseBaseRemoteAndBranchResult = await this.postMessage({ + const response: ChooseBaseRemoteAndBranchResult | undefined = await this.postMessage({ command: 'pr.changeBaseRemoteAndBranch', args + }).catch(error => { + this.updateState({ warning: typeof error === 'string' ? error : error instanceof Error ? error.message : 'Unable to change the base branch.' }); + return undefined; }); + if (!response) { + return; + } const updateValues: Partial = { baseRemote: response.baseRemote, baseBranch: response.baseBranch, + stackCandidate: response.stackCandidate, + addToStack: false, createError: '' }; if ((startingBaseOwner !== response.baseRemote.owner) || (startingBaseRepo !== response.baseRemote.repositoryName)) { @@ -177,18 +198,24 @@ export class CreatePRContextNew { currentRemote, currentBranch }; - const response: ChooseCompareRemoteAndBranchResult = await this.postMessage({ - command: 'pr.changeCompareRemoteAndBranch', - args - }); - - const updateValues: Partial = { - compareRemote: response.compareRemote, - compareBranch: response.compareBranch, - createError: '' - }; + try { + const response: ChooseCompareRemoteAndBranchResult = await this.postMessage({ + command: 'pr.changeCompareRemoteAndBranch', + args + }); - this.updateState(updateValues); + const updateValues: Partial = { + compareRemote: response.compareRemote, + compareBranch: response.compareBranch, + stackCandidate: response.stackCandidate, + addToStack: false, + createError: '' + }; + + this.updateState(updateValues); + } catch (error) { + this.updateState({ warning: typeof error === 'string' ? error : error instanceof Error ? error.message : 'Unable to change the merge branch.' }); + } }; public generateTitle = async (useCopilot: boolean): Promise => { @@ -267,8 +294,11 @@ export class CreatePRContextNew { compareOwner: this.createParams.compareRemote!.owner, compareRepo: this.createParams.compareRemote!.repositoryName, draft: !!this.createParams.isDraft, - autoMerge: !!this.createParams.autoMerge, + autoMerge: !!this.createParams.autoMerge && !this.createParams.addToStack, autoMergeMethod: this.createParams.autoMergeMethod, + addToStack: !!this.createParams.addToStack, + stackParentPullRequest: this.createParams.stackCandidate?.parentPullRequestNumber, + stackNumber: this.createParams.stackCandidate?.stackNumber, labels: this.createParams.labels ?? [], projects: this.createParams.projects ?? [], assignees: this.createParams.assignees ?? [], @@ -280,6 +310,9 @@ export class CreatePRContextNew { public submit = async (): Promise => { try { this.updateState({ creating: false }); + if (this.createParams.addToStack && !this.createParams.stackCandidate) { + throw new Error('The selected base branch is no longer eligible for a pull request stack.'); + } const args: CreatePullRequestNew = this.copyParams(); await this.postMessage({ command: 'pr.create', @@ -305,6 +338,16 @@ export class CreatePRContextNew { if (!message.params) { return; } + const current = this.createParams; + const branchChanged = (message.params.baseBranch !== undefined && message.params.baseBranch !== current.baseBranch) + || (message.params.compareBranch !== undefined && message.params.compareBranch !== current.compareBranch); + const selectionChanged = branchChanged + || remoteChanged(message.params.baseRemote, current.baseRemote) + || remoteChanged(message.params.compareRemote, current.compareRemote) + || stackCandidateChanged(message.params, current.stackCandidate); + if (selectionChanged) { + message.params.addToStack = false; + } if (this.createParams.pendingTitle === undefined) { message.params.pendingTitle = message.params.defaultTitle; } @@ -362,6 +405,7 @@ export class CreatePRContextNew { message.params.compareBranch = message.params.defaultCompareBranch ?? this.createParams.compareBranch; message.params.compareRemote = message.params.defaultCompareRemote ?? this.createParams.compareRemote; message.params.autoMerge = (message.params.autoMergeDefault !== undefined ? message.params.autoMergeDefault : this.createParams.autoMerge); + message.params.addToStack = false; message.params.autoMergeMethod = (message.params.defaultMergeMethod !== undefined ? message.params.defaultMergeMethod : this.createParams.autoMergeMethod); message.params.isDraft = (message.params.isDraftDefault !== undefined ? message.params.isDraftDefault : this.createParams.isDraft); if (message.params.autoMergeDefault) { diff --git a/webviews/createPullRequestViewNew/app.tsx b/webviews/createPullRequestViewNew/app.tsx index 0cf9c32f50..51cc13daf3 100644 --- a/webviews/createPullRequestViewNew/app.tsx +++ b/webviews/createPullRequestViewNew/app.tsx @@ -6,18 +6,70 @@ import React, { useCallback, useContext, useEffect, useRef, useState } from 'react'; import { render } from 'react-dom'; import { RemoteInfo } from '../../common/types'; -import { CreateParamsNew } from '../../common/views'; +import { CreateParamsNew, StackCandidate } from '../../common/views'; import { isITeam, MergeMethod } from '../../src/github/interface'; import { ChangeTemplateReply } from '../../src/github/views'; import PullRequestContextNew from '../common/createContextNew'; import { ErrorBoundary } from '../common/errorBoundary'; import { LabelCreate } from '../common/label'; import { ContextDropdown } from '../components/contextDropdown'; -import { accountIcon, feedbackIcon, gitCompareIcon, milestoneIcon, notebookTemplate, prMergeIcon, projectIcon, settingsIcon, sparkleIcon, stopCircleIcon, tagIcon } from '../components/icon'; +import { accountIcon, feedbackIcon, gitCompareIcon, layersIcon, milestoneIcon, notebookTemplate, prMergeIcon, projectIcon, settingsIcon, sparkleIcon, stopCircleIcon, tagIcon } from '../components/icon'; import { Avatar } from '../components/user'; type CreateMethod = 'create-draft' | 'create' | 'create-automerge-squash' | 'create-automerge-rebase' | 'create-automerge-merge'; +interface CreateMenuContext { + 'preventDefaultContextMenuItems': true; + 'github:createPrMenu': true; + 'github:createPrMenuDraft': true; + 'github:createPrMenuMergeWhenReady'?: true; + 'github:createPrMenuMerge'?: true; + 'github:createPrMenuSquash'?: true; + 'github:createPrMenuRebase'?: true; +} + +export const StackOption = ({ candidate, checked, disabled, onChange }: { candidate: StackCandidate; checked: boolean; disabled: boolean; onChange: (checked: boolean) => void }) => ( +
+ + onChange(event.target.checked)} /> + +
+ + + This pull request will be stacked with #{candidate.parentPullRequestNumber} + {candidate.size > 1 ? ` and ${candidate.size - 1} other pull request${candidate.size === 2 ? '' : 's'}` : ''}. + +
+
+); + +export function makeCreateMenuContext(createParams: CreateParamsNew): string { + const createMenuContexts: CreateMenuContext = { + 'preventDefaultContextMenuItems': true, + 'github:createPrMenu': true, + 'github:createPrMenuDraft': true + }; + if (!createParams.addToStack) { + if (createParams.baseHasMergeQueue) { + createMenuContexts['github:createPrMenuMergeWhenReady'] = true; + } else { + if (createParams.allowAutoMerge && createParams.mergeMethodsAvailability?.merge) { + createMenuContexts['github:createPrMenuMerge'] = true; + } + if (createParams.allowAutoMerge && createParams.mergeMethodsAvailability?.squash) { + createMenuContexts['github:createPrMenuSquash'] = true; + } + if (createParams.allowAutoMerge && createParams.mergeMethodsAvailability?.rebase) { + createMenuContexts['github:createPrMenuRebase'] = true; + } + } + } + return JSON.stringify(createMenuContexts); +} + export const ChooseRemoteAndBranch = ({ onClick, defaultRemote, defaultBranch, isBase, remoteCount = 0, disabled }: { onClick: (remote?: RemoteInfo, branch?: string) => Promise, defaultRemote: RemoteInfo | undefined, defaultBranch: string | undefined, isBase: boolean, remoteCount: number | undefined, disabled: boolean }) => { @@ -131,33 +183,10 @@ export function main() { autoMergeMethod = 'merge'; break; } - ctx.updateState({ isDraft, autoMerge, autoMergeMethod }); + ctx.updateState({ isDraft, autoMerge: autoMerge && !params.addToStack, autoMergeMethod }); return create(); }; - function makeCreateMenuContext(createParams: CreateParamsNew) { - const createMenuContexts = { - 'preventDefaultContextMenuItems': true, - 'github:createPrMenu': true, - 'github:createPrMenuDraft': true - }; - if (createParams.baseHasMergeQueue) { - createMenuContexts['github:createPrMenuMergeWhenReady'] = true; - } else { - if (createParams.allowAutoMerge && createParams.mergeMethodsAvailability && createParams.mergeMethodsAvailability['merge']) { - createMenuContexts['github:createPrMenuMerge'] = true; - } - if (createParams.allowAutoMerge && createParams.mergeMethodsAvailability && createParams.mergeMethodsAvailability['squash']) { - createMenuContexts['github:createPrMenuSquash'] = true; - } - if (createParams.allowAutoMerge && createParams.mergeMethodsAvailability && createParams.mergeMethodsAvailability['rebase']) { - createMenuContexts['github:createPrMenuRebase'] = true; - } - } - const stringified = JSON.stringify(createMenuContexts); - return stringified; - } - if (params.creating) { create(); } @@ -357,6 +386,10 @@ export function main() { disabled={!ctx.initialized || isBusy || isGeneratingTitle || params.reviewing}> + {params.stackCandidate ? ctx.updateState({ addToStack: checked, autoMerge: checked ? false : params.autoMerge })} /> : null} +
{params.createError} @@ -375,8 +408,8 @@ export function main() { makeCreateMenuContext(params)} defaultAction={onCreateButton} - defaultOptionLabel={() => createMethodLabel(ctx.createParams.isDraft, ctx.createParams.autoMerge, ctx.createParams.autoMergeMethod, ctx.createParams.baseHasMergeQueue).label} - defaultOptionValue={() => createMethodLabel(ctx.createParams.isDraft, ctx.createParams.autoMerge, ctx.createParams.autoMergeMethod, ctx.createParams.baseHasMergeQueue).value} + defaultOptionLabel={() => createMethodLabel(ctx.createParams.isDraft, !ctx.createParams.addToStack && ctx.createParams.autoMerge, ctx.createParams.autoMergeMethod, ctx.createParams.baseHasMergeQueue).label} + defaultOptionValue={() => createMethodLabel(ctx.createParams.isDraft, !ctx.createParams.addToStack && ctx.createParams.autoMerge, ctx.createParams.autoMergeMethod, ctx.createParams.baseHasMergeQueue).value} optionsTitle='Create with Option' disabled={isBusy || isGeneratingTitle || params.reviewing || !ctx.isCreatable || !ctx.initialized} spreadable={true} diff --git a/webviews/createPullRequestViewNew/index.css b/webviews/createPullRequestViewNew/index.css index 4a598d4564..7d605c234b 100644 --- a/webviews/createPullRequestViewNew/index.css +++ b/webviews/createPullRequestViewNew/index.css @@ -169,6 +169,45 @@ button.input-box { max-height: 500px; } +.stack-option { + display: flex; + gap: 8px; + align-items: flex-start; + padding: 12px; + margin-top: 12px; + border-radius: 4px; + background: var(--vscode-panel-background); +} + +.stack-option-content { + min-width: 0; +} + +.stack-option-title { + display: flex; + align-items: flex-start; + gap: 6px; + cursor: pointer; +} + +.stack-option-title .icon { + display: flex; + align-items: center; + flex-shrink: 0; + height: 18px; +} + +.stack-option-title > span:last-child { + min-width: 0; +} + +.stack-option-description { + display: block; + margin-top: 4px; + color: var(--vscode-descriptionForeground); + font-size: 0.9em; +} + input[type=text], textarea { padding: 5px; diff --git a/webviews/createPullRequestViewNew/test/app.test.tsx b/webviews/createPullRequestViewNew/test/app.test.tsx new file mode 100644 index 0000000000..ca79dd7e9f --- /dev/null +++ b/webviews/createPullRequestViewNew/test/app.test.tsx @@ -0,0 +1,215 @@ +/*--------------------------------------------------------------------------------------------- + * 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 * as React from 'react'; +import { unmountComponentAtNode } from 'react-dom'; +import { act, cleanup, fireEvent, render } from 'react-testing-library'; +import { createSandbox, SinonSandbox } from 'sinon'; +import { StackCandidate } from '../../../common/views'; +import { CreatePRContextNew } from '../../common/createContextNew'; +import { MessageHandler, vscode } from '../../common/message'; +import { main, makeCreateMenuContext, StackOption } from '../app'; + +describe('Create pull request stack', function () { + let sinon: SinonSandbox; + let previousState: ReturnType; + const candidate: StackCandidate = { parentPullRequestNumber: 795, stackNumber: 12, size: 3, url: 'https://github.com/owner/repo/pull/795' }; + + beforeEach(function () { + sinon = createSandbox(); + previousState = vscode.getState(); + vscode.setState(undefined); + }); + + afterEach(function () { + cleanup(); + const app = document.getElementById('app'); + if (app) { + unmountComponentAtNode(app); + app.remove(); + } + CreatePRContextNew.instance.onchange = null; + vscode.setState(previousState); + sinon.restore(); + }); + + it('shows the stack option unchecked with the parent and stack size', function () { + const onChange = sinon.spy(); + const out = render(); + + const checkbox = out.getByLabelText(/Add this pull request to a stack/) as HTMLInputElement; + assert.strictEqual(checkbox.checked, false); + assert(checkbox.parentElement?.classList.contains('checkbox-wrapper')); + assert.strictEqual(out.container.querySelector('.stack-option-description')?.textContent, + 'This pull request will be stacked with #795 and 2 other pull requests.'); + const link = out.getByText('#795') as HTMLAnchorElement; + assert.strictEqual(link.href, candidate.url); + link.addEventListener('click', event => event.preventDefault()); + fireEvent.click(link); + assert(onChange.notCalled); + fireEvent.click(checkbox); + assert(onChange.calledOnceWithExactly(true)); + }); + + it('offers to create a new stack when the base PR is not already stacked', function () { + const out = render( { }} />); + + assert(out.getByLabelText(/Create a stack with this pull request/)); + assert(out.getByText('#795')); + assert.strictEqual(out.queryByText(/other pull requests/), null); + }); + + it('hides auto-merge menu choices only when adding to the stack', function () { + const context = new CreatePRContextNew(); + const params = { + ...context.createParams, + allowAutoMerge: true, + baseHasMergeQueue: false, + mergeMethodsAvailability: { merge: true, squash: true, rebase: true }, + }; + assert.strictEqual(JSON.parse(makeCreateMenuContext({ ...params, addToStack: false }))['github:createPrMenuSquash'], true); + const stacked = JSON.parse(makeCreateMenuContext({ ...params, addToStack: true })); + assert.strictEqual(stacked['github:createPrMenuDraft'], true); + assert.strictEqual(stacked['github:createPrMenuSquash'], undefined); + assert.strictEqual(JSON.parse(makeCreateMenuContext({ ...params, baseHasMergeQueue: true, addToStack: true }))['github:createPrMenuMergeWhenReady'], undefined); + }); + + it('updates the create button and menu when stacking is checked', function () { + const app = document.createElement('div'); + app.id = 'app'; + document.body.appendChild(app); + const context = CreatePRContextNew.instance; + context.updateState({ + defaultBaseRemote: { owner: 'owner', repositoryName: 'repo' }, + defaultBaseBranch: 'D3', + baseRemote: { owner: 'owner', repositoryName: 'repo' }, + baseBranch: 'D3', + compareRemote: { owner: 'owner', repositoryName: 'repo' }, + compareBranch: 'D4', + pendingTitle: 'Fourth change', + pendingDescription: '', + stackCandidate: candidate, + allowAutoMerge: true, + autoMerge: true, + autoMergeMethod: 'squash', + mergeMethodsAvailability: { merge: true, squash: true, rebase: true }, + }, true); + act(() => { main(); }); + + const checkbox = app.querySelector('.stack-option input'); + assert(checkbox); + const primary = app.querySelector('.group-actions .split-left'); + const menu = app.querySelector('.group-actions .split-right'); + assert.strictEqual(primary?.textContent, 'Create + Auto-Squash'); + assert.strictEqual(JSON.parse(menu!.getAttribute('data-vscode-context')!)['github:createPrMenuSquash'], true); + + act(() => { fireEvent.click(checkbox); }); + + assert.strictEqual(checkbox.checked, true); + assert.strictEqual(primary?.textContent, 'Create'); + assert.strictEqual(JSON.parse(menu!.getAttribute('data-vscode-context')!)['github:createPrMenuSquash'], undefined); + + act(() => { fireEvent.click(checkbox); }); + assert.strictEqual(checkbox.checked, false); + assert.strictEqual(JSON.parse(menu!.getAttribute('data-vscode-context')!)['github:createPrMenuSquash'], true); + }); + + it('submits a stack request without auto-merge even when it was previously selected', async function () { + const handler = new MessageHandler(null); + const postMessage = sinon.stub(handler, 'postMessage').resolves({}); + const context = new CreatePRContextNew(null, handler); + context.updateState({ + baseRemote: { owner: 'owner', repositoryName: 'repo' }, + baseBranch: 'D3', + compareRemote: { owner: 'owner', repositoryName: 'repo' }, + compareBranch: 'D4', + pendingTitle: 'Fourth change', + pendingDescription: '', + stackCandidate: candidate, + addToStack: true, + autoMerge: true, + }); + + await context.submit(); + + assert(postMessage.calledOnce); + assert.strictEqual(postMessage.firstCall.args[0].args.addToStack, true); + assert.strictEqual(postMessage.firstCall.args[0].args.stackParentPullRequest, 795); + assert.strictEqual(postMessage.firstCall.args[0].args.stackNumber, 12); + assert.strictEqual(postMessage.firstCall.args[0].args.autoMerge, false); + }); + + it('clears a checked stack option when the base branch changes', async function () { + const handler = new MessageHandler(null); + sinon.stub(handler, 'postMessage').resolves({ + baseRemote: { owner: 'owner', repositoryName: 'repo' }, + baseBranch: 'D2', + stackCandidate: { parentPullRequestNumber: 794, size: 1, url: 'https://github.com/owner/repo/pull/794' }, + }); + const context = new CreatePRContextNew(null, handler); + context.updateState({ + baseRemote: { owner: 'owner', repositoryName: 'repo' }, + baseBranch: 'D3', + stackCandidate: candidate, + addToStack: true, + }); + + await context.changeBaseRemoteAndBranch(context.createParams.baseRemote, 'D3'); + + assert.strictEqual(context.createParams.baseBranch, 'D2'); + assert.strictEqual(context.createParams.addToStack, false); + assert.strictEqual(context.createParams.stackCandidate?.parentPullRequestNumber, 794); + }); + + it('clears stack selection only when branch, repository or stack membership changes', async function () { + const cases = [ + { baseBranch: 'D2' }, + { compareBranch: 'D5' }, + { baseRemote: { owner: 'other', repositoryName: 'repo' } }, + { compareRemote: { owner: 'owner', repositoryName: 'other' } }, + { stackCandidate: undefined }, + { stackCandidate: { ...candidate, stackNumber: 13 } }, + ]; + for (const params of cases) { + const context = new CreatePRContextNew(); + context.updateState({ + baseRemote: { owner: 'owner', repositoryName: 'repo' }, + compareRemote: { owner: 'owner', repositoryName: 'repo' }, + baseBranch: 'D3', + compareBranch: 'D4', + stackCandidate: candidate, + addToStack: true, + }); + await context.handleMessage({ command: 'pr.initialize', params }); + assert.strictEqual(context.createParams.addToStack, false); + } + + const context = new CreatePRContextNew(); + context.updateState({ stackCandidate: candidate, addToStack: true }); + await context.handleMessage({ command: 'pr.initialize', params: { pendingTitle: 'Fourth change' } }); + assert.strictEqual(context.createParams.addToStack, true); + await context.handleMessage({ command: 'pr.initialize', params: { stackCandidate: { ...candidate, url: 'https://example.com/795' } } }); + assert.strictEqual(context.createParams.addToStack, true); + }); + + it('keeps the selected branch and stack option when changing compare branches fails', async function () { + const handler = new MessageHandler(null); + sinon.stub(handler, 'postMessage').rejects(new Error('Branch does not exist locally.')); + const context = new CreatePRContextNew(null, handler); + context.updateState({ + compareRemote: { owner: 'owner', repositoryName: 'repo' }, + compareBranch: 'D4', + stackCandidate: candidate, + addToStack: true, + }); + + await context.changeMergeRemoteAndBranch(context.createParams.compareRemote, context.createParams.compareBranch); + + assert.strictEqual(context.createParams.compareBranch, 'D4'); + assert.strictEqual(context.createParams.addToStack, true); + assert.strictEqual(context.createParams.warning, 'Branch does not exist locally.'); + }); +}); From 15205d5fa8c4416f379e1a9605238416c0baa0d1 Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:49:58 +0200 Subject: [PATCH 2/3] Address PR comments --- common/views.ts | 2 + src/github/createPRViewProvider.ts | 44 +++++++++++++------ src/github/githubRepository.ts | 10 ++++- src/test/github/createPRViewProvider.test.ts | 29 ++++++++++++ src/test/github/pullRequestModel.test.ts | 35 +++++++++++++++ webviews/common/createContextNew.ts | 6 ++- .../test/app.test.tsx | 41 +++++++++++++++++ 7 files changed, 150 insertions(+), 17 deletions(-) diff --git a/common/views.ts b/common/views.ts index bdec0fa4c7..0c8d58bd28 100644 --- a/common/views.ts +++ b/common/views.ts @@ -157,6 +157,7 @@ export interface ChooseBaseRemoteAndBranchResult { autoMergeDefault: boolean; baseHasMergeQueue: boolean; stackCandidate?: StackCandidate; + warning?: string; defaultTitle: string; defaultDescription: string; } @@ -166,6 +167,7 @@ export interface ChooseCompareRemoteAndBranchResult { compareBranch: string; defaultCompareBranch: string; stackCandidate?: StackCandidate; + warning?: string; } export interface StackCandidate { diff --git a/src/github/createPRViewProvider.ts b/src/github/createPRViewProvider.ts index 02975b2f49..28f5ca0713 100644 --- a/src/github/createPRViewProvider.ts +++ b/src/github/createPRViewProvider.ts @@ -747,9 +747,25 @@ export class CreatePullRequestViewProvider extends BaseCreatePullRequestViewProv return; } this._defaultCompareBranch = compareBranch.name; - this.model.setCompareBranch(compareBranch.name); - this.changeBranch(compareBranch.name, false).then(async titleAndDescription => { - if (compareBranch.name !== this.model.compareBranch) { + const { baseOwner, baseBranch, compareOwner } = this.model; + const isCurrentSelection = () => + this.model.baseOwner === baseOwner && this.model.baseBranch === baseBranch + && this.model.compareOwner === compareOwner && this.model.compareBranch === compareBranch.name; + try { + const titleAndDescription = await this.changeBranch(compareBranch.name, false); + if (!isCurrentSelection()) { + return; + } + const [warning, stackCandidate] = await Promise.all([ + this.existingPRMessage(), + this.getStackCandidateForView( + { owner: baseOwner, repositoryName: this.model.repositoryName }, + baseBranch, + { owner: compareOwner, repositoryName: this.model.repositoryName }, + compareBranch.name + ), + ]); + if (!isCurrentSelection()) { return; } const params: Partial = { @@ -757,20 +773,17 @@ export class CreatePullRequestViewProvider extends BaseCreatePullRequestViewProv defaultDescription: titleAndDescription.description, compareBranch: compareBranch.name, defaultCompareBranch: compareBranch.name, - warning: await this.existingPRMessage(), - stackCandidate: await this.getStackCandidateForView( - { owner: this.model.baseOwner, repositoryName: this.model.repositoryName }, - this.model.baseBranch, - { owner: this.model.compareOwner, repositoryName: this.model.repositoryName }, - this.model.compareBranch - ), + warning, + stackCandidate, }; - return this._postMessage({ + await this._postMessage({ command: 'pr.initialize', params, }); - }); - + } catch (error) { + Logger.error(`Failed to change the default compare branch: ${formatError(error)}`, CreatePullRequestViewProvider.ID); + void vscode.window.showErrorMessage(vscode.l10n.t('Unable to change compare branch: {0}', formatError(error))); + } } public override show(compareBranch?: Branch): void { @@ -1158,6 +1171,7 @@ Don't forget to commit your template file to the repository so that it can be us let chooseResult: ChooseBaseRemoteAndBranchResult | ChooseCompareRemoteAndBranchResult; if (isBase) { + const warning = await this.existingPRMessage(); commands.setContext(contexts.CREATE_PR_PERMISSIONS, viewerPermission); const baseRemoteChanged = this.model.baseOwner !== result.remote.owner; const baseBranchChanged = baseRemoteChanged || this.model.baseBranch !== result.branch; @@ -1188,6 +1202,7 @@ Don't forget to commit your template file to the repository so that it can be us baseHasMergeQueue: !!mergeQueueMethodForBranch, stackCandidate: await this.getStackCandidateForView(result.remote, result.branch, { owner: this.model.compareOwner, repositoryName: this.model.repositoryName }, this.model.compareBranch), + warning, mergeMethodsAvailability: mergeConfiguration?.mergeMethodsAvailability ?? { merge: true, squash: true, rebase: true }, autoMergeDefault, defaultTitle: titleAndDescription.title, @@ -1219,7 +1234,8 @@ Don't forget to commit your template file to the repository so that it can be us defaultCompareBranch: defaultBranch, stackCandidate: await this.getStackCandidateForView( { owner: this.model.baseOwner, repositoryName: this.model.repositoryName }, - this.model.baseBranch, result.remote, result.branch) + this.model.baseBranch, result.remote, result.branch), + warning: await this.existingPRMessage() }; } catch (error) { const ownerChanged = this.model.compareOwner !== previousOwner; diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index ebf0583dc8..1f9a9ff3d6 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -842,7 +842,15 @@ export class GitHubRepository extends Disposable { headers: { 'X-GitHub-Api-Version': '2026-03-10' }, }))); } catch (error) { - if (isObject(error) && error.status === 404) { + const response = isObject(error) && isObject(error.response) ? error.response.data : undefined; + const unsupportedVersion = isObject(response) && [response.message, response.errors].some(detail => { + if (typeof detail !== 'string') { + return false; + } + const text = detail.toLowerCase(); + return text.includes('version') && (text.includes('not supported') || text.includes('not a supported version')); + }); + if (isObject(error) && (error.status === 404 || (error.status === 400 && unsupportedVersion))) { Logger.debug('Pull request stacks are not supported by this GitHub server.', this.id); return; } diff --git a/src/test/github/createPRViewProvider.test.ts b/src/test/github/createPRViewProvider.test.ts index cc61f733e3..2ee262e3e7 100644 --- a/src/test/github/createPRViewProvider.test.ts +++ b/src/test/github/createPRViewProvider.test.ts @@ -36,6 +36,14 @@ import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; class TestCreatePullRequestViewProvider extends CreatePullRequestViewProvider { failTitleAndDescription = false; + public override getStackCandidateForView(baseRemote: RemoteInfo | undefined, baseBranch: string | undefined, compareRemote: RemoteInfo | undefined, compareBranch: string | undefined) { + return super.getStackCandidateForView(baseRemote, baseBranch, compareRemote, compareBranch); + } + + public override _postMessage(message: any) { + return super._postMessage(message); + } + protected override async getTitleAndDescription(_compareBranch: Branch, _baseBranch: string) { if (this.failTitleAndDescription) { throw new Error('Unable to compute the title'); @@ -142,6 +150,27 @@ describe('Create pull request stack', function () { assert(getCandidate.calledWithExactly('D3')); }); + it('does not post a stale default compare branch after its stack lookup completes', async function () { + let releaseLookup!: (candidate: StackCandidate | undefined) => void; + const pendingLookup = new Promise(resolve => { releaseLookup = resolve; }); + let lookupStarted!: () => void; + const started = new Promise(resolve => { lookupStarted = resolve; }); + sinon.stub(provider, 'getStackCandidateForView').callsFake(async () => { + lookupStarted(); + return pendingLookup; + }); + const postMessage = sinon.stub(provider, '_postMessage').resolves(); + + const update = provider.setDefaultCompareBranch(await repository.getBranch('D4')); + await started; + await repository.createBranch('D5', false, 'new-commit-sha'); + await model.setCompareBranch('D5'); + releaseLookup({ parentPullRequestNumber: 795, size: 1, url: 'https://github.com/github/test/pull/795' }); + await update; + + assert(postMessage.getCalls().every(call => call.args[0].params?.compareBranch !== 'D4')); + }); + it('keeps the old owner when the selected compare branch is not available locally', async function () { sinon.stub(githubRepository, 'getViewerPermission').resolves(ViewerPermission.Write); sinon.stub(githubRepository, 'getDefaultBranch').resolves('main'); diff --git a/src/test/github/pullRequestModel.test.ts b/src/test/github/pullRequestModel.test.ts index 46014956bd..3e0a401bee 100644 --- a/src/test/github/pullRequestModel.test.ts +++ b/src/test/github/pullRequestModel.test.ts @@ -505,6 +505,41 @@ describe('PullRequestModel', function () { assert.strictEqual(await repo.getStackCandidate('D3'), undefined); }); + it('does not offer stack creation when the requested API version is unsupported', async function () { + sinon.stub(repo, 'getPullRequestForBranch').resolves(parentModel()); + repo.queryProvider.expectOctokitError(['request'], [listRoute, listParams], Object.assign(new Error('Bad Request'), { + status: 400, + response: { + data: { + message: 'Bad Request', + errors: 'The version you specified in the "X-GitHub-API-Version" request header, "2026-03-10", is not a supported version.', + }, + }, + })); + + assert.strictEqual(await repo.getStackCandidate('D3'), undefined); + }); + + it('recognizes the Enterprise unsupported API version response', async function () { + sinon.stub(repo, 'getPullRequestForBranch').resolves(parentModel()); + repo.queryProvider.expectOctokitError(['request'], [listRoute, listParams], Object.assign(new Error('Bad Request'), { + status: 400, + response: { data: { message: 'The requested API version is not supported' } }, + })); + + assert.strictEqual(await repo.getStackCandidate('D3'), undefined); + }); + + it('does not hide unrelated validation errors as unsupported API versions', async function () { + sinon.stub(repo, 'getPullRequestForBranch').resolves(parentModel()); + repo.queryProvider.expectOctokitError(['request'], [listRoute, listParams], Object.assign(new Error('Bad Request'), { + status: 400, + response: { data: { message: 'Validation failed', errors: 'pull_request must be an integer' } }, + })); + + await assert.rejects(repo.getStackCandidate('D3'), /Bad Request/); + }); + it('reports failures other than unsupported Stacks API', async function () { sinon.stub(repo, 'getPullRequestForBranch').resolves(parentModel()); repo.queryProvider.expectOctokitError(['request'], [listRoute, listParams], Object.assign(new Error('Forbidden'), { status: 403 })); diff --git a/webviews/common/createContextNew.ts b/webviews/common/createContextNew.ts index bce5f17fc8..7dfb26a9a0 100644 --- a/webviews/common/createContextNew.ts +++ b/webviews/common/createContextNew.ts @@ -163,7 +163,8 @@ export class CreatePRContextNew { baseBranch: response.baseBranch, stackCandidate: response.stackCandidate, addToStack: false, - createError: '' + createError: '', + warning: response.warning }; if ((startingBaseOwner !== response.baseRemote.owner) || (startingBaseRepo !== response.baseRemote.repositoryName)) { updateValues.defaultMergeMethod = response.defaultMergeMethod; @@ -209,7 +210,8 @@ export class CreatePRContextNew { compareBranch: response.compareBranch, stackCandidate: response.stackCandidate, addToStack: false, - createError: '' + createError: '', + warning: response.warning }; this.updateState(updateValues); diff --git a/webviews/createPullRequestViewNew/test/app.test.tsx b/webviews/createPullRequestViewNew/test/app.test.tsx index ca79dd7e9f..5a61966574 100644 --- a/webviews/createPullRequestViewNew/test/app.test.tsx +++ b/webviews/createPullRequestViewNew/test/app.test.tsx @@ -212,4 +212,45 @@ describe('Create pull request stack', function () { assert.strictEqual(context.createParams.addToStack, true); assert.strictEqual(context.createParams.warning, 'Branch does not exist locally.'); }); + + it('clears a stale branch warning after a successful selection of the same base branch', async function () { + const handler = new MessageHandler(null); + sinon.stub(handler, 'postMessage').resolves({ + baseRemote: { owner: 'owner', repositoryName: 'repo' }, + baseBranch: 'D3', + stackCandidate: candidate, + warning: undefined, + }); + const context = new CreatePRContextNew(null, handler); + context.updateState({ + baseRemote: { owner: 'owner', repositoryName: 'repo' }, + baseBranch: 'D3', + stackCandidate: candidate, + warning: 'Unable to change the base branch.', + }); + + await context.changeBaseRemoteAndBranch(context.createParams.baseRemote, 'D3'); + + assert.strictEqual(context.createParams.warning, undefined); + }); + + it('replaces a stale compare-branch warning with the current server warning', async function () { + const handler = new MessageHandler(null); + sinon.stub(handler, 'postMessage').resolves({ + compareRemote: { owner: 'owner', repositoryName: 'repo' }, + compareBranch: 'D4', + stackCandidate: candidate, + warning: 'A pull request already exists for this branch.', + }); + const context = new CreatePRContextNew(null, handler); + context.updateState({ + compareRemote: { owner: 'owner', repositoryName: 'repo' }, + compareBranch: 'D4', + warning: 'Unable to change the merge branch.', + }); + + await context.changeMergeRemoteAndBranch(context.createParams.compareRemote, 'D4'); + + assert.strictEqual(context.createParams.warning, 'A pull request already exists for this branch.'); + }); }); From 07fc0d8ddc9f9bb767a2511a281af1ea0d926a2b Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:32:12 +0200 Subject: [PATCH 3/3] Fix test --- src/test/view/prsTree.test.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/view/prsTree.test.ts b/src/test/view/prsTree.test.ts index d9be651570..43b90ac57f 100644 --- a/src/test/view/prsTree.test.ts +++ b/src/test/view/prsTree.test.ts @@ -46,21 +46,23 @@ describe('GitHub Pull Requests view', function () { let mockThemeWatcher: MockThemeWatcher; let mockNotificationsManager: MockNotificationManager; let prsTreeModel: PrsTreeModel; + let discoveredRepository: MockGitHubRepository | undefined; beforeEach(function () { sinon = createSandbox(); + discoveredRepository = undefined; MockCommandRegistry.install(sinon); mockThemeWatcher = new MockThemeWatcher(); context = new MockExtensionContext(); telemetry = new MockTelemetry(); + credentialStore = new CredentialStore(telemetry, context); reposManager = new RepositoriesManager( credentialStore, telemetry, ); prsTreeModel = new PrsTreeModel(telemetry, reposManager, context); - credentialStore = new CredentialStore(telemetry, context); provider = new PullRequestsTreeDataProvider(prsTreeModel, telemetry, context, reposManager); mockNotificationsManager = new MockNotificationManager(); createPrHelper = new CreatePullRequestHelper(); @@ -83,8 +85,18 @@ describe('GitHub Pull Requests view', function () { }); }); + function stubRepositoryDiscovery(folderManager: FolderRepositoryManager): void { + const url = 'git@github.com:aaa/bbb'; + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + const githubRepository = new MockGitHubRepository(remote, credentialStore, telemetry, sinon); + githubRepository.buildMetadata(metadata => metadata.clone_url('https://github.com/aaa/bbb')); + discoveredRepository = githubRepository; + sinon.stub(folderManager, 'createGitHubRepository').resolves(githubRepository); + } + afterEach(function () { provider.dispose(); + discoveredRepository?.dispose(); context.dispose(); sinon.restore(); }); @@ -116,9 +128,11 @@ describe('GitHub Pull Requests view', function () { }); it('opens the viewlet and displays the default categories', async function () { + this.timeout(10000); const repository = new MockRepository(); repository.addRemote('origin', 'git@github.com:aaa/bbb'); const folderManager = new FolderRepositoryManager(0, context, repository, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher); + stubRepositoryDiscovery(folderManager); sinon.stub(folderManager, 'getPullRequestDefaults').returns(Promise.resolve({ owner: 'aaa', repo: 'bbb', base: 'main' })); reposManager.insertFolderManager(folderManager); sinon.stub(credentialStore, 'isAuthenticated').returns(true); @@ -141,6 +155,7 @@ describe('GitHub Pull Requests view', function () { const repository = new MockRepository(); repository.addRemote('origin', 'git@github.com:aaa/bbb'); const folderManager = new FolderRepositoryManager(0, context, repository, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher); + stubRepositoryDiscovery(folderManager); sinon.stub(folderManager, 'getPullRequestDefaults').resolves({ owner: 'aaa', repo: 'bbb', base: 'main' }); reposManager.insertFolderManager(folderManager); sinon.stub(credentialStore, 'isAuthenticated').returns(true); @@ -160,6 +175,7 @@ describe('GitHub Pull Requests view', function () { const repository = new MockRepository(); repository.addRemote('origin', 'git@github.com:aaa/bbb'); const folderManager = new FolderRepositoryManager(0, context, repository, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher); + stubRepositoryDiscovery(folderManager); sinon.stub(folderManager, 'getPullRequestDefaults').returns(Promise.resolve({ owner: 'aaa', repo: 'bbb', base: 'main' })); reposManager.insertFolderManager(folderManager); provider.initialize([], mockNotificationsManager as NotificationsManager);