From 76684c8da97b904ca580e8c1582754a08f7d2b0d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 08:53:00 +0000 Subject: [PATCH 1/5] Initial plan From a59549caefb28c0a8c69e97fec174644c2f28723 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:01:50 +0000 Subject: [PATCH 2/5] Replace branch deletion picker with concise modal actions Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> --- package.nls.json | 6 +- src/github/pullRequestReviewCommon.ts | 56 +++--- .../github/folderRepositoryManager.test.ts | 170 +++++++++++++++++- src/test/github/pullRequestOverview.test.ts | 11 +- 4 files changed, 204 insertions(+), 39 deletions(-) diff --git a/package.nls.json b/package.nls.json index b1942119d2..2425ec25d3 100644 --- a/package.nls.json +++ b/package.nls.json @@ -43,9 +43,9 @@ "githubPullRequests.fileListLayout.description": "The layout to use when displaying changed files list.", "githubPullRequests.hideViewedFiles.description": "Hide files that have been marked as viewed in the pull request changes tree.", "githubPullRequests.fileAutoReveal.description": "Automatically reveal open files in the pull request changes tree.", - "githubPullRequests.defaultDeletionMethod.selectLocalBranch.description": "When true, the option to delete the local branch will be selected by default when deleting a branch from a pull request.", - "githubPullRequests.defaultDeletionMethod.selectRemote.description": "When true, the option to delete the remote will be selected by default when deleting a branch from a pull request.", - "githubPullRequests.defaultDeletionMethod.selectWorktree.description": "When true, the option to remove the associated worktree will be selected by default when deleting a branch from a pull request.", + "githubPullRequests.defaultDeletionMethod.selectLocalBranch.description": "When true, delete the local branch during automatic branch cleanup after merging a pull request or adding it to a merge queue.", + "githubPullRequests.defaultDeletionMethod.selectRemote.description": "When true, delete the unused remote during automatic branch cleanup after merging a pull request.", + "githubPullRequests.defaultDeletionMethod.selectWorktree.description": "When true, remove the associated worktree during automatic branch cleanup after merging a pull request, and select worktree removal by default during bulk branch cleanup.", "githubPullRequests.deleteBranchAfterMerge.description": "Automatically delete the branch after merging a pull request. This setting only applies when the pull request is merged through this extension. When using merge queues, this will only delete the local branch.", "githubPullRequests.enableAttestationCommits.description": "Enables adding an attestation commit (an empty, signed commit) to the head of a pull request branch as a way to attest to a pull request even when its individual commits are unsigned. Requires commit signing to be configured for git. Set to `true` to enable with the default commit message, set to a string to use that string as the commit message, or set to `false` to disable.", "githubPullRequests.terminalLinksHandler.description": "Default handler for terminal links.", diff --git a/src/github/pullRequestReviewCommon.ts b/src/github/pullRequestReviewCommon.ts index 952a7bc48a..6341eff2e8 100644 --- a/src/github/pullRequestReviewCommon.ts +++ b/src/github/pullRequestReviewCommon.ts @@ -308,7 +308,7 @@ export namespace PullRequestReviewCommon { export async function deleteBranch(folderRepositoryManager: FolderRepositoryManager, item: PullRequestModel): Promise<{ isReply: boolean, message: any }> { const branchInfo = await folderRepositoryManager.getBranchNameForPullRequest(item); - const actions: (vscode.QuickPickItem & SelectedAction)[] = []; + const actions: (vscode.MessageItem & SelectedAction & { detail: string })[] = []; const defaultBranch = await folderRepositoryManager.getPullRequestRepositoryDefaultBranch(item); if (item.isResolved()) { @@ -318,53 +318,43 @@ export namespace PullRequestReviewCommon { const isDefaultBranch = defaultBranch === item.head.ref; if (!isDefaultBranch && !item.isRemoteHeadDeleted) { actions.push({ - label: vscode.l10n.t('Delete remote branch {0}', `${headRepo?.remote.remoteName}/${branchHeadRef}`), - description: `${item.remote.normalizedHost}/${item.head.repositoryCloneUrl.owner}/${item.remote.repositoryName}`, + title: vscode.l10n.t('Delete Remote Branch'), + detail: vscode.l10n.t('Delete remote branch {0} ({1})', `${headRepo?.remote.remoteName}/${branchHeadRef}`, `${item.remote.normalizedHost}/${item.head.repositoryCloneUrl.owner}/${item.remote.repositoryName}`), type: 'remoteHead', - picked: true, }); } } if (branchInfo) { - const preferredLocalBranchDeletionMethod = vscode.workspace - .getConfiguration(PR_SETTINGS_NAMESPACE) - .get(`${DEFAULT_DELETION_METHOD}.${SELECT_LOCAL_BRANCH}`); actions.push({ - label: vscode.l10n.t('Delete local branch {0}', branchInfo.branch), + title: vscode.l10n.t('Delete Local Branch'), + detail: vscode.l10n.t('Delete local branch {0}', branchInfo.branch), type: 'local', - picked: !!preferredLocalBranchDeletionMethod, }); - const preferredRemoteDeletionMethod = vscode.workspace - .getConfiguration(PR_SETTINGS_NAMESPACE) - .get(`${DEFAULT_DELETION_METHOD}.${SELECT_REMOTE}`); - if (branchInfo.remote && branchInfo.createdForPullRequest && !branchInfo.remoteInUse) { actions.push({ - label: vscode.l10n.t('Delete remote {0}, which is no longer used by any other branch', branchInfo.remote), + title: vscode.l10n.t('Delete Remote'), + detail: vscode.l10n.t('Delete remote {0}, which is no longer used by any other branch', branchInfo.remote), type: 'remote', - picked: !!preferredRemoteDeletionMethod, }); } const worktreePath = folderRepositoryManager.getWorktreeForBranch(branchInfo.branch); if (worktreePath && !isWorktreeInWorkspace(worktreePath)) { - const preferredWorktreeDeletion = vscode.workspace - .getConfiguration(PR_SETTINGS_NAMESPACE) - .get(`${DEFAULT_DELETION_METHOD}.${SELECT_WORKTREE}`); actions.push({ - label: vscode.l10n.t('Remove worktree {0}', worktreePath.fsPath), + title: vscode.l10n.t('Remove Worktree'), + detail: vscode.l10n.t('Remove worktree {0}', worktreePath.fsPath), type: 'worktree', worktreePath: worktreePath.fsPath, - picked: !!preferredWorktreeDeletion, }); } } if (vscode.env.remoteName === 'codespaces') { actions.push({ - label: vscode.l10n.t('Suspend Codespace'), + title: vscode.l10n.t('Suspend Codespace'), + detail: vscode.l10n.t('Suspend Codespace'), type: 'suspend' }); } @@ -381,14 +371,22 @@ export namespace PullRequestReviewCommon { }; } - const selectedActions = await vscode.window.showQuickPick(actions, { - canPickMany: true, - ignoreFocusOut: true, - }); - - - if (selectedActions) { - const deletedBranchTypes: string[] = await performBranchDeletion(folderRepositoryManager, item, defaultBranch, branchInfo!, selectedActions); + const options: (vscode.MessageItem & { actions: SelectedAction[] })[] = actions.map(action => ({ + title: action.title, + actions: [action], + })); + const deletionActions = actions.filter(action => action.type !== 'suspend'); + if (deletionActions.length > 1) { + options.unshift({ title: vscode.l10n.t('Delete All'), actions: deletionActions }); + } + const selectedOption = await vscode.window.showWarningMessage( + vscode.l10n.t('Choose what to delete for Pull Request #{0}', item.number), + { modal: true, detail: actions.map(action => action.detail).join('\n') }, + ...options, + ); + + if (selectedOption) { + const deletedBranchTypes: string[] = await performBranchDeletion(folderRepositoryManager, item, defaultBranch, branchInfo!, selectedOption.actions); return { isReply: false, diff --git a/src/test/github/folderRepositoryManager.test.ts b/src/test/github/folderRepositoryManager.test.ts index 5ff6a3870c..5cacd2570e 100644 --- a/src/test/github/folderRepositoryManager.test.ts +++ b/src/test/github/folderRepositoryManager.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { default as assert } from 'assert'; -import { createSandbox, SinonSandbox } from 'sinon'; +import { createSandbox, SinonSandbox, SinonStub } from 'sinon'; import { FolderRepositoryManager, titleAndBodyFrom } from '../../github/folderRepositoryManager'; import { MockRepository } from '../mocks/mockRepository'; @@ -19,7 +19,7 @@ import { convertRESTPullRequestToRawPullRequest } from '../../github/utils'; import { GitApiImpl, RefType } from '../../api/api1'; import { CredentialStore } from '../../github/credentials'; import { MockExtensionContext } from '../mocks/mockExtensionContext'; -import { Uri } from 'vscode'; +import { commands, env, MessageItem, MessageOptions, Uri, window, workspace } from 'vscode'; import { GitHubServerType } from '../../common/authentication'; import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; import { RepositoriesManager } from '../../github/repositoriesManager'; @@ -225,6 +225,172 @@ describe('PullRequestManager', function () { }); }); + describe('deleteBranch modal', function () { + let pr: PullRequestModel; + let showWarningMessage: SinonStub; + let deleteRemoteBranch: SinonStub; + let getBranchInfo: SinonStub; + + beforeEach(async function () { + const url = 'https://github.com/aaa/bbb.git'; + const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom); + const githubRepository = new GitHubRepository(1, remote, repository.rootUri, manager.credentialStore, telemetry); + const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().head(head => head.ref('feature')).build(), githubRepository); + pr = new PullRequestModel(manager.credentialStore, telemetry, githubRepository, remote, prItem); + await repository.createBranch('local-feature', false); + getBranchInfo = sinon.stub(manager, 'getBranchNameForPullRequest').resolves({ + branch: 'local-feature', + createdForPullRequest: false, + }); + sinon.stub(manager, 'getPullRequestRepositoryDefaultBranch').resolves('main'); + sinon.stub(manager, 'findRepo').returns(githubRepository); + sinon.stub(env, 'remoteName').value(undefined); + sinon.stub(workspace, 'workspaceFolders').value([]); + showWarningMessage = sinon.stub(window, 'showWarningMessage').resolves(undefined); + deleteRemoteBranch = sinon.stub(manager, 'deleteBranch').resolves(); + sinon.stub(repository, 'fetch').resolves(); + }); + + it('shows concise buttons and branch details in a modal, and cancels without deleting', async function () { + const showQuickPick = sinon.stub(window, 'showQuickPick'); + const deleteLocalBranch = sinon.spy(repository, 'deleteBranch'); + + const result = await PullRequestReviewCommon.deleteBranch(manager, pr); + + assert.deepStrictEqual(result, { isReply: true, message: { cancelled: true } }); + assert.strictEqual(showWarningMessage.calledOnce, true); + assert.strictEqual(showWarningMessage.firstCall.args[0], `Choose what to delete for Pull Request #${pr.number}`); + const options = showWarningMessage.firstCall.args[1] as MessageOptions; + assert.strictEqual(options.modal, true); + assert.ok(options.detail?.includes('origin/feature')); + assert.ok(options.detail?.includes('local-feature')); + assert.deepStrictEqual(showWarningMessage.firstCall.args.slice(2).map((item: MessageItem) => item.title), + ['Delete All', 'Delete Remote Branch', 'Delete Local Branch']); + assert.strictEqual(showQuickPick.notCalled, true); + assert.strictEqual(deleteRemoteBranch.notCalled, true); + assert.strictEqual(deleteLocalBranch.notCalled, true); + }); + + for (const [title, expectedTypes] of [ + ['Delete Remote Branch', ['remoteHead']], + ['Delete Local Branch', ['local']], + ['Delete All', ['local', 'remoteHead']], + ] as const) { + it(`executes only the actions for "${title}"`, async function () { + showWarningMessage.callsFake(async (_message, _options, ...items: MessageItem[]) => items.find(item => item.title === title)); + const deleteLocalBranch = sinon.spy(repository, 'deleteBranch'); + + const result = await PullRequestReviewCommon.deleteBranch(manager, pr); + + assert.strictEqual(result.isReply, false); + assert.strictEqual(result.message.command, 'pr.deleteBranch'); + assert.deepStrictEqual(result.message.branchTypes.sort(), [...expectedTypes]); + assert.strictEqual(deleteRemoteBranch.calledOnce, expectedTypes.some(type => type === 'remoteHead')); + assert.strictEqual(deleteLocalBranch.calledOnce, expectedTypes.some(type => type === 'local')); + if (deleteLocalBranch.calledOnce) { + sinon.assert.calledWithExactly(deleteLocalBranch, 'local-feature', true); + } + }); + } + + it('offers only local deletion when the remote branch has been deleted', async function () { + pr.isRemoteHeadDeleted = true; + + await PullRequestReviewCommon.deleteBranch(manager, pr); + + assert.deepStrictEqual(showWarningMessage.firstCall.args.slice(2).map((item: MessageItem) => item.title), ['Delete Local Branch']); + }); + + it('offers only remote branch deletion when there is no local branch', async function () { + getBranchInfo.resolves(undefined); + showWarningMessage.callsFake(async (_message, _options, ...items: MessageItem[]) => items[0]); + + const result = await PullRequestReviewCommon.deleteBranch(manager, pr); + + assert.deepStrictEqual(showWarningMessage.firstCall.args.slice(2).map((item: MessageItem) => item.title), ['Delete Remote Branch']); + assert.deepStrictEqual(result.message.branchTypes, ['remoteHead']); + }); + + it('does not offer deletion of the default remote branch', async function () { + assert.ok(pr.head); + pr.head.ref = 'main'; + + await PullRequestReviewCommon.deleteBranch(manager, pr); + + assert.deepStrictEqual(showWarningMessage.firstCall.args.slice(2).map((item: MessageItem) => item.title), ['Delete Local Branch']); + }); + + it('warns without showing a modal when there are no actions', async function () { + pr.isRemoteHeadDeleted = true; + getBranchInfo.resolves(undefined); + + const result = await PullRequestReviewCommon.deleteBranch(manager, pr); + + assert.deepStrictEqual(result, { isReply: true, message: { cancelled: true } }); + sinon.assert.calledOnce(showWarningMessage); + assert.strictEqual(showWarningMessage.firstCall.args.length, 1); + assert.strictEqual(deleteRemoteBranch.notCalled, true); + }); + + for (const title of ['Delete Remote', 'Remove Worktree', 'Delete All']) { + it(`supports unused remote and worktree cleanup with "${title}"`, async function () { + getBranchInfo.resolves({ branch: 'local-feature', remote: 'fork', createdForPullRequest: true, remoteInUse: false }); + const worktreePath = Uri.file('/worktrees/local-feature'); + sinon.stub(manager, 'getWorktreeForBranch').returns(worktreePath); + const removeWorktree = sinon.stub(manager, 'removeWorktree').resolves(); + const removeRemote = sinon.stub(repository, 'removeRemote').resolves(); + const deleteLocalBranch = sinon.spy(repository, 'deleteBranch'); + showWarningMessage.callsFake(async (_message, _options, ...items: MessageItem[]) => items.find(item => item.title === title)); + + const result = await PullRequestReviewCommon.deleteBranch(manager, pr); + + assert.deepStrictEqual(showWarningMessage.firstCall.args.slice(2).map((item: MessageItem) => item.title), + ['Delete All', 'Delete Remote Branch', 'Delete Local Branch', 'Delete Remote', 'Remove Worktree']); + assert.ok((showWarningMessage.firstCall.args[1] as MessageOptions).detail?.includes(worktreePath.fsPath)); + assert.ok((showWarningMessage.firstCall.args[1] as MessageOptions).detail?.includes('fork')); + const expectedTypes = title === 'Delete All' ? ['local', 'remote', 'remoteHead', 'worktree'] : title === 'Delete Remote' ? ['remote'] : ['worktree']; + assert.deepStrictEqual(result.message.branchTypes.sort(), expectedTypes); + assert.strictEqual(removeRemote.calledOnce, title !== 'Remove Worktree'); + assert.strictEqual(removeWorktree.calledOnce, title !== 'Delete Remote'); + if (removeWorktree.calledOnce) { + sinon.assert.calledWithExactly(removeWorktree, worktreePath.fsPath); + } + if (title === 'Delete All') { + sinon.assert.callOrder(removeWorktree, deleteLocalBranch); + } + }); + } + + it('does not offer removal of an in-use remote or a worktree in the workspace', async function () { + getBranchInfo.resolves({ branch: 'local-feature', remote: 'fork', createdForPullRequest: true, remoteInUse: true }); + const worktreePath = Uri.file('/worktrees/local-feature'); + sinon.stub(manager, 'getWorktreeForBranch').returns(worktreePath); + sinon.stub(workspace, 'workspaceFolders').value([{ uri: worktreePath, name: 'local-feature', index: 0 }]); + + await PullRequestReviewCommon.deleteBranch(manager, pr); + + assert.deepStrictEqual(showWarningMessage.firstCall.args.slice(2).map((item: MessageItem) => item.title), + ['Delete All', 'Delete Remote Branch', 'Delete Local Branch']); + }); + + for (const title of ['Suspend Codespace', 'Delete All']) { + it(`keeps Codespace suspension separate from deletion with "${title}"`, async function () { + sinon.stub(env, 'remoteName').value('codespaces'); + const executeCommand = sinon.stub(commands, 'executeCommand').resolves(); + showWarningMessage.callsFake(async (_message, _options, ...items: MessageItem[]) => items.find(item => item.title === title)); + + const result = await PullRequestReviewCommon.deleteBranch(manager, pr); + + assert.ok(showWarningMessage.firstCall.args.slice(2).some((item: MessageItem) => item.title === 'Suspend Codespace')); + assert.deepStrictEqual(result.message.branchTypes.sort(), title === 'Suspend Codespace' ? ['suspend'] : ['local', 'remoteHead']); + assert.strictEqual(executeCommand.calledOnce, title === 'Suspend Codespace'); + if (executeCommand.calledOnce) { + sinon.assert.calledWithExactly(executeCommand, 'github.codespaces.disconnectSuspend'); + } + }); + } + }); + describe('deleteRemotes', function () { it('continues deleting remotes after one fails', async function () { await repository.addRemote('locked', 'https://github.com/owner/locked'); diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index d9e2bb8322..e70758f4d6 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -328,7 +328,7 @@ describe('PullRequestOverview', function () { createdForPullRequest: false, }); sinon.stub(pullRequestManager, 'getPullRequestRepositoryDefaultBranch').resolves('main'); - const showQuickPick = sinon.stub(vscode.window, 'showQuickPick').resolves(undefined); + const showWarningMessage = sinon.stub(vscode.window, 'showWarningMessage').resolves(undefined); const replyMessage = sinon.stub(panel as any, '_replyMessage'); await (panel as any).mergePullRequest({ @@ -336,11 +336,12 @@ describe('PullRequestOverview', function () { args: { title: '', description: '', method: 'squash' }, }); - assert.strictEqual(showQuickPick.calledOnce, true); - const actions = showQuickPick.firstCall.args[0] as readonly (vscode.QuickPickItem & { type: string })[]; - assert.strictEqual(actions.some(action => action.type === 'local'), true); + assert.strictEqual(showWarningMessage.calledOnce, true); + assert.strictEqual((showWarningMessage.firstCall.args[1] as vscode.MessageOptions).modal, true); + const actions = showWarningMessage.firstCall.args.slice(2) as vscode.MessageItem[]; + assert.strictEqual(actions.some(action => action.title === 'Delete Local Branch'), true); assert.strictEqual(replyMessage.firstCall.args[1].state, GithubItemStateEnum.Merged); - sinon.assert.callOrder(replyMessage, showQuickPick); + sinon.assert.callOrder(replyMessage, showWarningMessage); }); }); }); From a10231eb16e0a77266edd69a2c9db6c932083bb6 Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Thu, 10 Sep 2026 17:10:22 +0200 Subject: [PATCH 3/5] Better wording --- src/github/activityBarViewProvider.ts | 6 +-- src/github/pullRequestOverview.ts | 6 +-- src/github/pullRequestReviewCommon.ts | 39 +++++++++++++++---- .../github/folderRepositoryManager.test.ts | 37 ++++++++++++++++-- src/test/github/pullRequestOverview.test.ts | 23 +++++++++++ webviews/common/context.tsx | 10 ++++- 6 files changed, 98 insertions(+), 23 deletions(-) diff --git a/src/github/activityBarViewProvider.ts b/src/github/activityBarViewProvider.ts index ae559e5462..27d5282078 100644 --- a/src/github/activityBarViewProvider.ts +++ b/src/github/activityBarViewProvider.ts @@ -447,11 +447,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W private async deleteBranch(message: IRequestMessage) { const result = await PullRequestReviewCommon.deleteBranch(this._folderRepositoryManager, this._item); - if (result.isReply) { - this._replyMessage(message, result.message); - } else { - this._postMessage(result.message); - } + await this._replyMessage(message, result.message); } private async setReadyForReview(message: IRequestMessage>): Promise { diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index e5f613c559..654337c618 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -1033,12 +1033,10 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel) { const result = await PullRequestReviewCommon.deleteBranch(this._folderRepositoryManager, this._item); - if (result.isReply) { - this._replyMessage(message, result.message); - } else { + if (!result.isReply) { this.refreshPanel(); - this._postMessage(result.message); } + await this._replyMessage(message, result.message); } private async setReadyForReview(message: IRequestMessage<{}>): Promise { diff --git a/src/github/pullRequestReviewCommon.ts b/src/github/pullRequestReviewCommon.ts index 6341eff2e8..bb6cf22a39 100644 --- a/src/github/pullRequestReviewCommon.ts +++ b/src/github/pullRequestReviewCommon.ts @@ -306,9 +306,15 @@ export namespace PullRequestReviewCommon { }); } + function isBranchNotFoundError(error: unknown): boolean { + const stderr = error && typeof error === 'object' ? Reflect.get(error, 'stderr') : undefined; + return typeof stderr === 'string' && stderr.includes('not found'); + } + export async function deleteBranch(folderRepositoryManager: FolderRepositoryManager, item: PullRequestModel): Promise<{ isReply: boolean, message: any }> { const branchInfo = await folderRepositoryManager.getBranchNameForPullRequest(item); - const actions: (vscode.MessageItem & SelectedAction & { detail: string })[] = []; + const actions: (vscode.MessageItem & SelectedAction)[] = []; + const cleanupDetails: string[] = []; const defaultBranch = await folderRepositoryManager.getPullRequestRepositoryDefaultBranch(item); if (item.isResolved()) { @@ -317,46 +323,50 @@ export namespace PullRequestReviewCommon { const isDefaultBranch = defaultBranch === item.head.ref; if (!isDefaultBranch && !item.isRemoteHeadDeleted) { + const remoteBranch = headRepo ? `${headRepo.remote.remoteName}/${branchHeadRef}` : branchHeadRef; actions.push({ title: vscode.l10n.t('Delete Remote Branch'), - detail: vscode.l10n.t('Delete remote branch {0} ({1})', `${headRepo?.remote.remoteName}/${branchHeadRef}`, `${item.remote.normalizedHost}/${item.head.repositoryCloneUrl.owner}/${item.remote.repositoryName}`), type: 'remoteHead', }); + cleanupDetails.push( + vscode.l10n.t('Remote branch: {0}', remoteBranch), + vscode.l10n.t('Remote repository: {0}', `${item.remote.normalizedHost}/${item.head.repositoryCloneUrl.owner}/${item.remote.repositoryName}`), + ); } } if (branchInfo) { actions.push({ title: vscode.l10n.t('Delete Local Branch'), - detail: vscode.l10n.t('Delete local branch {0}', branchInfo.branch), type: 'local', }); + cleanupDetails.push(vscode.l10n.t('Local branch: {0}', branchInfo.branch)); if (branchInfo.remote && branchInfo.createdForPullRequest && !branchInfo.remoteInUse) { actions.push({ title: vscode.l10n.t('Delete Remote'), - detail: vscode.l10n.t('Delete remote {0}, which is no longer used by any other branch', branchInfo.remote), type: 'remote', }); + cleanupDetails.push(vscode.l10n.t('Unused Git remote: {0}', branchInfo.remote)); } const worktreePath = folderRepositoryManager.getWorktreeForBranch(branchInfo.branch); if (worktreePath && !isWorktreeInWorkspace(worktreePath)) { actions.push({ title: vscode.l10n.t('Remove Worktree'), - detail: vscode.l10n.t('Remove worktree {0}', worktreePath.fsPath), type: 'worktree', worktreePath: worktreePath.fsPath, }); + cleanupDetails.push(vscode.l10n.t('Worktree: {0}', worktreePath.fsPath)); } } if (vscode.env.remoteName === 'codespaces') { actions.push({ title: vscode.l10n.t('Suspend Codespace'), - detail: vscode.l10n.t('Suspend Codespace'), type: 'suspend' }); + cleanupDetails.push(vscode.l10n.t('Codespace: current Codespace')); } if (!actions.length) { @@ -381,7 +391,13 @@ export namespace PullRequestReviewCommon { } const selectedOption = await vscode.window.showWarningMessage( vscode.l10n.t('Choose what to delete for Pull Request #{0}', item.number), - { modal: true, detail: actions.map(action => action.detail).join('\n') }, + { + modal: true, + detail: vscode.l10n.t( + 'Choose an action below to clean up the resources associated with this pull request.\n\n{0}', + cleanupDetails.join('\n'), + ) + }, ...options, ); @@ -460,7 +476,14 @@ export namespace PullRequestReviewCommon { } await folderRepositoryManager.checkoutDefaultBranch(defaultBranch, item); } - await folderRepositoryManager.repository.deleteBranch(branchInfo!.branch, true); + try { + await folderRepositoryManager.repository.deleteBranch(branchInfo!.branch, true); + } catch (error) { + if (!isBranchNotFoundError(error)) { + throw error; + } + Logger.debug(`Local branch ${branchInfo!.branch} no longer exists.`, 'PullRequestReviewCommon'); + } return deletedBranchTypes.push(action.type); case 'remote': deletedBranchTypes.push(action.type); diff --git a/src/test/github/folderRepositoryManager.test.ts b/src/test/github/folderRepositoryManager.test.ts index 5cacd2570e..d44d2918fd 100644 --- a/src/test/github/folderRepositoryManager.test.ts +++ b/src/test/github/folderRepositoryManager.test.ts @@ -262,8 +262,13 @@ describe('PullRequestManager', function () { assert.strictEqual(showWarningMessage.firstCall.args[0], `Choose what to delete for Pull Request #${pr.number}`); const options = showWarningMessage.firstCall.args[1] as MessageOptions; assert.strictEqual(options.modal, true); - assert.ok(options.detail?.includes('origin/feature')); - assert.ok(options.detail?.includes('local-feature')); + assert.strictEqual(options.detail, [ + 'Choose an action below to clean up the resources associated with this pull request.', + '', + 'Remote branch: origin/feature', + 'Remote repository: github.com/aaa/bbb', + 'Local branch: local-feature', + ].join('\n')); assert.deepStrictEqual(showWarningMessage.firstCall.args.slice(2).map((item: MessageItem) => item.title), ['Delete All', 'Delete Remote Branch', 'Delete Local Branch']); assert.strictEqual(showQuickPick.notCalled, true); @@ -301,6 +306,23 @@ describe('PullRequestManager', function () { assert.deepStrictEqual(showWarningMessage.firstCall.args.slice(2).map((item: MessageItem) => item.title), ['Delete Local Branch']); }); + it('treats a local branch that no longer exists as deleted', async function () { + await repository.deleteBranch('local-feature', true); + showWarningMessage.callsFake(async (_message, _options, ...items: MessageItem[]) => items.find(item => item.title === 'Delete Local Branch')); + + const result = await PullRequestReviewCommon.deleteBranch(manager, pr); + + assert.deepStrictEqual(result.message.branchTypes, ['local']); + }); + + it('rethrows other local branch deletion errors', async function () { + const error = new Error('Unable to delete local branch'); + sinon.stub(repository, 'deleteBranch').rejects(error); + showWarningMessage.callsFake(async (_message, _options, ...items: MessageItem[]) => items.find(item => item.title === 'Delete Local Branch')); + + await assert.rejects(PullRequestReviewCommon.deleteBranch(manager, pr), error); + }); + it('offers only remote branch deletion when there is no local branch', async function () { getBranchInfo.resolves(undefined); showWarningMessage.callsFake(async (_message, _options, ...items: MessageItem[]) => items[0]); @@ -346,8 +368,15 @@ describe('PullRequestManager', function () { assert.deepStrictEqual(showWarningMessage.firstCall.args.slice(2).map((item: MessageItem) => item.title), ['Delete All', 'Delete Remote Branch', 'Delete Local Branch', 'Delete Remote', 'Remove Worktree']); - assert.ok((showWarningMessage.firstCall.args[1] as MessageOptions).detail?.includes(worktreePath.fsPath)); - assert.ok((showWarningMessage.firstCall.args[1] as MessageOptions).detail?.includes('fork')); + assert.strictEqual((showWarningMessage.firstCall.args[1] as MessageOptions).detail, [ + 'Choose an action below to clean up the resources associated with this pull request.', + '', + 'Remote branch: origin/feature', + 'Remote repository: github.com/aaa/bbb', + 'Local branch: local-feature', + 'Unused Git remote: fork', + `Worktree: ${worktreePath.fsPath}`, + ].join('\n')); const expectedTypes = title === 'Delete All' ? ['local', 'remote', 'remoteHead', 'worktree'] : title === 'Delete Remote' ? ['remote'] : ['worktree']; assert.deepStrictEqual(result.message.branchTypes.sort(), expectedTypes); assert.strictEqual(removeRemote.calledOnce, title !== 'Remove Worktree'); diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index e70758f4d6..aae198a9af 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -28,6 +28,7 @@ import { CreatePullRequestHelper } from '../../view/createPullRequestHelper'; import { RepositoriesManager } from '../../github/repositoriesManager'; import { MockThemeWatcher } from '../mocks/mockThemeWatcher'; import { TimelineEvent } from '../../common/timelineEvent'; +import { PullRequestReviewCommon } from '../../github/pullRequestReviewCommon'; const EXTENSION_URI = vscode.Uri.joinPath(vscode.Uri.file(__dirname), '../../..'); @@ -314,6 +315,28 @@ describe('PullRequestOverview', function () { r.pullRequest(pr => pr.number(1000)); }); }); + + describe('deleteBranch', function () { + it('replies with the deletion state after deletion completes', async function () { + const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); + const prModel = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem); + const identity = { owner: prModel.remote.owner, repo: prModel.remote.repositoryName, number: prModel.number }; + await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, prModel); + const panel = PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number)!; + const response = { command: 'pr.deleteBranch', branchTypes: ['local'] }; + sinon.stub(PullRequestReviewCommon, 'deleteBranch').resolves({ isReply: false, message: response }); + const replyMessage = sinon.stub(panel as any, '_replyMessage').resolves(); + const postMessage = sinon.stub(panel as any, '_postMessage').resolves(); + const refreshPanel = sinon.stub(panel as any, 'refreshPanel').resolves(); + const message = { req: '1', command: 'pr.deleteBranch', args: undefined }; + + await (panel as any).deleteBranch(message); + + sinon.assert.calledOnceWithExactly(replyMessage, message, response); + sinon.assert.calledOnce(refreshPanel); + sinon.assert.notCalled(postMessage); + }); + }); }); const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index cc7e617b8e..92c9c7613a 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -98,7 +98,13 @@ export class PRContext { public openOnGitHub = () => this.postMessage({ command: 'pr.openOnGitHub' }); - public deleteBranch = () => this.postMessage({ command: 'pr.deleteBranch' }); + public deleteBranch = async () => { + const result = await this.postMessage({ command: 'pr.deleteBranch' }); + if (result?.command === 'pr.deleteBranch') { + this.handleMessage(result); + } + return result; + }; public revert = async () => { this.updatePR({ busy: true }); @@ -562,7 +568,7 @@ export class PRContext { message.branchTypes && message.branchTypes.map((branchType: string) => { if (branchType === 'local') { stateChange.isLocalHeadDeleted = true; - } else if ((branchType === 'remote') || (branchType === 'upstream')) { + } else if ((branchType === 'remoteHead') || (branchType === 'remote') || (branchType === 'upstream')) { stateChange.isRemoteHeadDeleted = true; } }); From 4a5c1ff18544ed0c8da7a20c10110c3e9f7ea390 Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Thu, 10 Sep 2026 17:24:17 +0200 Subject: [PATCH 4/5] Fix test --- src/test/github/pullRequestOverview.test.ts | 45 +++++++++++---------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/test/github/pullRequestOverview.test.ts b/src/test/github/pullRequestOverview.test.ts index aae198a9af..b7d4b331b3 100644 --- a/src/test/github/pullRequestOverview.test.ts +++ b/src/test/github/pullRequestOverview.test.ts @@ -315,28 +315,6 @@ describe('PullRequestOverview', function () { r.pullRequest(pr => pr.number(1000)); }); }); - - describe('deleteBranch', function () { - it('replies with the deletion state after deletion completes', async function () { - const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); - const prModel = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem); - const identity = { owner: prModel.remote.owner, repo: prModel.remote.repositoryName, number: prModel.number }; - await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, prModel); - const panel = PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number)!; - const response = { command: 'pr.deleteBranch', branchTypes: ['local'] }; - sinon.stub(PullRequestReviewCommon, 'deleteBranch').resolves({ isReply: false, message: response }); - const replyMessage = sinon.stub(panel as any, '_replyMessage').resolves(); - const postMessage = sinon.stub(panel as any, '_postMessage').resolves(); - const refreshPanel = sinon.stub(panel as any, 'refreshPanel').resolves(); - const message = { req: '1', command: 'pr.deleteBranch', args: undefined }; - - await (panel as any).deleteBranch(message); - - sinon.assert.calledOnceWithExactly(replyMessage, message, response); - sinon.assert.calledOnce(refreshPanel); - sinon.assert.notCalled(postMessage); - }); - }); }); const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); @@ -367,4 +345,27 @@ describe('PullRequestOverview', function () { sinon.assert.callOrder(replyMessage, showWarningMessage); }); }); + + describe('deleteBranch', function () { + it('replies with the deletion state after deletion completes', async function () { + const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo); + const prModel = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem); + const identity = { owner: prModel.remote.owner, repo: prModel.remote.repositoryName, number: prModel.number }; + await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, prModel); + const panel = PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number)!; + const response = { command: 'pr.deleteBranch', branchTypes: ['local'] }; + sinon.stub(PullRequestReviewCommon, 'deleteBranch').resolves({ isReply: false, message: response }); + const replyMessage = sinon.stub(panel as any, '_replyMessage').resolves(); + const postMessage = sinon.stub(panel as any, '_postMessage').resolves(); + const refreshPanel = sinon.stub(panel as any, 'refreshPanel').resolves(); + const message = { req: '1', command: 'pr.deleteBranch', args: undefined }; + + await (panel as any).deleteBranch(message); + + sinon.assert.calledOnce(replyMessage); + sinon.assert.calledWithExactly(replyMessage, message, response); + sinon.assert.calledOnce(refreshPanel); + sinon.assert.notCalled(postMessage); + }); + }); }); From 3916e3a1fb743d92e9bfc2778f34402ae63d1681 Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Fri, 11 Sep 2026 10:24:08 +0200 Subject: [PATCH 5/5] Fix another test --- src/github/pullRequestReviewCommon.ts | 4 +++- src/test/github/folderRepositoryManager.test.ts | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/github/pullRequestReviewCommon.ts b/src/github/pullRequestReviewCommon.ts index bb6cf22a39..2692244eb6 100644 --- a/src/github/pullRequestReviewCommon.ts +++ b/src/github/pullRequestReviewCommon.ts @@ -324,13 +324,15 @@ export namespace PullRequestReviewCommon { const isDefaultBranch = defaultBranch === item.head.ref; if (!isDefaultBranch && !item.isRemoteHeadDeleted) { const remoteBranch = headRepo ? `${headRepo.remote.remoteName}/${branchHeadRef}` : branchHeadRef; + const remoteRepository = item.head.repositoryCloneUrl.toString() ?? + `${item.remote.normalizedHost}/${item.head.repositoryCloneUrl.owner}/${item.head.repositoryCloneUrl.repositoryName}`; actions.push({ title: vscode.l10n.t('Delete Remote Branch'), type: 'remoteHead', }); cleanupDetails.push( vscode.l10n.t('Remote branch: {0}', remoteBranch), - vscode.l10n.t('Remote repository: {0}', `${item.remote.normalizedHost}/${item.head.repositoryCloneUrl.owner}/${item.remote.repositoryName}`), + vscode.l10n.t('Remote repository: {0}', remoteRepository), ); } } diff --git a/src/test/github/folderRepositoryManager.test.ts b/src/test/github/folderRepositoryManager.test.ts index d44d2918fd..469ea17542 100644 --- a/src/test/github/folderRepositoryManager.test.ts +++ b/src/test/github/folderRepositoryManager.test.ts @@ -266,7 +266,7 @@ describe('PullRequestManager', function () { 'Choose an action below to clean up the resources associated with this pull request.', '', 'Remote branch: origin/feature', - 'Remote repository: github.com/aaa/bbb', + 'Remote repository: https://github.com/octocat/reponame', 'Local branch: local-feature', ].join('\n')); assert.deepStrictEqual(showWarningMessage.firstCall.args.slice(2).map((item: MessageItem) => item.title), @@ -372,7 +372,7 @@ describe('PullRequestManager', function () { 'Choose an action below to clean up the resources associated with this pull request.', '', 'Remote branch: origin/feature', - 'Remote repository: github.com/aaa/bbb', + 'Remote repository: https://github.com/octocat/reponame', 'Local branch: local-feature', 'Unused Git remote: fork', `Worktree: ${worktreePath.fsPath}`,