From 7912894ea602d1b928dbedfa07dc6730773fa063 Mon Sep 17 00:00:00 2001 From: Satya Patel Date: Tue, 27 Jan 2026 15:10:10 -0800 Subject: [PATCH] fix(desktop): validate branch name for git ref path conflicts before workspace creation Adds early validation to detect when a new branch name would conflict with existing branches due to Git's ref storage structure (e.g., can't create "release/v61" if "release" exists). This provides a clear error message with a suggested alternative before any database writes or background jobs start, ensuring MCP and UI both get immediate feedback. --- .../routers/workspaces/procedures/create.ts | 18 +++++++ apps/desktop/src/shared/utils/branch.test.ts | 52 +++++++++++++++++++ apps/desktop/src/shared/utils/branch.ts | 38 ++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts index 441591e8a07..f5952ca9fb5 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts @@ -6,6 +6,7 @@ import { track } from "main/lib/analytics"; import { localDb } from "main/lib/local-db"; import { workspaceInitManager } from "main/lib/workspace-init-manager"; import { SUPERSET_DIR_NAME, WORKTREES_DIR_NAME } from "shared/constants"; +import { findBranchPathConflict } from "shared/utils/branch"; import { z } from "zod"; import { publicProcedure, router } from "../../.."; import { @@ -328,6 +329,23 @@ export const createCreateProcedures = () => { branch = generateBranchName({ existingBranches, authorPrefix }); } + // Check for Git ref path conflicts when creating a new branch + // e.g., can't create "release/v61" if "release" exists (or vice versa) + if (!existingBranchName) { + const conflictingBranch = findBranchPathConflict( + branch, + existingBranches, + ); + if (conflictingBranch) { + const suggestedName = branch.replace(/\//g, "-"); + throw new Error( + `Cannot create branch "${branch}" because branch "${conflictingBranch}" already exists. ` + + `Git cannot have branches where one is a path prefix of another. ` + + `Try using "${suggestedName}" instead.`, + ); + } + } + const worktreePath = join( homedir(), SUPERSET_DIR_NAME, diff --git a/apps/desktop/src/shared/utils/branch.test.ts b/apps/desktop/src/shared/utils/branch.test.ts index 568dab99f29..4e3a4a820bf 100644 --- a/apps/desktop/src/shared/utils/branch.test.ts +++ b/apps/desktop/src/shared/utils/branch.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { + findBranchPathConflict, sanitizeAuthorPrefix, sanitizeBranchName, sanitizeSegment, @@ -86,3 +87,54 @@ describe("sanitizeBranchName", () => { expect(sanitizeBranchName("///")).toBe(""); }); }); + +describe("findBranchPathConflict", () => { + test("detects conflict when new branch is child of existing", () => { + // Creating release/v61 when "release" exists + expect(findBranchPathConflict("release/v61", ["release", "main"])).toBe( + "release", + ); + }); + + test("detects conflict when new branch is parent of existing", () => { + // Creating "release" when "release/v61" exists + expect(findBranchPathConflict("release", ["release/v61", "main"])).toBe( + "release/v61", + ); + }); + + test("detects deep nested conflicts", () => { + // Creating feature/auth/oauth when feature/auth exists + expect( + findBranchPathConflict("feature/auth/oauth", ["feature/auth", "main"]), + ).toBe("feature/auth"); + }); + + test("returns null when no conflict exists", () => { + expect(findBranchPathConflict("feature/new", ["release", "main"])).toBe( + null, + ); + }); + + test("returns null for sibling branches", () => { + // release-v61 is not a child of release + expect(findBranchPathConflict("release-v61", ["release", "main"])).toBe( + null, + ); + }); + + test("handles case insensitive comparison", () => { + expect(findBranchPathConflict("Release/V61", ["release", "main"])).toBe( + "release", + ); + }); + + test("returns null for empty existing branches", () => { + expect(findBranchPathConflict("release/v61", [])).toBe(null); + }); + + test("does not match exact same branch name", () => { + // Exact match is not a path conflict (it's a duplicate, handled elsewhere) + expect(findBranchPathConflict("release", ["release"])).toBe(null); + }); +}); diff --git a/apps/desktop/src/shared/utils/branch.ts b/apps/desktop/src/shared/utils/branch.ts index cace94a9c1a..1ebd9008393 100644 --- a/apps/desktop/src/shared/utils/branch.ts +++ b/apps/desktop/src/shared/utils/branch.ts @@ -20,3 +20,41 @@ export function sanitizeBranchName(name: string): string { .filter(Boolean) .join("/"); } + +/** + * Checks if a new branch name would conflict with existing branches due to + * Git's ref storage using file/directory structure. + * + * Git stores branches as files in .git/refs/heads/. This means: + * - If "release" exists as a branch (file at refs/heads/release), you cannot + * create "release/v1" (which would need refs/heads/release/ as a directory) + * - If "release/v1" exists, you cannot create "release" (same conflict) + * + * @param newBranch - The branch name to create + * @param existingBranches - List of existing branch names + * @returns The conflicting branch name if found, null otherwise + */ +export function findBranchPathConflict( + newBranch: string, + existingBranches: string[], +): string | null { + const newBranchLower = newBranch.toLowerCase(); + + for (const existing of existingBranches) { + const existingLower = existing.toLowerCase(); + + // Check if the new branch would be a "child" of an existing branch + // e.g., creating "release/v61" when "release" exists + if (newBranchLower.startsWith(`${existingLower}/`)) { + return existing; + } + + // Check if the new branch would be a "parent" of an existing branch + // e.g., creating "release" when "release/v61" exists + if (existingLower.startsWith(`${newBranchLower}/`)) { + return existing; + } + } + + return null; +}