From 2348d69fcccd98e36a5ccbad700675a9629e37bf Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Sun, 4 Oct 2026 11:56:42 +0000 Subject: [PATCH 1/4] fix(shared): preserve generated branch namespaces --- .../CodexTextGeneration.test.ts | 2 +- packages/shared/src/git.test.ts | 47 +++++++++++++++++++ packages/shared/src/git.ts | 11 ++--- 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/apps/server/src/textGeneration/CodexTextGeneration.test.ts b/apps/server/src/textGeneration/CodexTextGeneration.test.ts index 98f0c9836e47..8eed90663e70 100644 --- a/apps/server/src/textGeneration/CodexTextGeneration.test.ts +++ b/apps/server/src/textGeneration/CodexTextGeneration.test.ts @@ -353,7 +353,7 @@ it.layer(CodexTextGenerationTestLayer)("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 36e1b2a0291f..a69a771c2b09 100644 --- a/packages/shared/src/git.test.ts +++ b/packages/shared/src/git.test.ts @@ -9,6 +9,8 @@ import { normalizeGitRemoteUrl, parseGitHubRepositoryNameWithOwnerFromRemoteUrl, parseOriginUrlFromGitConfig, + resolveAutoFeatureBranchName, + sanitizeFeatureBranchName, WORKTREE_BRANCH_PREFIX, } from "./git.ts"; @@ -276,6 +278,51 @@ describe("applyGitStatusStreamEvent", () => { }); }); +describe("sanitizeFeatureBranchName", () => { + it.each(["feature", "fix", "feat", "chore", "hotfix", "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"); + }); +}); + 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 d5866d32e5d1..6ff637cb2a44 100644 --- a/packages/shared/src/git.ts +++ b/packages/shared/src/git.ts @@ -61,21 +61,18 @@ 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 + * Resolve a unique generated refName that doesn't collide with * any existing refName. Appends a numeric suffix when needed. */ export function resolveAutoFeatureBranchName( From 9a58394e15227b8bfd0d6b2893196acc28f44b51 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Sun, 4 Oct 2026 12:48:59 +0000 Subject: [PATCH 2/4] fix(shared): avoid generated branch ref collisions --- apps/server/src/git/GitManager.test.ts | 59 ++++++++++++++++++++++++++ packages/shared/src/git.test.ts | 24 +++++++++++ packages/shared/src/git.ts | 33 ++++++++------ 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 1b6b44008467..bec9dced3771 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -3247,6 +3247,65 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); + it.effect.each([ + { + branch: "fix/name", + existingBranches: ["fix"], + expected: "fix-2/name", + }, + { + branch: "team/jules/fix/name", + existingBranches: [ + "team/jules", + "team/jules-2/fix", + "team/jules-2/fix-2/name", + "team/jules-2/fix-2/name-2/child", + ], + expected: "team/jules-2/fix-2/name-3", + }, + { + branch: "fix/name", + existingBranches: ["fix/name/child", "fix/name-2/child"], + expected: "fix/name-3", + }, + ])("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/packages/shared/src/git.test.ts b/packages/shared/src/git.test.ts index a69a771c2b09..7ef5090cc63c 100644 --- a/packages/shared/src/git.test.ts +++ b/packages/shared/src/git.test.ts @@ -321,6 +321,30 @@ describe("resolveAutoFeatureBranchName", () => { 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"], + ["fix/name", ["fix", "fix-2"], "fix-3/name"], + ["team/jules/fix/name", ["team/jules"], "team/jules-2/fix/name"], + ["fix/name", ["fix/name/child"], "fix/name-2"], + ["fix/name", ["fix/name", "fix/name-2/child", "fix/name-3"], "fix/name-4"], + ["fix/name", ["fix", "fix-2", "fix-3/name", "fix-3/name-2/child"], "fix-3/name-3"], + [ + "team/jules/fix/name", + ["team/jules", "team/jules-2/fix", "team/jules-2/fix-2/name/child"], + "team/jules-2/fix-2/name-2", + ], + ["FIX/name", ["FIX", "FIX-2"], "fix-3/name"], + ["fix/name", ["FIX/NAME/child", "fix/name-2"], "fix/name-3"], + ["fix/name", ["fix", "fix-2/other"], "fix-2/name"], + ["fix/name", ["fixes", "fix/name-extra/child"], "fix/name"], + ["fix/name-2", ["fix/name-2"], "fix/name-2-2"], + ["name", ["feature"], "feature-2/name"], + [undefined, ["feature", "feature-2"], "feature-3/update"], + [undefined, ["feature/update/child", "feature/update-2/child"], "feature/update-3"], + ])("resolves %s against namespace blockers %j", (preferredBranch, existingNames, expected) => { + expect(resolveAutoFeatureBranchName(existingNames, preferredBranch)).toBe(expected); + }); }); describe("formatGeneratedBranchName", () => { diff --git a/packages/shared/src/git.ts b/packages/shared/src/git.ts index 6ff637cb2a44..085ca4ee673d 100644 --- a/packages/shared/src/git.ts +++ b/packages/shared/src/git.ts @@ -72,8 +72,8 @@ export function sanitizeFeatureBranchName(raw: string): string { const AUTO_FEATURE_BRANCH_FALLBACK = "feature/update"; /** - * Resolve a unique generated refName 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[], @@ -83,18 +83,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())); - - if (!existingNames.has(resolvedBase)) { - return resolvedBase; - } - - let suffix = 2; - while (existingNames.has(`${resolvedBase}-${suffix}`)) { - suffix += 1; + const existingNames = existingBranchNames.map((refName) => refName.toLowerCase()); + const existingRefs = new Set(existingNames); + const parts = resolvedBase.split("/"); + let resolved = ""; + + 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; } - return `${resolvedBase}-${suffix}`; + return resolved; } /** From f3e4fbf9257a685155df6873d63cb3aca1ff5112 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Sun, 4 Oct 2026 13:33:44 +0000 Subject: [PATCH 3/4] fix(git): preserve local namespaces when publishing --- apps/server/src/git/GitManager.test.ts | 54 +++++++++++++++++++- apps/server/src/vcs/GitVcsDriverCore.test.ts | 10 ++-- apps/server/src/vcs/GitVcsDriverCore.ts | 5 +- 3 files changed, 62 insertions(+), 7 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index bec9dced3771..a286562acab1 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -3247,12 +3247,56 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); + it.effect.each(["origin/main", "origin/team/fix"])("publishes local namespace $0", (branch) => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + const mainSha = (yield* runGit(remoteDir, ["rev-parse", "refs/heads/main"])).stdout.trim(); + NodeFS.writeFileSync(NodePath.join(repoDir, "README.md"), "hello\npublication\n"); + + const { manager } = yield* makeManager({ + textGeneration: { + generateCommitMessage: () => + Effect.succeed({ subject: "Fix branch publication", body: "", branch }), + }, + }); + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "commit_push", + featureBranch: true, + }); + + expect(result.branch).toEqual({ status: "created", name: branch }); + expect(result.commit.status).toBe("created"); + expect(result.push.status).toBe("pushed"); + expect((yield* runGit(repoDir, ["branch", "--show-current"])).stdout.trim()).toBe(branch); + expect((yield* runGit(remoteDir, ["rev-parse", "refs/heads/main"])).stdout.trim()).toBe( + mainSha, + ); + const headSha = (yield* runGit(repoDir, ["rev-parse", "HEAD"])).stdout.trim(); + expect((yield* runGit(remoteDir, ["rev-parse", `refs/heads/${branch}`])).stdout.trim()).toBe( + headSha, + ); + expect((yield* runGit(repoDir, ["config", `branch.${branch}.merge`])).stdout.trim()).toBe( + `refs/heads/${branch}`, + ); + }), + ); + it.effect.each([ { branch: "fix/name", existingBranches: ["fix"], expected: "fix-2/name", }, + { + branch: "heads/fix/name", + existingBranches: ["heads/fix"], + expected: "heads/fix-2/name", + }, { branch: "team/jules/fix/name", existingBranches: [ @@ -3275,6 +3319,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const mainSha = (yield* runGit(repoDir, ["rev-parse", "main"])).stdout.trim(); for (const branch of scenario.existingBranches) { yield* runGit(repoDir, ["branch", branch]); + yield* runGit(repoDir, ["tag", branch]); } NodeFS.writeFileSync(NodePath.join(repoDir, "README.md"), "hello\nnamespace-collision\n"); @@ -3301,7 +3346,14 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { ); 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); + expect((yield* runGit(repoDir, ["rev-parse", `refs/heads/${branch}`])).stdout.trim()).toBe( + mainSha, + ); + } + for (const tag of scenario.existingBranches) { + expect((yield* runGit(repoDir, ["rev-parse", `refs/tags/${tag}`])).stdout.trim()).toBe( + mainSha, + ); } }), ); diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index c720a2e4958a..9a0c80e8f746 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -3474,14 +3474,14 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { }), ); - it.effect("pushes to the requested remote instead of the primary remote", () => + it.effect.each(["main", "origin/main"])("publishes $0 to the requested remote", (branch) => Effect.gen(function* () { const cwd = yield* makeTmpDir(); const originRemote = yield* makeTmpDir("git-origin-remote-"); const publishRemote = yield* makeTmpDir("git-publish-remote-"); yield* initRepoWithCommit(cwd); const driver = yield* GitVcsDriver.GitVcsDriver; - yield* git(cwd, ["branch", "-M", "main"]); + yield* git(cwd, ["branch", "-M", branch]); yield* git(originRemote, ["init", "--bare"]); yield* git(publishRemote, ["init", "--bare"]); yield* git(cwd, ["remote", "add", "origin", originRemote]); @@ -3491,12 +3491,12 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { assert.deepInclude(pushed, { status: "pushed", - branch: "main", - upstreamBranch: "origin-1/main", + branch, + upstreamBranch: `origin-1/${branch}`, setUpstream: true, }); assert.equal( - yield* git(publishRemote, ["log", "-1", "--pretty=%s", "main"]), + yield* git(publishRemote, ["log", "-1", "--pretty=%s", `refs/heads/${branch}`]), "initial commit", ); const originMain = yield* driver.execute({ diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 971d0e7a9a95..9cbc41e46211 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -1444,6 +1444,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* cwd: string, branchName: string, ) { + if (yield* branchExists(cwd, branchName)) { + return branchName; + } const remoteNames = yield* listRemoteNames(cwd).pipe(Effect.orElseSucceed(() => [])); const parsedRemoteRef = parseRemoteRefWithRemoteNames(branchName, remoteNames); return parsedRemoteRef?.branchName ?? branchName; @@ -3764,7 +3767,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* "branch", "--list", "--no-column", - "--format=%(refname:short)", + "--format=%(refname:lstrip=2)", ]).pipe( Effect.map((stdout) => { const branchNames: Array = []; From 78565d36e8e6fdf8997389bc4fa4d2a2c8e918c1 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Sun, 4 Oct 2026 14:02:52 +0000 Subject: [PATCH 4/4] fix(git): keep branch status and comparisons unambiguous --- apps/server/src/git/GitManager.test.ts | 103 ++++++++++++++++++- apps/server/src/vcs/GitVcsDriverCore.test.ts | 82 +++++++++------ apps/server/src/vcs/GitVcsDriverCore.ts | 51 ++++----- 3 files changed, 171 insertions(+), 65 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index a286562acab1..d7d69acfd410 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -3247,7 +3247,14 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); - it.effect.each(["origin/main", "origin/team/fix"])("publishes local namespace $0", (branch) => + it.effect.each([ + ["origin/main", "origin/main", "origin"], + ["origin/team/fix", "origin/team/fix", "origin"], + ["origin/main", "origin/main", "fork"], + ["origin/main", "origin/main", "team/fork"], + ["heads/origin/main", "heads/origin/main", "team/fork"], + ["plain", "feature/plain", "team/fork"], + ] as const)("publishes local namespace $0 to $2", ([suggestedBranch, branch, remoteName]) => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); yield* initRepo(repoDir); @@ -3255,34 +3262,120 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); yield* runGit(repoDir, ["push", "-u", "origin", "main"]); const mainSha = (yield* runGit(remoteDir, ["rev-parse", "refs/heads/main"])).stdout.trim(); + const publishDir = remoteName === "origin" ? remoteDir : yield* createBareRemote(); + if (remoteName !== "origin") { + yield* configureRemote(repoDir, remoteName, publishDir, remoteName); + yield* runGit(repoDir, ["config", "remote.pushDefault", remoteName]); + } + if (branch.startsWith("heads/")) { + yield* runGit(repoDir, ["tag", branch]); + } NodeFS.writeFileSync(NodePath.join(repoDir, "README.md"), "hello\npublication\n"); const { manager } = yield* makeManager({ + ghScenario: { + prListByHeadSelector: { + [branch]: encodeCliJson([ + { + number: 7073, + title: "Preserve branch namespace", + url: "https://github.com/pingdotgg/codething-mvp/pull/7073", + baseRefName: "main", + headRefName: branch, + state: "OPEN", + isCrossRepository: remoteName !== "origin", + headRepository: { + nameWithOwner: `${remoteName === "origin" ? "pingdotgg" : "contributor"}/codething-mvp`, + }, + headRepositoryOwner: { + login: remoteName === "origin" ? "pingdotgg" : "contributor", + }, + }, + ]), + }, + }, textGeneration: { generateCommitMessage: () => - Effect.succeed({ subject: "Fix branch publication", body: "", branch }), + Effect.succeed({ + subject: "Fix branch publication", + body: "", + branch: suggestedBranch, + }), }, }); const result = yield* runStackedAction(manager, { cwd: repoDir, - action: "commit_push", + action: "commit", featureBranch: true, }); expect(result.branch).toEqual({ status: "created", name: branch }); expect(result.commit.status).toBe("created"); - expect(result.push.status).toBe("pushed"); + const unpublished = yield* manager.status({ cwd: repoDir }); + const driver = yield* GitVcsDriver.GitVcsDriver; + const preview = yield* driver.getReviewDiffPreview({ cwd: repoDir }); + const changes = preview.sources.find((source) => source.kind === "branch-range"); + expect( + yield* driver.getReviewDiffFileContents({ + cwd: repoDir, + sourceKind: "branch-range", + changeType: "change", + baseRef: changes?.baseRef ?? null, + headRef: branch, + oldPath: "README.md", + newPath: "README.md", + }), + ).toMatchObject({ oldContents: "hello\n", newContents: "hello\npublication\n" }); + expect(unpublished.aheadCount).toBe(1); + expect(unpublished.aheadOfDefaultCount).toBe(1); + expect(unpublished.branchChanges).toMatchObject({ insertions: 1, deletions: 0 }); + expect(changes).toMatchObject({ + baseRef: "refs/remotes/origin/main", + headRef: branch, + title: "Changes vs origin/main", + files: [{ path: "README.md", previousPath: null, additions: 1, deletions: 0 }], + }); + expect( + (yield* driver.getReviewDiffPreview({ + cwd: repoDir, + baseRef: changes?.baseRef ?? undefined, + file: { path: "README.md", previousPath: null, sourceKind: "branch-range" }, + })).sources.find((source) => source.kind === "branch-range")?.files, + ).toEqual(changes?.files); + expect((yield* runStackedAction(manager, { cwd: repoDir, action: "push" })).push.status).toBe( + "pushed", + ); expect((yield* runGit(repoDir, ["branch", "--show-current"])).stdout.trim()).toBe(branch); expect((yield* runGit(remoteDir, ["rev-parse", "refs/heads/main"])).stdout.trim()).toBe( mainSha, ); const headSha = (yield* runGit(repoDir, ["rev-parse", "HEAD"])).stdout.trim(); - expect((yield* runGit(remoteDir, ["rev-parse", `refs/heads/${branch}`])).stdout.trim()).toBe( + expect((yield* runGit(publishDir, ["rev-parse", `refs/heads/${branch}`])).stdout.trim()).toBe( headSha, ); expect((yield* runGit(repoDir, ["config", `branch.${branch}.merge`])).stdout.trim()).toBe( `refs/heads/${branch}`, ); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + remoteName, + `git@github.com:${remoteName === "origin" ? "pingdotgg" : "contributor"}/codething-mvp.git`, + publishDir, + ); + const status = yield* manager.status({ cwd: repoDir }); + expect(status.refName).toBe(branch); + expect(status.aheadCount).toBe(0); + expect(status.aheadOfDefaultCount).toBe(1); + expect(status.branchChanges).toMatchObject({ insertions: 1, deletions: 0 }); + expect( + (yield* driver.getReviewDiffPreview({ cwd: repoDir })).sources.find( + (source) => source.kind === "branch-range", + )?.files, + ).toEqual(changes?.files); + expect(status.pr).toMatchObject({ number: 7073, headRef: branch }); + expect( + (yield* manager.remoteStatus({ cwd: repoDir }, { refreshUpstream: false }))?.pr, + ).toEqual(status.pr); }), ); diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 9a0c80e8f746..dbc13fe22adf 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -332,7 +332,7 @@ it.effect("uses stable diagnostics for every parsed non-repository command", () assert.deepStrictEqual(commands, [ { args: ["rev-parse", "--git-path", "index"], lcAll: "C" }, { args: ["status", "--porcelain=2", "--branch"], lcAll: "C" }, - { args: ["rev-parse", "--abbrev-ref", "HEAD"], lcAll: "C" }, + { args: ["branch", "--show-current"], lcAll: "C" }, { args: ["rev-parse", "--git-common-dir"], lcAll: "C" }, ]); }).pipe(Effect.provide(layer)); @@ -1824,7 +1824,8 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { yield* git(cwd, ["fetch", "origin"]); const preview = yield* driver.getReviewDiffPreview({ cwd }); const changes = preview.sources.find((source) => source.kind === "branch-range")!; - assert.strictEqual(changes.baseRef, "origin/main"); + assert.strictEqual(changes.baseRef, "refs/remotes/origin/main"); + assert.strictEqual(changes.title, "Changes vs origin/main"); assert.deepStrictEqual(changes.files, [ { path: "unpushed.txt", previousPath: null, additions: 1, deletions: 0 }, ]); @@ -1930,7 +1931,7 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { includeBranchChanges: true, }); assert.deepStrictEqual(status.branchChanges, { - baseRef: "main", + baseRef: "refs/heads/main", insertions: 3, deletions: 0, }); @@ -1946,6 +1947,10 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { const refs = yield* driver.listRefs({ cwd }); assert.equal(refs.isRepo, false); assert.deepStrictEqual(refs.refs, []); + assert.equal( + (yield* driver.statusDetailsRemote(cwd, { refreshUpstream: false })).isRepo, + false, + ); }), ); @@ -2072,35 +2077,41 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { }), ); - it.effect("reports remote divergence without reading working-tree details", () => - Effect.gen(function* () { - const cwd = yield* makeTmpDir(); - const remote = yield* makeTmpDir("git-vcs-driver-remote-"); - const { initialBranch } = yield* initRepoWithCommit(cwd); - yield* git(remote, ["init", "--bare"]); - yield* git(cwd, ["remote", "add", "origin", remote]); - yield* git(cwd, ["push", "-u", "origin", initialBranch]); - yield* git(cwd, ["checkout", "-b", "feature/remote-status"]); - yield* writeTextFile(cwd, "feature.txt", "feature\n"); - yield* git(cwd, ["add", "feature.txt"]); - yield* git(cwd, ["commit", "-m", "feature commit"]); - yield* git(cwd, ["push", "-u", "origin", "feature/remote-status"]); - yield* writeTextFile(cwd, "untracked.txt", "local-only\n"); - - const status = yield* (yield* GitVcsDriver.GitVcsDriver).statusDetailsRemote(cwd); - - assert.equal(status.isRepo, true); - assert.equal(status.branch, "feature/remote-status"); - assert.equal(status.hasUpstream, true); - assert.equal(status.aheadCount, 0); - assert.equal(status.behindCount, 0); - assert.equal(status.aheadOfDefaultCount, 1); - assert.notProperty(status, "workingTree"); - assert.notProperty(status, "hasWorkingTreeChanges"); - }), + it.effect.each(["feature/remote-status", "origin/main", "heads/origin/main"])( + "reports remote divergence for $0 without reading working-tree details", + (branch) => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const remote = yield* makeTmpDir("git-vcs-driver-remote-"); + yield* initRepoWithCommit(cwd); + yield* git(cwd, ["branch", "-M", "main"]); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["push", "-u", "origin", "main"]); + yield* git(cwd, ["checkout", "-b", branch]); + if (branch.startsWith("heads/")) { + yield* git(cwd, ["tag", branch]); + } + yield* writeTextFile(cwd, "feature.txt", "feature\n"); + yield* git(cwd, ["add", "feature.txt"]); + yield* git(cwd, ["commit", "-m", "feature commit"]); + yield* git(cwd, ["push", "-u", "origin", `HEAD:refs/heads/${branch}`]); + yield* writeTextFile(cwd, "untracked.txt", "local-only\n"); + + const status = yield* (yield* GitVcsDriver.GitVcsDriver).statusDetailsRemote(cwd); + + assert.equal(status.isRepo, true); + assert.equal(status.branch, branch); + assert.equal(status.hasUpstream, true); + assert.equal(status.aheadCount, 0); + assert.equal(status.behindCount, 0); + assert.equal(status.aheadOfDefaultCount, 1); + assert.notProperty(status, "workingTree"); + assert.notProperty(status, "hasWorkingTreeChanges"); + }), ); - it.effect("reports remote status on unborn HEAD without failing", () => + it.effect("reports remote status on unborn or detached HEAD without failing", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); const driver = yield* GitVcsDriver.GitVcsDriver; @@ -2114,6 +2125,17 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { assert.equal(status.hasUpstream, false); assert.equal(status.aheadCount, 0); assert.equal(status.behindCount, 0); + yield* git(cwd, ["symbolic-ref", "HEAD", "refs/heads/heads/origin/main"]); + assert.equal( + (yield* driver.statusDetailsRemote(cwd, { refreshUpstream: false })).branch, + "heads/origin/main", + ); + yield* initRepoWithCommit(cwd); + yield* git(cwd, ["checkout", "--detach"]); + const detachedStatus = yield* driver.statusDetailsRemote(cwd, { refreshUpstream: false }); + assert.equal(detachedStatus.isRepo, true); + assert.equal(detachedStatus.branch, null); + assert.equal(detachedStatus.hasUpstream, false); }), ); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 9cbc41e46211..09585c7bcc39 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -1204,7 +1204,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* executeGit( "GitVcsDriver.resolveRepositoryPaths.currentBranch", cwd, - ["symbolic-ref", "--quiet", "--short", "HEAD"], + ["branch", "--show-current"], { timeoutMs: 5_000, allowNonZeroExit: true, @@ -1532,6 +1532,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* }); // `allowRemoteOfCurrent` lets the review diff compare the default branch with its remote copy. + // Full refs keep local branches and tags from shadowing the comparison base. const resolveBaseBranchForNoUpstream = Effect.fn("resolveBaseBranchForNoUpstream")(function* ( cwd: string, refName: string, @@ -1580,7 +1581,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* refName: normalizedCandidate, })) ) { - return `${primaryRemoteName}/${normalizedCandidate}`; + return `refs/remotes/${primaryRemoteName}/${normalizedCandidate}`; } continue; } @@ -1593,11 +1594,11 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* refName: normalizedCandidate, })) ) { - return `${primaryRemoteName}/${normalizedCandidate}`; + return `refs/remotes/${primaryRemoteName}/${normalizedCandidate}`; } if (yield* branchExists(cwd, normalizedCandidate)) { - return normalizedCandidate; + return `refs/heads/${normalizedCandidate}`; } } @@ -1631,7 +1632,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const branchResult = yield* executeGitWithStableDiagnostics( "GitVcsDriver.statusDetailsRemote.branch", cwd, - ["rev-parse", "--abbrev-ref", "HEAD"], + ["branch", "--show-current"], { allowNonZeroExit: true }, ).pipe( Effect.catchTags({ @@ -1643,35 +1644,23 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* if (branchResult === null) { return NON_REPOSITORY_REMOTE_STATUS_DETAILS; } - let branch: string | null; if (branchResult.exitCode !== 0) { if (isNonRepositoryGitStderr(branchResult.stderr)) { return NON_REPOSITORY_REMOTE_STATUS_DETAILS; } - if (!isUnbornHeadStderr(branchResult.stderr)) { - return yield* new GitCommandError({ - ...gitCommandContext({ - operation: "GitVcsDriver.statusDetailsRemote.branch", - cwd, - args: ["rev-parse", "--abbrev-ref", "HEAD"], - }), - detail: "Git branch lookup failed.", - exitCode: branchResult.exitCode, - stdoutLength: branchResult.stdout.length, - stderrLength: branchResult.stderr.length, - }); - } - - const branchValue = yield* runGitStdout( - "GitVcsDriver.statusDetailsRemote.unbornBranch", - cwd, - ["symbolic-ref", "--quiet", "--short", "HEAD"], - ); - branch = branchValue.trim() || null; - } else { - const branchValue = branchResult.stdout.trim(); - branch = branchValue.length > 0 && branchValue !== "HEAD" ? branchValue : null; + return yield* new GitCommandError({ + ...gitCommandContext({ + operation: "GitVcsDriver.statusDetailsRemote.branch", + cwd, + args: ["branch", "--show-current"], + }), + detail: "Git branch lookup failed.", + exitCode: branchResult.exitCode, + stdoutLength: branchResult.stdout.length, + stderrLength: branchResult.stderr.length, + }); } + const branch = branchResult.stdout.trim() || null; const upstream = yield* resolveCurrentUpstream(cwd); const upstreamRef = upstream?.upstreamRef ?? null; let aheadCount = 0; @@ -2730,7 +2719,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* { id: "branch-range", kind: "branch-range", - title: review.baseRef ? `Changes vs ${review.baseRef}` : "Changes", + title: review.baseRef + ? `Changes vs ${review.baseRef.replace(/^refs\/(?:heads|remotes)\//, "")}` + : "Changes", baseRef: review.baseRef, // For display only. The new side is the working tree. headRef: repository.currentBranch ?? "HEAD",