From c6f1f9ad8912f91d6cbd13c97ea7c4510c912c9d Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 24 Oct 2026 08:19:02 -0500 Subject: [PATCH 1/6] fix(server): PR worktrees keep local commits on an existing branch A worktree PR thread force-moved an existing local branch that no worktree had checked out onto the PR head, dropping commits only that branch held. Check the branch out as it is and let the reused-worktree refresh move it only where that loses nothing. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/git/GitManager.test.ts | 74 ++++++++++++++++++++++++++ apps/server/src/git/GitManager.ts | 46 ++++++++++------ 2 files changed, 105 insertions(+), 15 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 51651f4dfa70f..324f80e968353 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -4337,6 +4337,80 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); + it.effect.each([ + { localBranch: "ahead", expectedHead: "local", isOnPullRequestHead: false }, + { localBranch: "behind", expectedHead: "pull request", isOnPullRequestHead: true }, + ] as const)( + "moves an existing PR branch into a new worktree only when nothing is lost ($localBranch)", + ({ localBranch, expectedHead, isOnPullRequestHead }) => + 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"]); + yield* runGit(repoDir, ["checkout", "-b", "feature/pr-existing"]); + NodeFS.writeFileSync(NodePath.join(repoDir, "existing.txt"), "pushed\n"); + yield* runGit(repoDir, ["add", "existing.txt"]); + yield* runGit(repoDir, ["commit", "-m", "Pushed PR commit"]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-existing"]); + if (localBranch === "ahead") { + NodeFS.writeFileSync(NodePath.join(repoDir, "mine.txt"), "mine\n"); + yield* runGit(repoDir, ["add", "mine.txt"]); + yield* runGit(repoDir, ["commit", "-m", "My unpublished commit"]); + } else { + yield* runGit(repoDir, ["checkout", "-b", "author-push"]); + NodeFS.writeFileSync(NodePath.join(repoDir, "authored.txt"), "authored\n"); + yield* runGit(repoDir, ["add", "authored.txt"]); + yield* runGit(repoDir, ["commit", "-m", "New PR head commit"]); + yield* runGit(repoDir, ["push", "origin", "author-push:feature/pr-existing"]); + } + const localHead = (yield* runGit(repoDir, [ + "rev-parse", + "feature/pr-existing", + ])).stdout.trim(); + const pullHead = (yield* runGit(repoDir, [ + "rev-parse", + "origin/feature/pr-existing", + ])).stdout.trim(); + yield* runGit(repoDir, ["checkout", "main"]); + + const { manager } = yield* makeManager({ + providerScenario: { + pullRequest: { + number: 88, + title: "Existing branch PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/88", + baseRefName: "main", + headRefName: "feature/pr-existing", + state: "open", + isCrossRepository: false, + headRepositoryNameWithOwner: "pingdotgg/codething-mvp", + headRepositoryOwnerLogin: "pingdotgg", + }, + repositoryCloneUrls: { + "pingdotgg/codething-mvp": { url: remoteDir, sshUrl: remoteDir }, + }, + }, + }); + + const result = yield* preparePullRequestThread(manager, { + cwd: repoDir, + reference: "88", + mode: "worktree", + }); + + const head = expectedHead === "local" ? localHead : pullHead; + expect((yield* runGit(repoDir, ["rev-parse", "feature/pr-existing"])).stdout.trim()).toBe( + head, + ); + expect( + (yield* runGit(result.worktreePath as string, ["rev-parse", "HEAD"])).stdout.trim(), + ).toBe(head); + expect(result).toMatchObject({ branch: "feature/pr-existing", isOnPullRequestHead }); + }), + ); + it.effect("refreshes a reused PR worktree onto the updated pull request head", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index de5fb137845b1..d4067533c7beb 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -2815,6 +2815,7 @@ export const make = Effect.gen(function* () { const reuseExistingWorktree = Effect.fn("reuseExistingWorktree")(function* ( worktreePath: string, checkedOutBranch: string, + isNewWorktree = false, ) { if (checkedOutBranch !== localPullRequestBranch) { // findLocalHeadBranch also accepts a branch that merely shares the head's bare name — @@ -2889,7 +2890,7 @@ export const make = Effect.gen(function* () { // Only when the checkout actually moved: another thread may be running in this worktree, // and re-running the setup script under it buys nothing when the code did not change. - if (refreshed.moved) { + if (refreshed.moved || isNewWorktree) { yield* maybeRunSetupScript(worktreePath); } @@ -2935,6 +2936,23 @@ export const make = Effect.gen(function* () { return null; }); + const createPullRequestWorktree = Effect.fn("createPullRequestWorktree")(function* () { + return yield* gitCore.createWorktree( + { + cwd: input.cwd, + refName: localPullRequestBranch, + path: null, + }, + { + worktreesDirectory: yield* readWorktreesDirectory, + submodules: yield* projectSettingsFor(input).pipe( + Effect.map(({ settings }) => settings.worktreeSubmodules), + Effect.orElseSucceed(() => null), + ), + }, + ); + }); + const existingBranchBeforeFetch = yield* findLocalHeadBranch(input.cwd); const existingBranchBeforeFetchPath = existingBranchBeforeFetch?.worktreePath ? yield* canonicalizeExistingPath(existingBranchBeforeFetch.worktreePath) @@ -2956,6 +2974,17 @@ export const make = Effect.gen(function* () { "This PR branch is already checked out in the main repo. Use Local, or switch the main repo off that branch before creating a worktree thread.", }); } + // A branch no worktree has checked out can still carry commits nothing else holds, and + // materializing would force it onto the head. Check it out as it is, and let the reused + // worktree refresh move it only where that loses nothing. + if (existingBranchBeforeFetch !== null) { + const worktree = yield* createPullRequestWorktree(); + return yield* reuseExistingWorktree( + worktree.worktree.path, + existingBranchBeforeFetch.name, + true, + ); + } yield* materializePullRequestHeadBranch( input.cwd, @@ -2985,20 +3014,7 @@ export const make = Effect.gen(function* () { }); } - const worktree = yield* gitCore.createWorktree( - { - cwd: input.cwd, - refName: localPullRequestBranch, - path: null, - }, - { - worktreesDirectory: yield* readWorktreesDirectory, - submodules: yield* projectSettingsFor(input).pipe( - Effect.map(({ settings }) => settings.worktreeSubmodules), - Effect.orElseSucceed(() => null), - ), - }, - ); + const worktree = yield* createPullRequestWorktree(); yield* ensureExistingWorktreeUpstream(worktree.worktree.path); yield* maybeRunSetupScript(worktree.worktree.path); From 84288a1ee25c8dbf165fb2db5c5546fd7fa2151a Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 24 Oct 2026 20:21:57 -0500 Subject: [PATCH 2/6] fix(server): a moved PR branch's new worktree checks out the head's submodules Checking out an existing PR branch as it is and then fast-forwarding it left the worktree's submodules at the commit it was created at, so a PR head that adds or moves a submodule set up an incomplete checkout. The submodule step of createWorktree is now its own driver method, and the new-worktree path runs it again when the refresh moved the checkout. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/git/GitManager.test.ts | 64 +++++++++++++++++++ apps/server/src/git/GitManager.ts | 17 ++++-- apps/server/src/vcs/GitVcsDriver.ts | 5 ++ apps/server/src/vcs/GitVcsDriverCore.ts | 81 ++++++++++++++----------- 4 files changed, 127 insertions(+), 40 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 324f80e968353..644e525c22564 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -4411,6 +4411,70 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); + it.effect("checks out the submodules of the head an existing PR branch moves onto", () => + Effect.gen(function* () { + // Git refuses `file:` submodule transports by default (CVE-2022-39253) and ignores + // repo-level config for it, so a local fixture needs the env allowance. + const previousAllowedProtocol = process.env.GIT_ALLOW_PROTOCOL; + process.env.GIT_ALLOW_PROTOCOL = "file"; + yield* Effect.addFinalizer(() => + Effect.sync(() => { + if (previousAllowedProtocol === undefined) { + delete process.env.GIT_ALLOW_PROTOCOL; + } else { + process.env.GIT_ALLOW_PROTOCOL = previousAllowedProtocol; + } + }), + ); + + const submoduleDir = yield* makeTempDir("t3code-git-submodule-"); + yield* initRepo(submoduleDir); + 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"]); + yield* runGit(repoDir, ["checkout", "-b", "feature/pr-existing"]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-existing"]); + yield* runGit(repoDir, ["checkout", "-b", "author-push"]); + yield* runGit(repoDir, ["submodule", "add", submoduleDir, "shared"]); + yield* runGit(repoDir, ["commit", "-m", "Add shared submodule"]); + yield* runGit(repoDir, ["push", "origin", "author-push:feature/pr-existing"]); + const pullHead = (yield* runGit(repoDir, ["rev-parse", "author-push"])).stdout.trim(); + yield* runGit(repoDir, ["checkout", "main"]); + + const { manager } = yield* makeManager({ + providerScenario: { + pullRequest: { + number: 89, + title: "Submodule PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/89", + baseRefName: "main", + headRefName: "feature/pr-existing", + state: "open", + isCrossRepository: false, + headRepositoryNameWithOwner: "pingdotgg/codething-mvp", + headRepositoryOwnerLogin: "pingdotgg", + }, + repositoryCloneUrls: { + "pingdotgg/codething-mvp": { url: remoteDir, sshUrl: remoteDir }, + }, + }, + }); + + const result = yield* preparePullRequestThread(manager, { + cwd: repoDir, + reference: "89", + mode: "worktree", + }); + + const worktreePath = result.worktreePath as string; + expect((yield* runGit(worktreePath, ["rev-parse", "HEAD"])).stdout.trim()).toBe(pullHead); + expect(NodeFS.existsSync(NodePath.join(worktreePath, "shared", "README.md"))).toBe(true); + expect(result).toMatchObject({ isOnPullRequestHead: true }); + }), + ); + it.effect("refreshes a reused PR worktree onto the updated pull request head", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index d4067533c7beb..64c35e3b15317 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -2809,6 +2809,11 @@ export const make = Effect.gen(function* () { const localPullRequestBranch = resolvePullRequestWorktreeLocalBranchName(pullRequestWithRemoteInfo); + const readWorktreeSubmodules = projectSettingsFor(input).pipe( + Effect.map(({ settings }) => settings.worktreeSubmodules), + Effect.orElseSucceed(() => null), + ); + // Git refuses to move a branch that is checked out in a worktree, so the // reuse paths cannot go through materializePullRequestHeadBranch and instead // advance the checkout from inside the worktree. @@ -2888,6 +2893,13 @@ export const make = Effect.gen(function* () { ), ); + // A new worktree checked out its submodules for the commit it was created at. + if (isNewWorktree && refreshed.moved) { + yield* gitCore.updateWorktreeSubmodules(worktreePath, { + submodules: yield* readWorktreeSubmodules, + }); + } + // Only when the checkout actually moved: another thread may be running in this worktree, // and re-running the setup script under it buys nothing when the code did not change. if (refreshed.moved || isNewWorktree) { @@ -2945,10 +2957,7 @@ export const make = Effect.gen(function* () { }, { worktreesDirectory: yield* readWorktreesDirectory, - submodules: yield* projectSettingsFor(input).pipe( - Effect.map(({ settings }) => settings.worktreeSubmodules), - Effect.orElseSucceed(() => null), - ), + submodules: yield* readWorktreeSubmodules, }, ); }); diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 0599b0caf24bd..6c1c8803d5639 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -357,6 +357,11 @@ export class GitVcsDriver extends Context.Service< input: VcsCreateWorktreeInput, options?: CreateWorktreeOptions, ) => Effect.Effect; + /** Checks out the submodules of the worktree's current revision, as `createWorktree` does. */ + readonly updateWorktreeSubmodules: ( + worktreePath: string, + options?: CreateWorktreeOptions, + ) => Effect.Effect; /** Force-updates local `branch` to the primary remote's `ref`. */ readonly fetchRefIntoBranch: ( input: GitFetchRefInput & { readonly branch: string }, diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 5a9aa74af60aa..f83719217bfa5 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -3781,6 +3781,50 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* progress.onWorktreeClaimed(worktreePath); } + yield* updateWorktreeSubmodules(worktreePath, options); + + const newRefName = input.newRefName; + if (newRefName && input.baseRefName) { + const base = yield* resolveBranchBase(input.cwd, input.baseRefName); + if (base) { + yield* Effect.gen(function* () { + yield* runGit("GitVcsDriver.createWorktree.configureBaseRef", input.cwd, [ + "config", + branchBaseConfigKey(newRefName), + base.branchName, + ]); + if (base.baseRef.startsWith("refs/remotes/")) { + // The base key names a branch on the host; this one keeps the remote it came from. + yield* runGit("GitVcsDriver.createWorktree.configureComparisonRef", input.cwd, [ + "config", + `branch.${input.newRefName}.t3-merge-base-ref`, + base.baseRef, + ]); + } + }).pipe( + Effect.catchTags({ + GitCommandError: () => + Effect.logWarning( + "worktree base-ref configuration failed; using base-branch fallback", + ), + }), + ); + } + } + + return { + worktree: { + path: worktreePath, + refName: targetBranch, + }, + }; + }); + + const updateWorktreeSubmodules = Effect.fn("updateWorktreeSubmodules")(function* ( + worktreePath: string, + options?: GitVcsDriver.CreateWorktreeOptions, + ) { + const progress = options?.progress; // `git worktree add` leaves submodules empty, so a repo that keeps agent // skills, tooling or source in one gets a worktree that is quietly missing // them. @@ -3848,42 +3892,6 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* }), ); } - - const newRefName = input.newRefName; - if (newRefName && input.baseRefName) { - const base = yield* resolveBranchBase(input.cwd, input.baseRefName); - if (base) { - yield* Effect.gen(function* () { - yield* runGit("GitVcsDriver.createWorktree.configureBaseRef", input.cwd, [ - "config", - branchBaseConfigKey(newRefName), - base.branchName, - ]); - if (base.baseRef.startsWith("refs/remotes/")) { - // The base key names a branch on the host; this one keeps the remote it came from. - yield* runGit("GitVcsDriver.createWorktree.configureComparisonRef", input.cwd, [ - "config", - `branch.${input.newRefName}.t3-merge-base-ref`, - base.baseRef, - ]); - } - }).pipe( - Effect.catchTags({ - GitCommandError: () => - Effect.logWarning( - "worktree base-ref configuration failed; using base-branch fallback", - ), - }), - ); - } - } - - return { - worktree: { - path: worktreePath, - refName: targetBranch, - }, - }; }); const fetchRefIntoBranch: GitVcsDriver.GitVcsDriver["Service"]["fetchRefIntoBranch"] = Effect.fn( @@ -4522,6 +4530,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* listRefs, createWorktree: (input, options) => withListRefsInvalidation(input.cwd, createWorktree(input, options)), + updateWorktreeSubmodules, fetchRefIntoBranch: (input) => withListRefsInvalidation(input.cwd, fetchRefIntoBranch(input)), fetchRefCommit, resolveCommit, From 8c9417ac010543c1116a459f4826fe291695e386 Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 24 Oct 2026 20:56:32 -0500 Subject: [PATCH 3/6] fix(server): move an existing PR branch before creating its worktree The worktree used to be created at the branch's old commit and then moved onto the pull request head. A head that removed a submodule left the old checkout behind as untracked files, so later reopens saw local changes. The branch now moves first, under the same lossless rule the reused-worktree refresh uses, and the worktree is created at the head. That also drops the second submodule pass and its driver method. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/git/GitManager.test.ts | 155 ++++++++++++++---------- apps/server/src/git/GitManager.ts | 121 ++++++++++++++---- apps/server/src/vcs/GitVcsDriver.ts | 5 - apps/server/src/vcs/GitVcsDriverCore.ts | 81 ++++++------- 4 files changed, 228 insertions(+), 134 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 644e525c22564..ea69195a67406 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -4340,6 +4340,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { it.effect.each([ { localBranch: "ahead", expectedHead: "local", isOnPullRequestHead: false }, { localBranch: "behind", expectedHead: "pull request", isOnPullRequestHead: true }, + { localBranch: "rewritten", expectedHead: "pull request", isOnPullRequestHead: true }, ] as const)( "moves an existing PR branch into a new worktree only when nothing is lost ($localBranch)", ({ localBranch, expectedHead, isOnPullRequestHead }) => @@ -4358,21 +4359,39 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { NodeFS.writeFileSync(NodePath.join(repoDir, "mine.txt"), "mine\n"); yield* runGit(repoDir, ["add", "mine.txt"]); yield* runGit(repoDir, ["commit", "-m", "My unpublished commit"]); - } else { + } else if (localBranch === "behind") { yield* runGit(repoDir, ["checkout", "-b", "author-push"]); NodeFS.writeFileSync(NodePath.join(repoDir, "authored.txt"), "authored\n"); yield* runGit(repoDir, ["add", "authored.txt"]); yield* runGit(repoDir, ["commit", "-m", "New PR head commit"]); yield* runGit(repoDir, ["push", "origin", "author-push:feature/pr-existing"]); + } else { + const staleHead = (yield* runGit(repoDir, ["rev-parse", "HEAD"])).stdout.trim(); + yield* runGit(repoDir, ["checkout", "-b", "author-rewrite"]); + NodeFS.writeFileSync(NodePath.join(repoDir, "existing.txt"), "rewritten\n"); + yield* runGit(repoDir, ["add", "existing.txt"]); + yield* runGit(repoDir, ["commit", "--amend", "-m", "Rewritten PR head"]); + yield* runGit(repoDir, [ + "push", + "--force", + "origin", + "author-rewrite:feature/pr-existing", + ]); + yield* runGit(repoDir, [ + "update-ref", + "refs/remotes/origin/feature/pr-existing", + staleHead, + ]); } const localHead = (yield* runGit(repoDir, [ "rev-parse", "feature/pr-existing", ])).stdout.trim(); const pullHead = (yield* runGit(repoDir, [ - "rev-parse", - "origin/feature/pr-existing", - ])).stdout.trim(); + "ls-remote", + "origin", + "refs/heads/feature/pr-existing", + ])).stdout.split("\t")[0]; yield* runGit(repoDir, ["checkout", "main"]); const { manager } = yield* makeManager({ @@ -4411,68 +4430,82 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); - it.effect("checks out the submodules of the head an existing PR branch moves onto", () => - Effect.gen(function* () { - // Git refuses `file:` submodule transports by default (CVE-2022-39253) and ignores - // repo-level config for it, so a local fixture needs the env allowance. - const previousAllowedProtocol = process.env.GIT_ALLOW_PROTOCOL; - process.env.GIT_ALLOW_PROTOCOL = "file"; - yield* Effect.addFinalizer(() => - Effect.sync(() => { - if (previousAllowedProtocol === undefined) { - delete process.env.GIT_ALLOW_PROTOCOL; - } else { - process.env.GIT_ALLOW_PROTOCOL = previousAllowedProtocol; - } - }), - ); + it.effect.each(["adds", "removes"] as const)( + "checks out the submodules of the head an existing PR branch moves onto (head %s one)", + (headChange) => + Effect.gen(function* () { + // Git refuses `file:` submodule transports by default (CVE-2022-39253) and ignores + // repo-level config for it, so a local fixture needs the env allowance. + const previousAllowedProtocol = process.env.GIT_ALLOW_PROTOCOL; + process.env.GIT_ALLOW_PROTOCOL = "file"; + yield* Effect.addFinalizer(() => + Effect.sync(() => { + if (previousAllowedProtocol === undefined) { + delete process.env.GIT_ALLOW_PROTOCOL; + } else { + process.env.GIT_ALLOW_PROTOCOL = previousAllowedProtocol; + } + }), + ); - const submoduleDir = yield* makeTempDir("t3code-git-submodule-"); - yield* initRepo(submoduleDir); - 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"]); - yield* runGit(repoDir, ["checkout", "-b", "feature/pr-existing"]); - yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-existing"]); - yield* runGit(repoDir, ["checkout", "-b", "author-push"]); - yield* runGit(repoDir, ["submodule", "add", submoduleDir, "shared"]); - yield* runGit(repoDir, ["commit", "-m", "Add shared submodule"]); - yield* runGit(repoDir, ["push", "origin", "author-push:feature/pr-existing"]); - const pullHead = (yield* runGit(repoDir, ["rev-parse", "author-push"])).stdout.trim(); - yield* runGit(repoDir, ["checkout", "main"]); + const submoduleDir = yield* makeTempDir("t3code-git-submodule-"); + yield* initRepo(submoduleDir); + 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"]); + yield* runGit(repoDir, ["checkout", "-b", "feature/pr-existing"]); + if (headChange === "removes") { + yield* runGit(repoDir, ["submodule", "add", submoduleDir, "shared"]); + yield* runGit(repoDir, ["commit", "-m", "Add shared submodule"]); + } + yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-existing"]); + yield* runGit(repoDir, ["checkout", "-b", "author-push"]); + if (headChange === "adds") { + yield* runGit(repoDir, ["submodule", "add", submoduleDir, "shared"]); + yield* runGit(repoDir, ["commit", "-m", "Add shared submodule"]); + } else { + yield* runGit(repoDir, ["rm", "shared"]); + yield* runGit(repoDir, ["commit", "-m", "Remove shared submodule"]); + } + yield* runGit(repoDir, ["push", "origin", "author-push:feature/pr-existing"]); + const pullHead = (yield* runGit(repoDir, ["rev-parse", "author-push"])).stdout.trim(); + yield* runGit(repoDir, ["checkout", "main"]); - const { manager } = yield* makeManager({ - providerScenario: { - pullRequest: { - number: 89, - title: "Submodule PR", - url: "https://github.com/pingdotgg/codething-mvp/pull/89", - baseRefName: "main", - headRefName: "feature/pr-existing", - state: "open", - isCrossRepository: false, - headRepositoryNameWithOwner: "pingdotgg/codething-mvp", - headRepositoryOwnerLogin: "pingdotgg", - }, - repositoryCloneUrls: { - "pingdotgg/codething-mvp": { url: remoteDir, sshUrl: remoteDir }, + const { manager } = yield* makeManager({ + providerScenario: { + pullRequest: { + number: 89, + title: "Submodule PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/89", + baseRefName: "main", + headRefName: "feature/pr-existing", + state: "open", + isCrossRepository: false, + headRepositoryNameWithOwner: "pingdotgg/codething-mvp", + headRepositoryOwnerLogin: "pingdotgg", + }, + repositoryCloneUrls: { + "pingdotgg/codething-mvp": { url: remoteDir, sshUrl: remoteDir }, + }, }, - }, - }); + }); - const result = yield* preparePullRequestThread(manager, { - cwd: repoDir, - reference: "89", - mode: "worktree", - }); + const result = yield* preparePullRequestThread(manager, { + cwd: repoDir, + reference: "89", + mode: "worktree", + }); - const worktreePath = result.worktreePath as string; - expect((yield* runGit(worktreePath, ["rev-parse", "HEAD"])).stdout.trim()).toBe(pullHead); - expect(NodeFS.existsSync(NodePath.join(worktreePath, "shared", "README.md"))).toBe(true); - expect(result).toMatchObject({ isOnPullRequestHead: true }); - }), + const worktreePath = result.worktreePath as string; + expect((yield* runGit(worktreePath, ["rev-parse", "HEAD"])).stdout.trim()).toBe(pullHead); + expect(NodeFS.existsSync(NodePath.join(worktreePath, "shared", "README.md"))).toBe( + headChange === "adds", + ); + expect((yield* runGit(worktreePath, ["status", "--porcelain"])).stdout).toBe(""); + expect(result).toMatchObject({ isOnPullRequestHead: true }); + }), ); it.effect("refreshes a reused PR worktree onto the updated pull request head", () => diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 64c35e3b15317..fab7f9db24528 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -2809,18 +2809,12 @@ export const make = Effect.gen(function* () { const localPullRequestBranch = resolvePullRequestWorktreeLocalBranchName(pullRequestWithRemoteInfo); - const readWorktreeSubmodules = projectSettingsFor(input).pipe( - Effect.map(({ settings }) => settings.worktreeSubmodules), - Effect.orElseSucceed(() => null), - ); - // Git refuses to move a branch that is checked out in a worktree, so the // reuse paths cannot go through materializePullRequestHeadBranch and instead // advance the checkout from inside the worktree. const reuseExistingWorktree = Effect.fn("reuseExistingWorktree")(function* ( worktreePath: string, checkedOutBranch: string, - isNewWorktree = false, ) { if (checkedOutBranch !== localPullRequestBranch) { // findLocalHeadBranch also accepts a branch that merely shares the head's bare name — @@ -2893,16 +2887,9 @@ export const make = Effect.gen(function* () { ), ); - // A new worktree checked out its submodules for the commit it was created at. - if (isNewWorktree && refreshed.moved) { - yield* gitCore.updateWorktreeSubmodules(worktreePath, { - submodules: yield* readWorktreeSubmodules, - }); - } - // Only when the checkout actually moved: another thread may be running in this worktree, // and re-running the setup script under it buys nothing when the code did not change. - if (refreshed.moved || isNewWorktree) { + if (refreshed.moved) { yield* maybeRunSetupScript(worktreePath); } @@ -2948,6 +2935,81 @@ export const make = Effect.gen(function* () { return null; }); + // A branch no worktree has checked out can still carry commits nothing else holds, so it + // takes the head only where refreshCheckedOutBranch would move a checkout. It moves before + // its worktree exists because `git worktree add` checks out submodules for the commit it + // starts at, and a later move would leave them at the old commit's. + const moveIdleBranchToPullRequestHead = Effect.fn("moveIdleBranchToPullRequestHead")( + function* (branch: string) { + const { commitSha: branchCommit } = yield* gitCore.resolveCommit({ + cwd: input.cwd, + revision: `refs/heads/${branch}`, + }); + // Read before the upstream fetch, for the same reason as in reuseExistingWorktree. + const upstreamCommitBeforeFetch = yield* gitCore + .resolveCommit({ cwd: input.cwd, revision: `${branch}@{upstream}` }) + .pipe( + Effect.map((resolved) => resolved.commitSha), + Effect.orElseSucceed(() => null), + ); + + yield* configurePullRequestHeadUpstream(input.cwd, pullRequestWithRemoteInfo, branch); + + const { commitSha: targetCommit } = yield* changeRequestHeadRef( + input.cwd, + pullRequest.number, + ).pipe( + Effect.flatMap((ref) => gitCore.fetchRefCommit({ cwd: input.cwd, ref })), + Effect.catch(() => + Effect.gen(function* () { + const merge = yield* gitCore.readConfigValue(input.cwd, `branch.${branch}.merge`); + if (merge !== `refs/heads/${pullRequest.headBranch}`) { + return yield* new GitManagerError({ + operation: "preparePullRequestThread", + cwd: input.cwd, + detail: "The pull request head could not be resolved for this branch.", + }); + } + return yield* gitCore.resolveCommit({ + cwd: input.cwd, + revision: `${branch}@{upstream}`, + }); + }), + ), + ); + if (targetCommit === branchCommit) { + return true; + } + + const isAncestor = yield* gitCore + .execute({ + operation: "GitManager.preparePullRequestThread.isAncestor", + cwd: input.cwd, + args: ["merge-base", "--is-ancestor", branchCommit, targetCommit], + allowNonZeroExit: true, + }) + .pipe(Effect.map((result) => result.exitCode === 0)); + if (!isAncestor && branchCommit !== upstreamCommitBeforeFetch) { + return false; + } + if (!isAncestor) { + // As refreshCheckedOutBranch does, so the rewritten-away commit stays findable. + yield* gitCore.execute({ + operation: "GitManager.preparePullRequestThread.keepPrevious", + cwd: input.cwd, + args: ["update-ref", "refs/t3code/pre-refresh", branchCommit], + }); + } + // The old value makes git refuse the move if the branch changed since it was read. + yield* gitCore.execute({ + operation: "GitManager.preparePullRequestThread.moveBranch", + cwd: input.cwd, + args: ["update-ref", `refs/heads/${branch}`, targetCommit, branchCommit], + }); + return true; + }, + ); + const createPullRequestWorktree = Effect.fn("createPullRequestWorktree")(function* () { return yield* gitCore.createWorktree( { @@ -2957,7 +3019,10 @@ export const make = Effect.gen(function* () { }, { worktreesDirectory: yield* readWorktreesDirectory, - submodules: yield* readWorktreeSubmodules, + submodules: yield* projectSettingsFor(input).pipe( + Effect.map(({ settings }) => settings.worktreeSubmodules), + Effect.orElseSucceed(() => null), + ), }, ); }); @@ -2983,16 +3048,26 @@ export const make = Effect.gen(function* () { "This PR branch is already checked out in the main repo. Use Local, or switch the main repo off that branch before creating a worktree thread.", }); } - // A branch no worktree has checked out can still carry commits nothing else holds, and - // materializing would force it onto the head. Check it out as it is, and let the reused - // worktree refresh move it only where that loses nothing. + // Materializing would force this branch onto the head. if (existingBranchBeforeFetch !== null) { - const worktree = yield* createPullRequestWorktree(); - return yield* reuseExistingWorktree( - worktree.worktree.path, - existingBranchBeforeFetch.name, - true, + const isOnPullRequestHead = yield* moveIdleBranchToPullRequestHead( + localPullRequestBranch, + ).pipe( + Effect.catch((error) => + Effect.logWarning("GitManager.preparePullRequestThread existing branch move failed", { + localBranch: localPullRequestBranch, + cause: error, + }).pipe(Effect.as(false)), + ), ); + const worktree = yield* createPullRequestWorktree(); + yield* maybeRunSetupScript(worktree.worktree.path); + return { + pullRequest, + branch: worktree.worktree.refName, + worktreePath: worktree.worktree.path, + isOnPullRequestHead, + }; } yield* materializePullRequestHeadBranch( diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 6c1c8803d5639..0599b0caf24bd 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -357,11 +357,6 @@ export class GitVcsDriver extends Context.Service< input: VcsCreateWorktreeInput, options?: CreateWorktreeOptions, ) => Effect.Effect; - /** Checks out the submodules of the worktree's current revision, as `createWorktree` does. */ - readonly updateWorktreeSubmodules: ( - worktreePath: string, - options?: CreateWorktreeOptions, - ) => Effect.Effect; /** Force-updates local `branch` to the primary remote's `ref`. */ readonly fetchRefIntoBranch: ( input: GitFetchRefInput & { readonly branch: string }, diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index f83719217bfa5..5a9aa74af60aa 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -3781,50 +3781,6 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* progress.onWorktreeClaimed(worktreePath); } - yield* updateWorktreeSubmodules(worktreePath, options); - - const newRefName = input.newRefName; - if (newRefName && input.baseRefName) { - const base = yield* resolveBranchBase(input.cwd, input.baseRefName); - if (base) { - yield* Effect.gen(function* () { - yield* runGit("GitVcsDriver.createWorktree.configureBaseRef", input.cwd, [ - "config", - branchBaseConfigKey(newRefName), - base.branchName, - ]); - if (base.baseRef.startsWith("refs/remotes/")) { - // The base key names a branch on the host; this one keeps the remote it came from. - yield* runGit("GitVcsDriver.createWorktree.configureComparisonRef", input.cwd, [ - "config", - `branch.${input.newRefName}.t3-merge-base-ref`, - base.baseRef, - ]); - } - }).pipe( - Effect.catchTags({ - GitCommandError: () => - Effect.logWarning( - "worktree base-ref configuration failed; using base-branch fallback", - ), - }), - ); - } - } - - return { - worktree: { - path: worktreePath, - refName: targetBranch, - }, - }; - }); - - const updateWorktreeSubmodules = Effect.fn("updateWorktreeSubmodules")(function* ( - worktreePath: string, - options?: GitVcsDriver.CreateWorktreeOptions, - ) { - const progress = options?.progress; // `git worktree add` leaves submodules empty, so a repo that keeps agent // skills, tooling or source in one gets a worktree that is quietly missing // them. @@ -3892,6 +3848,42 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* }), ); } + + const newRefName = input.newRefName; + if (newRefName && input.baseRefName) { + const base = yield* resolveBranchBase(input.cwd, input.baseRefName); + if (base) { + yield* Effect.gen(function* () { + yield* runGit("GitVcsDriver.createWorktree.configureBaseRef", input.cwd, [ + "config", + branchBaseConfigKey(newRefName), + base.branchName, + ]); + if (base.baseRef.startsWith("refs/remotes/")) { + // The base key names a branch on the host; this one keeps the remote it came from. + yield* runGit("GitVcsDriver.createWorktree.configureComparisonRef", input.cwd, [ + "config", + `branch.${input.newRefName}.t3-merge-base-ref`, + base.baseRef, + ]); + } + }).pipe( + Effect.catchTags({ + GitCommandError: () => + Effect.logWarning( + "worktree base-ref configuration failed; using base-branch fallback", + ), + }), + ); + } + } + + return { + worktree: { + path: worktreePath, + refName: targetBranch, + }, + }; }); const fetchRefIntoBranch: GitVcsDriver.GitVcsDriver["Service"]["fetchRefIntoBranch"] = Effect.fn( @@ -4530,7 +4522,6 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* listRefs, createWorktree: (input, options) => withListRefsInvalidation(input.cwd, createWorktree(input, options)), - updateWorktreeSubmodules, fetchRefIntoBranch: (input) => withListRefsInvalidation(input.cwd, fetchRefIntoBranch(input)), fetchRefCommit, resolveCommit, From 4e2c7ecaa9b07e328167c68517bd3bdf44e3a044 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Mon, 26 Oct 2026 15:12:18 +0000 Subject: [PATCH 4/6] fix(server): claim PR worktrees before refreshing existing branches --- apps/server/src/git/GitManager.test.ts | 38 ++++++- apps/server/src/git/GitManager.ts | 150 +++++++++++-------------- 2 files changed, 100 insertions(+), 88 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 60e561e26b1d8..e4c83bbe283db 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -4336,6 +4336,9 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { { localBranch: "ahead", expectedHead: "local", isOnPullRequestHead: false }, { localBranch: "behind", expectedHead: "pull request", isOnPullRequestHead: true }, { localBranch: "rewritten", expectedHead: "pull request", isOnPullRequestHead: true }, + { localBranch: "wrong-ref", expectedHead: "pull request", isOnPullRequestHead: true }, + { localBranch: "stale-head", expectedHead: "local", isOnPullRequestHead: false }, + { localBranch: "checkout-race", expectedHead: "pull request", isOnPullRequestHead: true }, ] as const)( "moves an existing PR branch into a new worktree only when nothing is lost ($localBranch)", ({ localBranch, expectedHead, isOnPullRequestHead }) => @@ -4354,7 +4357,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { NodeFS.writeFileSync(NodePath.join(repoDir, "mine.txt"), "mine\n"); yield* runGit(repoDir, ["add", "mine.txt"]); yield* runGit(repoDir, ["commit", "-m", "My unpublished commit"]); - } else if (localBranch === "behind") { + } else if (localBranch !== "rewritten") { yield* runGit(repoDir, ["checkout", "-b", "author-push"]); NodeFS.writeFileSync(NodePath.join(repoDir, "authored.txt"), "authored\n"); yield* runGit(repoDir, ["add", "authored.txt"]); @@ -4389,7 +4392,18 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { ])).stdout.split("\t")[0]; yield* runGit(repoDir, ["checkout", "main"]); + if (localBranch === "wrong-ref") { + yield* runGit(repoDir, ["push", "origin", "main:refs/pull/88/head"]); + } + const gate = { + matches: (args: ReadonlyArray) => + args[0] === "fetch" && + args.some((arg) => arg.startsWith("+refs/heads/feature/pr-existing:")), + reached: yield* Deferred.make(), + release: yield* Deferred.make(), + }; const { manager } = yield* makeManager({ + ...(localBranch === "checkout-race" ? { gitSpawnGate: gate } : {}), providerScenario: { pullRequest: { number: 88, @@ -4398,6 +4412,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { baseRefName: "main", headRefName: "feature/pr-existing", state: "open", + headSha: localBranch === "stale-head" ? localHead : pullHead, isCrossRepository: false, headRepositoryNameWithOwner: "pingdotgg/codething-mvp", headRepositoryOwnerLogin: "pingdotgg", @@ -4408,11 +4423,21 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }, }); - const result = yield* preparePullRequestThread(manager, { - cwd: repoDir, - reference: "88", - mode: "worktree", - }); + const preparing = yield* Effect.forkChild( + preparePullRequestThread(manager, { + cwd: repoDir, + reference: "88", + mode: "worktree", + }), + ); + if (localBranch === "checkout-race") { + yield* Deferred.await(gate.reached); + const checkout = yield* runGit(repoDir, ["checkout", "feature/pr-existing"], true); + yield* Deferred.succeed(gate.release, undefined); + expect(checkout.exitCode).not.toBe(0); + expect((yield* runGit(repoDir, ["branch", "--show-current"])).stdout.trim()).toBe("main"); + } + const result = yield* Fiber.join(preparing); const head = expectedHead === "local" ? localHead : pullHead; expect((yield* runGit(repoDir, ["rev-parse", "feature/pr-existing"])).stdout.trim()).toBe( @@ -4473,6 +4498,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { pullRequest: { number: 89, title: "Submodule PR", + headSha: pullHead, url: "https://github.com/pingdotgg/codething-mvp/pull/89", baseRefName: "main", headRefName: "feature/pr-existing", diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 507aa3ebd2b74..8ad8502fc880e 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -2902,82 +2902,58 @@ export const make = Effect.gen(function* () { return null; }); - // A branch no worktree has checked out can still carry commits nothing else holds, so it - // takes the head only where refreshCheckedOutBranch would move a checkout. It moves before - // its worktree exists because `git worktree add` checks out submodules for the commit it - // starts at, and a later move would leave them at the old commit's. - const moveIdleBranchToPullRequestHead = Effect.fn("moveIdleBranchToPullRequestHead")( - function* (branch: string) { - const { commitSha: branchCommit } = yield* gitCore.resolveCommit({ - cwd: input.cwd, - revision: `refs/heads/${branch}`, - }); - // Read before the upstream fetch, for the same reason as in reuseExistingWorktree. - const upstreamCommitBeforeFetch = yield* gitCore - .resolveCommit({ cwd: input.cwd, revision: `${branch}@{upstream}` }) - .pipe( - Effect.map((resolved) => resolved.commitSha), - Effect.orElseSucceed(() => null), - ); - - yield* configurePullRequestHeadUpstream(input.cwd, pullRequestWithRemoteInfo, branch); - - const { commitSha: targetCommit } = yield* changeRequestHeadRef( - input.cwd, - pullRequest.number, - ).pipe( - Effect.flatMap((ref) => gitCore.fetchRefCommit({ cwd: input.cwd, ref })), - Effect.catch(() => - Effect.gen(function* () { - const merge = yield* gitCore.readConfigValue(input.cwd, `branch.${branch}.merge`); - if (merge !== `refs/heads/${pullRequest.headBranch}`) { - return yield* new GitManagerError({ - operation: "preparePullRequestThread", - cwd: input.cwd, - detail: "The pull request head could not be resolved for this branch.", - }); - } - return yield* gitCore.resolveCommit({ - cwd: input.cwd, - revision: `${branch}@{upstream}`, - }); + // Claim the branch before refreshing, but initialize submodules only after selecting its head. + const refreshNewWorktree = Effect.fn("refreshNewWorktree")(function* (cwd: string) { + const upstreamCommitBeforeFetch = yield* gitCore + .resolveCommit({ cwd, revision: "@{upstream}" }) + .pipe( + Effect.map((resolved) => resolved.commitSha), + Effect.orElseSucceed(() => null), + ); + yield* configurePullRequestHeadUpstream( + cwd, + pullRequestWithRemoteInfo, + localPullRequestBranch, + ); + const target = yield* changeRequestHeadRef(cwd, pullRequest.number).pipe( + Effect.flatMap((ref) => gitCore.fetchRefCommit({ cwd, ref })), + Effect.filterOrFail( + (resolved) => + pullRequestSummary.headSha === undefined || + resolved.commitSha === pullRequestSummary.headSha, + () => + new GitManagerError({ + operation: "preparePullRequestThread", + cwd, + detail: "The fetched ref does not match the pull request head.", }), + ), + Effect.catch(() => + gitCore.resolveCommit({ cwd, revision: "@{upstream}" }).pipe( + Effect.filterOrFail( + (resolved) => resolved.commitSha === pullRequestSummary.headSha, + () => + new GitManagerError({ + operation: "preparePullRequestThread", + cwd, + detail: "The configured upstream does not match the pull request head.", + }), + ), ), - ); - if (targetCommit === branchCommit) { - return true; - } - - const isAncestor = yield* gitCore - .execute({ - operation: "GitManager.preparePullRequestThread.isAncestor", - cwd: input.cwd, - args: ["merge-base", "--is-ancestor", branchCommit, targetCommit], - allowNonZeroExit: true, - }) - .pipe(Effect.map((result) => result.exitCode === 0)); - if (!isAncestor && branchCommit !== upstreamCommitBeforeFetch) { - return false; - } - if (!isAncestor) { - // As refreshCheckedOutBranch does, so the rewritten-away commit stays findable. - yield* gitCore.execute({ - operation: "GitManager.preparePullRequestThread.keepPrevious", - cwd: input.cwd, - args: ["update-ref", "refs/t3code/pre-refresh", branchCommit], - }); - } - // The old value makes git refuse the move if the branch changed since it was read. - yield* gitCore.execute({ - operation: "GitManager.preparePullRequestThread.moveBranch", - cwd: input.cwd, - args: ["update-ref", `refs/heads/${branch}`, targetCommit, branchCommit], - }); - return true; - }, - ); + ), + ); + return yield* gitCore + .refreshCheckedOutBranch({ + cwd, + targetCommit: target.commitSha, + resetWhenHeadCommit: upstreamCommitBeforeFetch, + }) + .pipe(Effect.map((result) => result.onTarget)); + }); - const createPullRequestWorktree = Effect.fn("createPullRequestWorktree")(function* () { + const createPullRequestWorktree = Effect.fn("createPullRequestWorktree")(function* ( + onWorktreeClaimed?: (path: string) => Effect.Effect, + ) { return yield* gitCore.createWorktree( { cwd: input.cwd, @@ -2985,6 +2961,7 @@ export const make = Effect.gen(function* () { path: null, }, { + ...(onWorktreeClaimed ? { progress: { onWorktreeClaimed } } : {}), worktreesDirectory: yield* readWorktreesDirectory, submodules: yield* projectSettingsFor(input).pipe( Effect.map(({ settings }) => settings.worktreeSubmodules), @@ -3017,17 +2994,26 @@ export const make = Effect.gen(function* () { } // Materializing would force this branch onto the head. if (existingBranchBeforeFetch !== null) { - const isOnPullRequestHead = yield* moveIdleBranchToPullRequestHead( - localPullRequestBranch, - ).pipe( - Effect.catch((error) => - Effect.logWarning("GitManager.preparePullRequestThread existing branch move failed", { - localBranch: localPullRequestBranch, - cause: error, - }).pipe(Effect.as(false)), + let isOnPullRequestHead = false; + const worktree = yield* createPullRequestWorktree((cwd) => + refreshNewWorktree(cwd).pipe( + Effect.catch((error) => + Effect.logWarning( + "GitManager.preparePullRequestThread existing branch refresh failed", + { + localBranch: localPullRequestBranch, + cause: error, + }, + ).pipe(Effect.as(false)), + ), + Effect.tap((onTarget) => + Effect.sync(() => { + isOnPullRequestHead = onTarget; + }), + ), + Effect.asVoid, ), ); - const worktree = yield* createPullRequestWorktree(); yield* maybeRunSetupScript(worktree.worktree.path); return { pullRequest, From 9a36d311455412c99c3fb958f8b5a2ff5b68dc91 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Tue, 10 Nov 2026 05:19:40 +0000 Subject: [PATCH 5/6] test(server): pass git file protocol through HostProcess.Environment --- apps/server/src/git/GitManager.test.ts | 29 +++++++++++++------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 113a2cfeb7412..1f2a52401f1d4 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -21,6 +21,7 @@ import * as Result from "effect/Result"; import * as Scope from "effect/Scope"; import * as Stream from "effect/Stream"; import * as TestClock from "effect/testing/TestClock"; +import * as HostProcess from "@t3tools/shared/HostProcess"; import { ChildProcessSpawner } from "effect/process"; import { expect } from "vite-plus/test"; import type { @@ -5233,20 +5234,6 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { "checks out the submodules of the head an existing PR branch moves onto (head %s one)", (headChange) => Effect.gen(function* () { - // Git refuses `file:` submodule transports by default (CVE-2022-39253) and ignores - // repo-level config for it, so a local fixture needs the env allowance. - const previousAllowedProtocol = process.env.GIT_ALLOW_PROTOCOL; - process.env.GIT_ALLOW_PROTOCOL = "file"; - yield* Effect.addFinalizer(() => - Effect.sync(() => { - if (previousAllowedProtocol === undefined) { - delete process.env.GIT_ALLOW_PROTOCOL; - } else { - process.env.GIT_ALLOW_PROTOCOL = previousAllowedProtocol; - } - }), - ); - const submoduleDir = yield* makeTempDir("t3code-git-submodule-"); yield* initRepo(submoduleDir); const repoDir = yield* makeTempDir("t3code-git-manager-"); @@ -5305,7 +5292,19 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { ); expect((yield* runGit(worktreePath, ["status", "--porcelain"])).stdout).toBe(""); expect(result).toMatchObject({ isOnPullRequestHead: true }); - }), + }).pipe( + // Git refuses `file:` submodule transports by default (CVE-2022-39253) and ignores + // repo-level config for it, so a local fixture needs the env allowance. + (effect) => + Effect.flatMap(HostProcess.Environment, (env) => + effect.pipe( + Effect.provideService(HostProcess.Environment, { + ...env, + GIT_ALLOW_PROTOCOL: "file", + }), + ), + ), + ), ); it.effect("refreshes a reused PR worktree onto the updated pull request head", () => From 20acbb718fc2eedc7d6282ea5c76e64dd41a2d7c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 12 Nov 2026 22:12:01 +0000 Subject: [PATCH 6/6] fix(server): reset a published PR branch even after its upstream was fetched An idle local PR branch at an old, published head stayed behind when a fetch had already moved its remote-tracking ref to the rewritten head. Treat a head reachable from a remote-tracking ref, or in the upstream's reflog, as safe to reset. AI-assisted; verified with the tests and tools listed above. --- apps/server/src/git/GitManager.test.ts | 15 +++++++++------ apps/server/src/git/GitManager.ts | 24 +++++++++++++++++++++++- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index eaf5458b0d5dd..a0e6cf22ac67c 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -5391,6 +5391,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { { localBranch: "ahead", expectedHead: "local", isOnPullRequestHead: false }, { localBranch: "behind", expectedHead: "pull request", isOnPullRequestHead: true }, { localBranch: "rewritten", expectedHead: "pull request", isOnPullRequestHead: true }, + { localBranch: "rewritten-fetched", expectedHead: "pull request", isOnPullRequestHead: true }, { localBranch: "wrong-ref", expectedHead: "pull request", isOnPullRequestHead: true }, { localBranch: "stale-head", expectedHead: "local", isOnPullRequestHead: false }, { localBranch: "checkout-race", expectedHead: "pull request", isOnPullRequestHead: true }, @@ -5412,7 +5413,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { NodeFS.writeFileSync(NodePath.join(repoDir, "mine.txt"), "mine\n"); yield* runGit(repoDir, ["add", "mine.txt"]); yield* runGit(repoDir, ["commit", "-m", "My unpublished commit"]); - } else if (localBranch !== "rewritten") { + } else if (!localBranch.startsWith("rewritten")) { yield* runGit(repoDir, ["checkout", "-b", "author-push"]); NodeFS.writeFileSync(NodePath.join(repoDir, "authored.txt"), "authored\n"); yield* runGit(repoDir, ["add", "authored.txt"]); @@ -5430,11 +5431,13 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { "origin", "author-rewrite:feature/pr-existing", ]); - yield* runGit(repoDir, [ - "update-ref", - "refs/remotes/origin/feature/pr-existing", - staleHead, - ]); + if (localBranch === "rewritten") { + yield* runGit(repoDir, [ + "update-ref", + "refs/remotes/origin/feature/pr-existing", + staleHead, + ]); + } } const localHead = (yield* runGit(repoDir, [ "rev-parse", diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index f8f57fd68d10f..11b959ca3f422 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -3134,11 +3134,33 @@ export const make = Effect.gen(function* () { ), ), ); + const { commitSha: headCommit } = yield* gitCore.resolveCommit({ cwd, revision: "HEAD" }); + // A head that was ever published loses nothing when reset, even if a fetch already moved its upstream. + const gitLines = (args: ReadonlyArray) => + gitCore + .execute({ + operation: "GitManager.preparePullRequestThread.publishedHead", + cwd, + args, + allowNonZeroExit: true, + timeoutMs: 5_000, + }) + .pipe( + Effect.map((result) => + result.exitCode === 0 ? result.stdout.split("\n").filter(Boolean) : [], + ), + ); + const publishedHead = + (yield* gitLines(["for-each-ref", "--contains", headCommit, "refs/remotes"])).length > + 0 || + (yield* gitLines(["log", "--walk-reflogs", "--format=%H", "@{upstream}"])).includes( + headCommit, + ); return yield* gitCore .refreshCheckedOutBranch({ cwd, targetCommit: target.commitSha, - resetWhenHeadCommit: upstreamCommitBeforeFetch, + resetWhenHeadCommit: publishedHead ? headCommit : upstreamCommitBeforeFetch, }) .pipe(Effect.map((result) => result.onTarget)); });