From 27839622efccb65982fd297847973fdedf6365aa Mon Sep 17 00:00:00 2001 From: maria Date: Sun, 11 Oct 2026 23:59:44 +0000 Subject: [PATCH] fix(shared): preserve generated branch namespaces (#15615) --- apps/server/src/git/GitManager.test.ts | 41 ++++++++++++++ .../CodexTextGeneration.test.ts | 2 +- packages/shared/src/git.test.ts | 55 +++++++++++++++++++ packages/shared/src/git.ts | 40 ++++++++------ 4 files changed, 120 insertions(+), 18 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 73d23c415c17059c0f5aba436f2e415103674f23..67352160751b7f9b9f02f959247b8d163772b462 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -3834,6 +3834,47 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); + it.effect.each([ + { branch: "fix/name", existingBranches: ["fix"], expected: "fix-2/name" }, + { branch: "fix/name", existingBranches: ["fix/name/child"], expected: "fix/name-2" }, + ])("commits on $expected when $branch has ref namespace collisions", (scenario) => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const mainSha = (yield* runGit(repoDir, ["rev-parse", "main"])).stdout.trim(); + for (const branch of scenario.existingBranches) { + yield* runGit(repoDir, ["branch", branch]); + } + NodeFS.writeFileSync(NodePath.join(repoDir, "README.md"), "hello\nnamespace-collision\n"); + + const { manager } = yield* makeManager({ + textGeneration: { + generateCommitMessage: () => + Effect.succeed({ + subject: "Fix namespace collision", + body: "", + branch: scenario.branch, + }), + }, + }); + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "commit", + featureBranch: true, + }); + + expect(result.branch).toEqual({ status: "created", name: scenario.expected }); + expect(result.commit.status).toBe("created"); + expect((yield* runGit(repoDir, ["branch", "--show-current"])).stdout.trim()).toBe( + scenario.expected, + ); + expect((yield* runGit(repoDir, ["status", "--porcelain"])).stdout.trim()).toBe(""); + for (const branch of ["main", ...scenario.existingBranches]) { + expect((yield* runGit(repoDir, ["rev-parse", branch])).stdout.trim()).toBe(mainSha); + } + }), + ); + it.effect("featureBranch uses custom commit message and derives branch name", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); diff --git a/apps/server/src/textGeneration/CodexTextGeneration.test.ts b/apps/server/src/textGeneration/CodexTextGeneration.test.ts index e991ddf17fdfcd1d56711b2f04adf10f750b5685..0c245537108c23b2ee421b0383a57372c31e4600 100644 --- a/apps/server/src/textGeneration/CodexTextGeneration.test.ts +++ b/apps/server/src/textGeneration/CodexTextGeneration.test.ts @@ -353,7 +353,7 @@ it.layer(layerCodexTextGenerationTest)("CodexTextGeneration", (it) => { }); expect(generated.subject).toBe("Add important change"); - expect(generated.branch).toBe("feature/fix/important-system-change"); + expect(generated.branch).toBe("fix/important-system-change"); }), ), ); diff --git a/packages/shared/src/git.test.ts b/packages/shared/src/git.test.ts index 2c55f8a84c1cce1bb31dcbeb2072425ff4f47d22..7ca2678d4c42d5806baad745e21cb1f14fabf0bd 100644 --- a/packages/shared/src/git.test.ts +++ b/packages/shared/src/git.test.ts @@ -11,6 +11,8 @@ import { normalizeGitRemoteUrl, parseGitHubRepositoryNameWithOwnerFromRemoteUrl, parseOriginUrlFromGitConfig, + resolveAutoFeatureBranchName, + sanitizeFeatureBranchName, WORKTREE_BRANCH_PREFIX, } from "./git.ts"; @@ -294,6 +296,59 @@ describe("applyGitStatusStreamEvent", () => { }); }); +describe("sanitizeFeatureBranchName", () => { + it.each(["feature", "fix", "team/jules"])("preserves the %s namespace", (namespace) => { + expect(sanitizeFeatureBranchName(`${namespace}/refine-toolbar`)).toBe( + `${namespace}/refine-toolbar`, + ); + }); + + it("sanitizes an explicit namespace before preserving it", () => { + expect(sanitizeFeatureBranchName(' "FIX//Calendar recruitment filter" ')).toBe( + "fix/calendar-recruitment-filter", + ); + }); + + it.each([ + ["refine toolbar", "feature/refine-toolbar"], + ["", "feature/update"], + [" /?. / ", "feature/update"], + ["fix///", "feature/fix"], + ])("keeps the fallback for unprefixed input %s", (input, expected) => { + expect(sanitizeFeatureBranchName(input)).toBe(expected); + }); + + it("keeps the existing length limit for namespaced branches", () => { + expect(sanitizeFeatureBranchName(`fix/${"x".repeat(80)}`)).toBe(`fix/${"x".repeat(60)}`); + }); +}); + +describe("resolveAutoFeatureBranchName", () => { + it("resolves case-insensitive collisions within the generated namespace", () => { + expect( + resolveAutoFeatureBranchName( + ["FIX/refine-toolbar", "fix/refine-toolbar-2"], + "fix/refine-toolbar", + ), + ).toBe("fix/refine-toolbar-3"); + }); + + it("keeps the fallback when no preferred branch is supplied", () => { + expect(resolveAutoFeatureBranchName(["feature/update"])).toBe("feature/update-2"); + }); + + it.each([ + ["fix/name", ["fix"], "fix-2/name"], + ["team/jules/fix/name", ["team/jules"], "team/jules-2/fix/name"], + ["fix/name", ["fix/name/child"], "fix/name-2"], + ["FIX/name", ["FIX", "FIX-2"], "fix-3/name"], + ["fix/name", ["fixes", "fix/name-extra/child"], "fix/name"], + [undefined, ["feature", "feature-2"], "feature-3/update"], + ])("resolves %s against namespace blockers %j", (preferredBranch, existingNames, expected) => { + expect(resolveAutoFeatureBranchName(existingNames, preferredBranch)).toBe(expected); + }); +}); + describe("formatGeneratedBranchName", () => { it.each(["t3code", "t3code/"])("joins static prefix %s with one slash", (prefix) => { expect( diff --git a/packages/shared/src/git.ts b/packages/shared/src/git.ts index 6efef3f70ac3ccee3f56e263f3e91855dc48b798..3d9af4b7ed8f88a7d5ea4dc1f44917b16764a5db 100644 --- a/packages/shared/src/git.ts +++ b/packages/shared/src/git.ts @@ -66,22 +66,19 @@ export function formatGeneratedBranchName(raw: string, naming?: BranchNamingOpti } /** - * Sanitize a string into a `feature/…` refName name. - * Preserves an existing `feature/` prefix or slash-separated namespace. + * Sanitize a generated branch name, preserving any slash-separated namespace. + * Unprefixed fragments use the `feature/` namespace. */ export function sanitizeFeatureBranchName(raw: string): string { const sanitized = sanitizeBranchFragment(raw); - if (sanitized.includes("/")) { - return sanitized.startsWith("feature/") ? sanitized : `feature/${sanitized}`; - } - return `feature/${sanitized}`; + return sanitized.includes("/") ? sanitized : `feature/${sanitized}`; } const AUTO_FEATURE_BRANCH_FALLBACK = "feature/update"; /** - * Resolve a unique `feature/…` refName name that doesn't collide with - * any existing refName. Appends a numeric suffix when needed. + * Resolve a unique generated refName, suffixing only the path component + * blocked by an existing ref or, for the final component, its descendants. */ export function resolveAutoFeatureBranchName( existingBranchNames: readonly string[], @@ -91,18 +88,27 @@ export function resolveAutoFeatureBranchName( const resolvedBase = sanitizeFeatureBranchName( preferred && preferred.length > 0 ? preferred : AUTO_FEATURE_BRANCH_FALLBACK, ); - const existingNames = new Set(existingBranchNames.map((refName) => refName.toLowerCase())); + const existingNames = existingBranchNames.map((refName) => refName.toLowerCase()); + const existingRefs = new Set(existingNames); + const parts = resolvedBase.split("/"); + let resolved = ""; - if (!existingNames.has(resolvedBase)) { - return resolvedBase; + for (const [index, part] of parts.entries()) { + const base = resolved ? `${resolved}/${part}` : part; + let candidate = base; + let suffix = 2; + while ( + existingRefs.has(candidate) || + (index === parts.length - 1 && + existingNames.some((refName) => refName.startsWith(`${candidate}/`))) + ) { + candidate = `${base}-${suffix}`; + suffix += 1; + } + resolved = candidate; } - let suffix = 2; - while (existingNames.has(`${resolvedBase}-${suffix}`)) { - suffix += 1; - } - - return `${resolvedBase}-${suffix}`; + return resolved; } /** -- 2.54.0