From 53f77282cc998cd84d5670cf5aaa849b6908b2c8 Mon Sep 17 00:00:00 2001 From: Soorya U Date: Wed, 5 Aug 2026 22:40:00 +0530 Subject: [PATCH 1/2] Fix libgit2 conflict when a branch is already checked out elsewhere (#144) createGitWorktree now forks a fresh branch (named via generateName()) off the requested ref instead of checking out that ref directly, so a new worktree never collides with a branch already checked out in the main repo or another linked worktree. checkoutGitRef keeps the literal in-place checkout but now pre-checks via findBranchCheckedOutElsewhere and returns a branchAlreadyCheckedOutError instead of a raw libgit2 error, while treating "already on that branch here" as a no-op. Co-Authored-By: Claude Sonnet 5 --- apps/cli/src/git/checked-out.ts | 40 ++++++++++++++++++ apps/cli/src/git/checkout.ts | 16 +++++++- apps/cli/src/git/git.test.ts | 72 ++++++++++++++++++++++++++++++++- apps/cli/src/git/worktree.ts | 50 ++++++++++++++++++++--- shared/errors/src/git.ts | 28 ++++++++++++- 5 files changed, 197 insertions(+), 9 deletions(-) create mode 100644 apps/cli/src/git/checked-out.ts diff --git a/apps/cli/src/git/checked-out.ts b/apps/cli/src/git/checked-out.ts new file mode 100644 index 00000000..c42c000f --- /dev/null +++ b/apps/cli/src/git/checked-out.ts @@ -0,0 +1,40 @@ +import { resolve } from "node:path"; +import { Result } from "better-result"; +import { openRepositoryFromWorktree, type Repository } from "es-git"; + +function currentBranchRefName(repo: Repository): string | null { + return repo.findReference("HEAD")?.symbolicTarget() ?? null; +} + +export function findBranchCheckedOutElsewhere( + repo: Repository, + branch: string, + excludePath: string +): string | null { + const target = `refs/heads/${branch}`; + const exclude = resolve(excludePath); + + const workdir = repo.workdir(); + if ( + workdir && + resolve(workdir) !== exclude && + currentBranchRefName(repo) === target + ) { + return workdir; + } + + for (const name of repo.worktrees()) { + const worktree = repo.findWorktree(name); + const worktreePath = worktree.path(); + if (resolve(worktreePath) === exclude) continue; + + const worktreeRepo = Result.try(() => openRepositoryFromWorktree(worktree)); + if (worktreeRepo.isErr()) continue; + + if (currentBranchRefName(worktreeRepo.value) === target) { + return worktreePath; + } + } + + return null; +} diff --git a/apps/cli/src/git/checkout.ts b/apps/cli/src/git/checkout.ts index 02e5ca84..d55ba03e 100644 --- a/apps/cli/src/git/checkout.ts +++ b/apps/cli/src/git/checkout.ts @@ -1,5 +1,10 @@ -import { type GitError, GitNotRepositoryError } from "@cyrus/errors/git"; +import { + branchAlreadyCheckedOutError, + type GitError, + GitNotRepositoryError, +} from "@cyrus/errors/git"; import { Result } from "better-result"; +import { findBranchCheckedOutElsewhere } from "./checked-out"; import { openGitRepository, operationFailedFromUnknown } from "./open"; export async function checkoutGitRef( @@ -9,6 +14,15 @@ export async function checkoutGitRef( const opened = await openGitRepository(cwd); if (opened.isErr()) return Result.err(opened.error); + const conflictPath = findBranchCheckedOutElsewhere( + opened.value, + refName, + cwd + ); + if (conflictPath) { + return Result.err(branchAlreadyCheckedOutError(refName, conflictPath)); + } + const checkout = Result.try(() => { opened.value.setHead(`refs/heads/${refName}`); opened.value.checkoutHead(); diff --git a/apps/cli/src/git/git.test.ts b/apps/cli/src/git/git.test.ts index f45efcf4..3a569bbb 100644 --- a/apps/cli/src/git/git.test.ts +++ b/apps/cli/src/git/git.test.ts @@ -2,10 +2,13 @@ import { describe, expect, test } from "bun:test"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { initRepository } from "es-git"; +import { GitBranchCheckedOutError } from "@cyrus/errors/git"; +import { initRepository, openRepository } from "es-git"; +import { checkoutGitRef } from "@/git/checkout"; import { getGitPatch } from "@/git/patch"; import { defaultWorktreePath, sanitizeBranchDirName } from "@/git/paths"; import { getGitStatus } from "@/git/status"; +import { createGitWorktree } from "@/git/worktree"; async function initRepo(dir: string) { const repo = await initRepository(dir, { initialHead: "main" }); @@ -63,3 +66,70 @@ describe("git status", () => { } }); }); + +describe("createGitWorktree", () => { + test("forks a new branch instead of the checked-out source branch", async () => { + const dir = await mkdtemp(join(tmpdir(), "cyrus-git-")); + try { + await initRepo(dir); + const result = await createGitWorktree(dir, "main", "wt-main"); + expect(result.isOk()).toBe(true); + if (!result.isOk()) return; + + const worktreeRepo = await openRepository(result.value); + expect(worktreeRepo.head().shorthand()).not.toBe("main"); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("succeeds even when the source branch is checked out in the main repo", async () => { + const dir = await mkdtemp(join(tmpdir(), "cyrus-git-")); + try { + await initRepo(dir); + const first = await createGitWorktree(dir, "main", "wt-a"); + const second = await createGitWorktree(dir, "main", "wt-b"); + expect(first.isOk()).toBe(true); + expect(second.isOk()).toBe(true); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); + +describe("checkoutGitRef", () => { + test("succeeds as a no-op when already on the requested branch", async () => { + const dir = await mkdtemp(join(tmpdir(), "cyrus-git-")); + try { + await initRepo(dir); + const result = await checkoutGitRef(dir, "main"); + expect(result.isOk()).toBe(true); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("returns branchAlreadyCheckedOutError when the branch is checked out in another worktree", async () => { + const dir = await mkdtemp(join(tmpdir(), "cyrus-git-")); + try { + await initRepo(dir); + const repo = await openRepository(dir); + const head = repo.head(); + const oid = head.target(); + if (!oid) throw new Error("expected HEAD to resolve to a commit"); + const commit = repo.getCommit(oid); + repo.createBranch("feature", commit); + repo.worktree("feature", join(dir, "wt-feature"), { + refName: "refs/heads/feature", + checkoutExisting: true, + }); + + const result = await checkoutGitRef(dir, "feature"); + expect(result.isErr()).toBe(true); + if (!result.isErr()) return; + expect(GitBranchCheckedOutError.is(result.error)).toBe(true); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/apps/cli/src/git/worktree.ts b/apps/cli/src/git/worktree.ts index dae61f93..420b41b1 100644 --- a/apps/cli/src/git/worktree.ts +++ b/apps/cli/src/git/worktree.ts @@ -1,8 +1,9 @@ import { mkdir } from "node:fs/promises"; import { dirname, resolve } from "node:path"; import { type GitError, operationFailedError } from "@cyrus/errors/git"; +import { generateName } from "@cyrus/utils/identity"; import { Result } from "better-result"; -import type { Repository, Worktree } from "es-git"; +import type { Commit, Repository, Worktree } from "es-git"; import { openGitRepository, operationFailedFromUnknown, @@ -14,6 +15,8 @@ import { worktreeNameForBranch, } from "./paths"; +const MAX_BRANCH_NAME_ATTEMPTS = 5; + function findWorktreeByPath( repo: Repository, worktreePath: string @@ -26,6 +29,26 @@ function findWorktreeByPath( return null; } +function resolveBranchCommit( + repo: Repository, + refName: string +): Result { + return Result.try(() => { + const branch = repo.getBranch(refName, "Local"); + const oid = branch.referenceTarget(); + if (!oid) throw new Error(`Branch '${refName}' has no commits yet`); + return repo.getCommit(oid); + }).mapError(operationFailedFromUnknown); +} + +function generateUniqueBranchName(repo: Repository): string { + for (let attempt = 0; attempt < MAX_BRANCH_NAME_ATTEMPTS; attempt++) { + const candidate = generateName(); + if (!repo.findBranch(candidate, "Local")) return candidate; + } + return `${generateName()}-${crypto.randomUUID().slice(0, 8)}`; +} + export async function createGitWorktree( projectCwd: string, refName: string, @@ -39,23 +62,38 @@ export async function createGitWorktree( : Result.ok(undefined); if (resolvedPath.isErr()) return Result.err(resolvedPath.error); + const commit = resolveBranchCommit(opened.value, refName); + if (commit.isErr()) return Result.err(commit.error); + + const branchName = generateUniqueBranchName(opened.value); + const branch = Result.try(() => + opened.value.createBranch(branchName, commit.value) + ); + if (branch.isErr()) + return Result.err(operationFailedFromUnknown(branch.error)); + const worktreePath = - resolvedPath.value ?? defaultWorktreePath(projectCwd, refName); + resolvedPath.value ?? defaultWorktreePath(projectCwd, branchName); const prepared = await runGitOperationAsync(() => mkdir(dirname(worktreePath), { recursive: true }) ); - if (prepared.isErr()) return Result.err(prepared.error); + if (prepared.isErr()) { + Result.try(() => branch.value.delete()); + return Result.err(prepared.error); + } const created = Result.try(() => { - const name = worktreeNameForBranch(refName); + const name = worktreeNameForBranch(branchName); opened.value.worktree(name, worktreePath, { - refName: `refs/heads/${refName}`, + refName: `refs/heads/${branchName}`, checkoutExisting: true, }); return worktreePath; }); - if (created.isErr()) + if (created.isErr()) { + Result.try(() => branch.value.delete()); return Result.err(operationFailedFromUnknown(created.error)); + } return Result.ok(created.value); } diff --git a/shared/errors/src/git.ts b/shared/errors/src/git.ts index 1816c37f..76eb9b90 100644 --- a/shared/errors/src/git.ts +++ b/shared/errors/src/git.ts @@ -5,6 +5,7 @@ import type { EmptyPayload } from "./orpc"; const tags = { notRepository: errorTag(errorModules.git, "not_repository"), operationFailed: errorTag(errorModules.git, "operation_failed"), + branchCheckedOut: errorTag(errorModules.git, "branch_checked_out"), } as const; export class GitNotRepositoryError extends TaggedError( @@ -28,7 +29,25 @@ export class GitOperationFailedError extends TaggedError(tags.operationFailed)<{ } } -export type GitError = GitNotRepositoryError | GitOperationFailedError; +export class GitBranchCheckedOutError extends TaggedError( + tags.branchCheckedOut +)<{ + branch: string; + path: string; +}>() { + get message() { + return `Branch '${this.branch}' is already checked out at ${this.path}`; + } + + get orpcCode() { + return "BAD_REQUEST" as const; + } +} + +export type GitError = + | GitNotRepositoryError + | GitOperationFailedError + | GitBranchCheckedOutError; export function notRepositoryError(): GitError { return new GitNotRepositoryError({}); @@ -40,3 +59,10 @@ export function operationFailedError( ): GitError { return new GitOperationFailedError({ message, detail }); } + +export function branchAlreadyCheckedOutError( + branch: string, + path: string +): GitError { + return new GitBranchCheckedOutError({ branch, path }); +} From ae7e070bd974016b627f7d5f6dc9ff1cb5947ff4 Mon Sep 17 00:00:00 2001 From: Soorya U Date: Thu, 6 Aug 2026 12:05:17 +0530 Subject: [PATCH 2/2] Inline branch-checked-out-elsewhere check into checkout.ts findBranchCheckedOutElsewhere only had one caller (checkoutGitRef), so fold it and its helper back into checkout.ts instead of keeping a separate single-use module. Co-Authored-By: Claude Sonnet 5 --- apps/cli/src/git/checked-out.ts | 40 ---------------------------- apps/cli/src/git/checkout.ts | 46 +++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 45 deletions(-) delete mode 100644 apps/cli/src/git/checked-out.ts diff --git a/apps/cli/src/git/checked-out.ts b/apps/cli/src/git/checked-out.ts deleted file mode 100644 index c42c000f..00000000 --- a/apps/cli/src/git/checked-out.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { resolve } from "node:path"; -import { Result } from "better-result"; -import { openRepositoryFromWorktree, type Repository } from "es-git"; - -function currentBranchRefName(repo: Repository): string | null { - return repo.findReference("HEAD")?.symbolicTarget() ?? null; -} - -export function findBranchCheckedOutElsewhere( - repo: Repository, - branch: string, - excludePath: string -): string | null { - const target = `refs/heads/${branch}`; - const exclude = resolve(excludePath); - - const workdir = repo.workdir(); - if ( - workdir && - resolve(workdir) !== exclude && - currentBranchRefName(repo) === target - ) { - return workdir; - } - - for (const name of repo.worktrees()) { - const worktree = repo.findWorktree(name); - const worktreePath = worktree.path(); - if (resolve(worktreePath) === exclude) continue; - - const worktreeRepo = Result.try(() => openRepositoryFromWorktree(worktree)); - if (worktreeRepo.isErr()) continue; - - if (currentBranchRefName(worktreeRepo.value) === target) { - return worktreePath; - } - } - - return null; -} diff --git a/apps/cli/src/git/checkout.ts b/apps/cli/src/git/checkout.ts index d55ba03e..d1d8376c 100644 --- a/apps/cli/src/git/checkout.ts +++ b/apps/cli/src/git/checkout.ts @@ -1,12 +1,49 @@ +import { resolve } from "node:path"; import { branchAlreadyCheckedOutError, type GitError, GitNotRepositoryError, } from "@cyrus/errors/git"; import { Result } from "better-result"; -import { findBranchCheckedOutElsewhere } from "./checked-out"; +import { openRepositoryFromWorktree, type Repository } from "es-git"; import { openGitRepository, operationFailedFromUnknown } from "./open"; +function currentBranchRefName(repo: Repository): string | null { + return repo.findReference("HEAD")?.symbolicTarget() ?? null; +} + +function findBranchCheckedOutElsewhere( + repo: Repository, + branch: string, + excludePath: string +): string | null { + const target = `refs/heads/${branch}`; + const exclude = resolve(excludePath); + + const workdir = repo.workdir(); + if ( + workdir && + resolve(workdir) !== exclude && + currentBranchRefName(repo) === target + ) { + return workdir; + } + + for (const name of repo.worktrees()) { + const worktree = repo.findWorktree(name); + const worktreePath = worktree.path(); + if (resolve(worktreePath) === exclude) continue; + + const worktreeRepo = Result.try(() => openRepositoryFromWorktree(worktree)); + if (worktreeRepo.isErr()) continue; + + if (currentBranchRefName(worktreeRepo.value) === target) + return worktreePath; + } + + return null; +} + export async function checkoutGitRef( cwd: string, refName: string @@ -19,17 +56,16 @@ export async function checkoutGitRef( refName, cwd ); - if (conflictPath) { + if (conflictPath) return Result.err(branchAlreadyCheckedOutError(refName, conflictPath)); - } const checkout = Result.try(() => { opened.value.setHead(`refs/heads/${refName}`); opened.value.checkoutHead(); }); - if (checkout.isErr()) { + if (checkout.isErr()) return Result.err(operationFailedFromUnknown(checkout.error)); - } + return Result.ok(undefined); }