From 46741065e53b1777a1cfbf4f8967050529325225 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 3 Nov 2026 07:43:57 +0100 Subject: [PATCH] Prevent merging a branch into itself --- packages/app/src/git/policy.test.ts | 26 +++++++++++++++++++ packages/app/src/git/policy.ts | 17 ++++++++++-- packages/app/src/git/use-actions.tsx | 6 +++++ .../server/src/utils/checkout-git.test.ts | 7 +++++ packages/server/src/utils/checkout-git.ts | 2 +- 5 files changed, 55 insertions(+), 3 deletions(-) diff --git a/packages/app/src/git/policy.test.ts b/packages/app/src/git/policy.test.ts index e5ea1ee509..9a3dffb9d5 100644 --- a/packages/app/src/git/policy.test.ts +++ b/packages/app/src/git/policy.test.ts @@ -6,6 +6,7 @@ import { actionsWithoutPrimary, buildGitActions, isCurrentBranchBase, + isCurrentBranchLocalMergeTarget, type BuildGitActionsInput, } from "./policy"; import { deriveMergeCapability, type ForgeSpecificStatusFacts } from "./merge-capability"; @@ -77,6 +78,7 @@ function createInput( hasRemote: false, isPaseoOwnedWorktree: false, isOnBaseBranch: true, + isOnLocalMergeTarget: false, hasUncommittedChanges: false, baseRefAvailable: true, baseRefLabel: "main", @@ -219,6 +221,30 @@ describe("git-actions-policy", () => { expect(isCurrentBranchBase("release", "release")).toBe(true); expect(isCurrentBranchBase("release", "origin/release")).toBe(true); expect(isCurrentBranchBase(null, "refs/heads/release")).toBe(false); + expect(isCurrentBranchLocalMergeTarget("release", "refs/heads/release")).toBe(true); + expect(isCurrentBranchLocalMergeTarget("feature", "refs/remotes/origin/release")).toBe(false); + expect(isCurrentBranchLocalMergeTarget("release", "refs/remotes/upstream/release")).toBe(false); + }); + + it("does not offer merging into the current local branch when the remote base shares its name", () => { + const actions = buildGitActions( + createInput({ + isPaseoOwnedWorktree: true, + isOnBaseBranch: false, + isOnLocalMergeTarget: isCurrentBranchLocalMergeTarget( + "release", + "refs/remotes/origin/release", + ), + baseRefLabel: "release", + aheadCount: 1, + behindBaseCount: 1, + aheadOfOrigin: null, + behindOfOrigin: null, + shipDefault: "merge", + }), + ); + expect(actions.primary?.id).toBe("merge-from-base"); + expect(actionsWithoutPrimary(actions).map((action) => action.id)).not.toContain("merge-branch"); }); it("offers update from the same-named remote base when it is ahead", () => { diff --git a/packages/app/src/git/policy.ts b/packages/app/src/git/policy.ts index 7dcd55d64e..7d823031ad 100644 --- a/packages/app/src/git/policy.ts +++ b/packages/app/src/git/policy.ts @@ -61,6 +61,18 @@ export function isCurrentBranchBase( return currentBranch === baseRef.replace(/^origin\//, ""); } +export function isCurrentBranchLocalMergeTarget( + currentBranch: string | null | undefined, + baseRef: string | null | undefined, +): boolean { + if (!currentBranch || !baseRef) return false; + const localTarget = baseRef + .replace(/^refs\/heads\//, "") + .replace(/^refs\/remotes\/origin\//, "") + .replace(/^origin\//, ""); + return currentBranch === localTarget; +} + export interface BuildGitActionsInput { isGit: boolean; githubFeaturesEnabled: boolean; @@ -79,6 +91,7 @@ export interface BuildGitActionsInput { hasRemote: boolean; isPaseoOwnedWorktree: boolean; isOnBaseBranch: boolean; + isOnLocalMergeTarget: boolean; hasUncommittedChanges: boolean; baseRefAvailable: boolean; baseRefLabel: string; @@ -360,7 +373,7 @@ function getPrimaryActionId(input: BuildGitActionsInput): GitActionId | null { if (input.shipDefault === "pr" && canUsePullRequestActionAsShipDefault(input)) { return "pr"; } - if (!input.isOnBaseBranch && input.aheadCount > 0) { + if (!input.isOnBaseBranch && !input.isOnLocalMergeTarget && input.aheadCount > 0) { return "merge-branch"; } if (!input.isOnBaseBranch && canMergeFromBase(input)) { @@ -391,7 +404,7 @@ function getPullRequestActionIds(filter: { function getFeatureActionIds(input: BuildGitActionsInput): GitActionId[] { return [ "merge-from-base", - "merge-branch", + ...(!input.isOnLocalMergeTarget ? (["merge-branch"] as const) : []), ...getPullRequestActionIds({ roles: ["status", "direct", "auto"], input }), ]; } diff --git a/packages/app/src/git/use-actions.tsx b/packages/app/src/git/use-actions.tsx index 250581598a..ea9de74511 100644 --- a/packages/app/src/git/use-actions.tsx +++ b/packages/app/src/git/use-actions.tsx @@ -13,6 +13,7 @@ import { type CheckoutPrStatusPayload, useCheckoutPrStatusQuery } from "@/git/us import { buildGitActions, isCurrentBranchBase, + isCurrentBranchLocalMergeTarget, narrowPullRequestState, type BuildGitActionsInput, type GitAction, @@ -130,6 +131,7 @@ interface DerivedGitActionsState { hasRemote: boolean; isPaseoOwnedWorktree: boolean; isOnBaseBranch: boolean; + isOnLocalMergeTarget: boolean; shouldPromoteArchive: boolean; } @@ -180,6 +182,7 @@ function deriveGitActionsState(args: DeriveGitActionsStateArgs): DerivedGitActio hasRemote: gitStatus?.hasRemote ?? false, isPaseoOwnedWorktree, isOnBaseBranch: isCurrentBranchBase(gitStatus?.currentBranch, baseRef), + isOnLocalMergeTarget: isCurrentBranchLocalMergeTarget(gitStatus?.currentBranch, baseRef), shouldPromoteArchive: computeShouldPromoteArchive({ hasUncommittedChanges, postShipArchiveSuggested, @@ -666,6 +669,7 @@ export function useGitActions({ serverId, cwd, icons }: UseGitActionsInput): Use hasRemote, isPaseoOwnedWorktree, isOnBaseBranch, + isOnLocalMergeTarget, shouldPromoteArchive, } = derived; @@ -696,6 +700,7 @@ export function useGitActions({ serverId, cwd, icons }: UseGitActionsInput): Use hasRemote, isPaseoOwnedWorktree, isOnBaseBranch, + isOnLocalMergeTarget, hasUncommittedChanges, baseRefAvailable: Boolean(baseRef), baseRefLabel, @@ -813,6 +818,7 @@ export function useGitActions({ serverId, cwd, icons }: UseGitActionsInput): Use behindBaseCount, isPaseoOwnedWorktree, isOnBaseBranch, + isOnLocalMergeTarget, githubFeaturesEnabled, forge, githubAutoMergeActionsEnabled, diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index 12abec88ff..075f7ebda1 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -1971,6 +1971,13 @@ const x = 1; expect(status.currentBranch).toBe("feature/plain"); }); + it("rejects merging a branch into its own same-named remote base", async () => { + execFileSync("git", ["update-ref", "refs/remotes/origin/main", "HEAD"], { cwd: repoDir }); + await expect(mergeToBase(repoDir, { baseRef: "refs/remotes/origin/main" })).rejects.toThrow( + "Cannot merge a branch into itself", + ); + }); + it("merges the current branch into base from a worktree checkout", async () => { const worktree = await createLegacyWorktreeForTest({ branchName: "main", diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 40507c54c5..d793e2c683 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -3942,7 +3942,7 @@ export async function mergeToBase( const normalizedBaseRef = resolveMergeTargetBranch(baseRef); const currentWorktreeRoot = (await getWorktreeRoot(cwd, context)) ?? cwd; if (normalizedBaseRef === currentBranch) { - return currentWorktreeRoot; + throw new Error("Cannot merge a branch into itself"); } const baseWorktree = await getWorktreePathForBranch(cwd, normalizedBaseRef);