From f7570712a741136999a77d5a8f6490a8c7d9e249 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 9 Mar 2026 16:46:53 -0700 Subject: [PATCH] Avoid fork PR head branch collisions when creating worktrees - create a unique local branch name for cross-repo PR worktrees (`pr/-`) - fetch/set upstream against that local branch instead of overwriting local `main` - add regression tests for fork PRs where head branch is `main` --- apps/server/src/git/Layers/GitManager.test.ts | 111 ++++++++++++++++++ apps/server/src/git/Layers/GitManager.ts | 67 ++++++++--- 2 files changed, 164 insertions(+), 14 deletions(-) diff --git a/apps/server/src/git/Layers/GitManager.test.ts b/apps/server/src/git/Layers/GitManager.test.ts index ea0097e59825bd4c9f0774204789e3dcec460e4b..2cef6489c2b12ea6ca79cb1c09972cb8fcc66b4f 100644 --- a/apps/server/src/git/Layers/GitManager.test.ts +++ b/apps/server/src/git/Layers/GitManager.test.ts @@ -1407,6 +1407,117 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); + it.effect("does not block fork PR worktree prep when the fork head branch collides with root main", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const originDir = yield* createBareRemote(); + const forkDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", originDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* runGit(repoDir, ["remote", "add", "fork-seed", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "fork-main-source"]); + fs.writeFileSync(path.join(repoDir, "fork-main.txt"), "fork main\n"); + yield* runGit(repoDir, ["add", "fork-main.txt"]); + yield* runGit(repoDir, ["commit", "-m", "Fork main branch"]); + yield* runGit(repoDir, ["push", "-u", "fork-seed", "fork-main-source:main"]); + yield* runGit(repoDir, ["checkout", "main"]); + const mainBefore = (yield* runGit(repoDir, ["rev-parse", "main"])).stdout.trim(); + + const { manager } = yield* makeManager({ + ghScenario: { + pullRequest: { + number: 91, + title: "Fork main PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/91", + baseRefName: "main", + headRefName: "main", + state: "open", + isCrossRepository: true, + headRepositoryNameWithOwner: "octocat/codething-mvp", + headRepositoryOwnerLogin: "octocat", + }, + repositoryCloneUrls: { + "octocat/codething-mvp": { + url: forkDir, + sshUrl: forkDir, + }, + }, + }, + }); + + const result = yield* preparePullRequestThread(manager, { + cwd: repoDir, + reference: "91", + mode: "worktree", + }); + + expect(result.branch).toBe("pr/91-main"); + expect(result.worktreePath).not.toBeNull(); + expect((yield* runGit(repoDir, ["branch", "--show-current"])).stdout.trim()).toBe("main"); + expect((yield* runGit(repoDir, ["rev-parse", "main"])).stdout.trim()).toBe(mainBefore); + expect( + (yield* runGit(result.worktreePath as string, ["branch", "--show-current"])).stdout.trim(), + ).toBe("pr/91-main"); + }), + ); + + it.effect("does not overwrite an existing local main branch when preparing a fork PR worktree", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const originDir = yield* createBareRemote(); + const forkDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", originDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* runGit(repoDir, ["remote", "add", "fork-seed", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "fork-main-source"]); + fs.writeFileSync(path.join(repoDir, "fork-main-second.txt"), "fork main second\n"); + yield* runGit(repoDir, ["add", "fork-main-second.txt"]); + yield* runGit(repoDir, ["commit", "-m", "Fork main second branch"]); + yield* runGit(repoDir, ["push", "-u", "fork-seed", "fork-main-source:main"]); + yield* runGit(repoDir, ["checkout", "main"]); + const localMainBefore = (yield* runGit(repoDir, ["rev-parse", "main"])).stdout.trim(); + yield* runGit(repoDir, ["checkout", "-b", "feature/root-branch"]); + + const { manager } = yield* makeManager({ + ghScenario: { + pullRequest: { + number: 92, + title: "Fork main overwrite PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/92", + baseRefName: "main", + headRefName: "main", + state: "open", + isCrossRepository: true, + headRepositoryNameWithOwner: "octocat/codething-mvp", + headRepositoryOwnerLogin: "octocat", + }, + repositoryCloneUrls: { + "octocat/codething-mvp": { + url: forkDir, + sshUrl: forkDir, + }, + }, + }, + }); + + const result = yield* preparePullRequestThread(manager, { + cwd: repoDir, + reference: "92", + mode: "worktree", + }); + + expect(result.branch).toBe("pr/92-main"); + expect((yield* runGit(repoDir, ["rev-parse", "main"])).stdout.trim()).toBe(localMainBefore); + expect( + (yield* runGit(result.worktreePath as string, ["rev-parse", "--abbrev-ref", "@{upstream}"])) + .stdout + .trim(), + ).toBe("fork-seed/main"); + }), + ); + it.effect("reuses an existing PR worktree and restores fork upstream tracking", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); diff --git a/apps/server/src/git/Layers/GitManager.ts b/apps/server/src/git/Layers/GitManager.ts index 94285a0c23ff59a1435105231e4da565f7b43ff3..072bdcc1e374c665411e394e83d06ab223d2bdd1 100644 --- a/apps/server/src/git/Layers/GitManager.ts +++ b/apps/server/src/git/Layers/GitManager.ts @@ -2,7 +2,11 @@ import { randomUUID } from "node:crypto"; import { realpathSync } from "node:fs"; import { Effect, FileSystem, Layer, Path } from "effect"; -import { resolveAutoFeatureBranchName, sanitizeFeatureBranchName } from "@t3tools/shared/git"; +import { + resolveAutoFeatureBranchName, + sanitizeBranchFragment, + sanitizeFeatureBranchName, +} from "@t3tools/shared/git"; import { GitManagerError } from "../Errors.ts"; import { GitManager, type GitManagerShape } from "../Services/GitManager.ts"; @@ -66,6 +70,18 @@ function resolveHeadRepositoryNameWithOwner( return `${ownerLogin}/${repositoryName}`; } +function resolvePullRequestWorktreeLocalBranchName( + pullRequest: ResolvedPullRequest & PullRequestHeadRemoteInfo, +): string { + if (!pullRequest.isCrossRepository) { + return pullRequest.headBranch; + } + + const sanitizedHeadBranch = sanitizeBranchFragment(pullRequest.headBranch).trim(); + const suffix = sanitizedHeadBranch.length > 0 ? sanitizedHeadBranch : "head"; + return `pr/${pullRequest.number}-${suffix}`; +} + function parsePullRequestList(raw: unknown): PullRequestInfo[] { if (!Array.isArray(raw)) return []; @@ -322,6 +338,7 @@ export const makeGitManager = Effect.gen(function* () { const materializePullRequestHeadBranch = ( cwd: string, pullRequest: ResolvedPullRequest & PullRequestHeadRemoteInfo, + localBranch = pullRequest.headBranch, ) => Effect.gen(function* () { const repositoryNameWithOwner = resolveHeadRepositoryNameWithOwner(pullRequest) ?? ""; @@ -330,7 +347,7 @@ export const makeGitManager = Effect.gen(function* () { yield* gitCore.fetchPullRequestBranch({ cwd, prNumber: pullRequest.number, - branch: pullRequest.headBranch, + branch: localBranch, }); return; } @@ -355,11 +372,11 @@ export const makeGitManager = Effect.gen(function* () { cwd, remoteName, remoteBranch: pullRequest.headBranch, - localBranch: pullRequest.headBranch, + localBranch, }); yield* gitCore.setBranchUpstream({ cwd, - branch: pullRequest.headBranch, + branch: localBranch, remoteName, remoteBranch: pullRequest.headBranch, }); @@ -368,7 +385,7 @@ export const makeGitManager = Effect.gen(function* () { gitCore.fetchPullRequestBranch({ cwd, prNumber: pullRequest.number, - branch: pullRequest.headBranch, + branch: localBranch, }), ), ); @@ -701,14 +718,32 @@ export const makeGitManager = Effect.gen(function* () { ...pullRequest, ...toPullRequestHeadRemoteInfo(pullRequestSummary), } as const; + const localPullRequestBranch = resolvePullRequestWorktreeLocalBranchName( + pullRequestWithRemoteInfo, + ); const findLocalHeadBranch = (cwd: string) => gitCore.listBranches({ cwd }).pipe( - Effect.map((result) => - result.branches.find( - (branch) => !branch.isRemote && branch.name === pullRequest.headBranch, - ), - ), + Effect.map((result) => { + const localBranch = result.branches.find( + (branch) => !branch.isRemote && branch.name === localPullRequestBranch, + ); + if (localBranch) { + return localBranch; + } + if (localPullRequestBranch === pullRequest.headBranch) { + return null; + } + return ( + result.branches.find( + (branch) => + !branch.isRemote && + branch.name === pullRequest.headBranch && + branch.worktreePath !== null && + canonicalizeExistingPath(branch.worktreePath) !== rootWorktreePath, + ) ?? null + ); + }), ); const existingBranchBeforeFetch = yield* findLocalHeadBranch(input.cwd); @@ -722,7 +757,7 @@ export const makeGitManager = Effect.gen(function* () { yield* ensureExistingWorktreeUpstream(existingBranchBeforeFetch.worktreePath); return { pullRequest, - branch: pullRequest.headBranch, + branch: localPullRequestBranch, worktreePath: existingBranchBeforeFetch.worktreePath, }; } @@ -733,7 +768,11 @@ export const makeGitManager = Effect.gen(function* () { ); } - yield* materializePullRequestHeadBranch(input.cwd, pullRequestWithRemoteInfo); + yield* materializePullRequestHeadBranch( + input.cwd, + pullRequestWithRemoteInfo, + localPullRequestBranch, + ); const existingBranchAfterFetch = yield* findLocalHeadBranch(input.cwd); const existingBranchAfterFetchPath = existingBranchAfterFetch?.worktreePath @@ -746,7 +785,7 @@ export const makeGitManager = Effect.gen(function* () { yield* ensureExistingWorktreeUpstream(existingBranchAfterFetch.worktreePath); return { pullRequest, - branch: pullRequest.headBranch, + branch: localPullRequestBranch, worktreePath: existingBranchAfterFetch.worktreePath, }; } @@ -759,7 +798,7 @@ export const makeGitManager = Effect.gen(function* () { const worktree = yield* gitCore.createWorktree({ cwd: input.cwd, - branch: pullRequest.headBranch, + branch: localPullRequestBranch, path: null, }); yield* ensureExistingWorktreeUpstream(worktree.worktree.path); -- 2.54.0