diff --git a/apps/server/src/git/Layers/GitCore.test.ts b/apps/server/src/git/Layers/GitCore.test.ts index 8845808ac1c33ba3879f5d9975dbe81cf28f966e..2325f8e5f5d5bf7c2013e4f49606fe49601c32ae 100644 --- a/apps/server/src/git/Layers/GitCore.test.ts +++ b/apps/server/src/git/Layers/GitCore.test.ts @@ -414,6 +414,38 @@ it.layer(TestLayer)("git integration", (it) => { ).toBe(true); }), ); + + it.effect("includes remoteName metadata for remotes with slash in the name", () => + Effect.gen(function* () { + const remote = yield* makeTmpDir(); + const tmp = yield* makeTmpDir(); + const remoteName = "my-org/upstream"; + + yield* git(remote, ["init", "--bare"]); + yield* initRepoWithCommit(tmp); + const defaultBranch = (yield* listGitBranches({ cwd: tmp })).branches.find( + (branch) => branch.current, + )!.name; + + yield* git(tmp, ["remote", "add", remoteName, remote]); + yield* git(tmp, ["push", "-u", remoteName, defaultBranch]); + + const remoteOnlyBranch = "feature/remote-with-remote-name"; + yield* git(tmp, ["checkout", "-b", remoteOnlyBranch]); + yield* git(tmp, ["push", "-u", remoteName, remoteOnlyBranch]); + yield* git(tmp, ["checkout", defaultBranch]); + yield* git(tmp, ["branch", "-D", remoteOnlyBranch]); + + const result = yield* listGitBranches({ cwd: tmp }); + const remoteBranch = result.branches.find( + (branch) => branch.name === `${remoteName}/${remoteOnlyBranch}`, + ); + + expect(remoteBranch).toBeDefined(); + expect(remoteBranch?.isRemote).toBe(true); + expect(remoteBranch?.remoteName).toBe(remoteName); + }), + ); }); // ── checkoutGitBranch ── @@ -636,6 +668,60 @@ it.layer(TestLayer)("git integration", (it) => { }), ); + it.effect( + "does not silently checkout a local branch when a remote ref no longer exists", + () => + Effect.gen(function* () { + const remote = yield* makeTmpDir(); + const source = yield* makeTmpDir(); + yield* git(remote, ["init", "--bare"]); + + yield* initRepoWithCommit(source); + const defaultBranch = (yield* listGitBranches({ cwd: source })).branches.find( + (branch) => branch.current, + )!.name; + yield* git(source, ["remote", "add", "origin", remote]); + yield* git(source, ["push", "-u", "origin", defaultBranch]); + + yield* createGitBranch({ cwd: source, branch: "feature" }); + + const checkoutResult = yield* Effect.result( + checkoutGitBranch({ cwd: source, branch: "origin/feature" }), + ); + expect(checkoutResult._tag).toBe("Failure"); + expect(yield* git(source, ["branch", "--show-current"])).toBe(defaultBranch); + }), + ); + + it.effect("checks out a remote tracking branch when remote name contains slashes", () => + Effect.gen(function* () { + const remote = yield* makeTmpDir(); + const source = yield* makeTmpDir(); + const remoteName = "my-org/upstream"; + const featureBranch = "feature"; + yield* git(remote, ["init", "--bare"]); + + yield* initRepoWithCommit(source); + const defaultBranch = (yield* listGitBranches({ cwd: source })).branches.find( + (branch) => branch.current, + )!.name; + yield* git(source, ["remote", "add", remoteName, remote]); + yield* git(source, ["push", "-u", remoteName, defaultBranch]); + + yield* git(source, ["checkout", "-b", featureBranch]); + yield* writeTextFile(path.join(source, "feature.txt"), "feature content\n"); + yield* git(source, ["add", "feature.txt"]); + yield* git(source, ["commit", "-m", "feature commit"]); + yield* git(source, ["push", "-u", remoteName, featureBranch]); + yield* git(source, ["checkout", defaultBranch]); + yield* git(source, ["branch", "-D", featureBranch]); + + yield* checkoutGitBranch({ cwd: source, branch: `${remoteName}/${featureBranch}` }); + + expect(yield* git(source, ["branch", "--show-current"])).toBe("upstream/feature"); + }), + ); + it.effect("throws when checkout would overwrite uncommitted changes", () => Effect.gen(function* () { const tmp = yield* makeTmpDir(); diff --git a/apps/server/src/git/Layers/GitCore.ts b/apps/server/src/git/Layers/GitCore.ts index 8226d7d17e9b4a335669d6a9399bf0ba3c199aac..76968fcded808bc1cec98bea1fc052c409cd74f9 100644 --- a/apps/server/src/git/Layers/GitCore.ts +++ b/apps/server/src/git/Layers/GitCore.ts @@ -90,17 +90,61 @@ function parseBranchLine(line: string): { name: string; current: boolean } | nul }; } -function parseRemoteTrackingCandidate( +function parseRemoteNames(stdout: string): ReadonlyArray { + return stdout + .split("\n") + .map((line) => line.trim()) + .filter((line) => line.length > 0) + .toSorted((a, b) => b.length - a.length); +} + +function parseRemoteRefWithRemoteNames( branchName: string, -): { remoteRef: string; localBranch: string } | null { - const separatorIndex = branchName.indexOf("/"); - if (separatorIndex <= 0 || separatorIndex === branchName.length - 1) return null; - const localBranch = branchName.slice(separatorIndex + 1).trim(); - if (localBranch.length === 0) return null; - return { - remoteRef: branchName.trim(), - localBranch, - }; + remoteNames: ReadonlyArray, +): { remoteRef: string; remoteName: string; localBranch: string } | null { + const trimmedBranchName = branchName.trim(); + if (trimmedBranchName.length === 0) return null; + + for (const remoteName of remoteNames) { + const remotePrefix = `${remoteName}/`; + if (!trimmedBranchName.startsWith(remotePrefix)) { + continue; + } + const localBranch = trimmedBranchName.slice(remotePrefix.length).trim(); + if (localBranch.length === 0) { + return null; + } + return { + remoteRef: trimmedBranchName, + remoteName, + localBranch, + }; + } + + return null; +} + +function parseTrackingBranchByUpstreamRef( + stdout: string, + upstreamRef: string, +): string | null { + for (const line of stdout.split("\n")) { + const trimmedLine = line.trim(); + if (trimmedLine.length === 0) { + continue; + } + const [branchNameRaw, upstreamBranchRaw = ""] = trimmedLine.split("\t"); + const branchName = branchNameRaw?.trim() ?? ""; + const upstreamBranch = upstreamBranchRaw.trim(); + if (branchName.length === 0 || upstreamBranch.length === 0) { + continue; + } + if (upstreamBranch === upstreamRef) { + return branchName; + } + } + + return null; } function commandLabel(args: readonly string[]): string { @@ -619,7 +663,8 @@ const makeGitCore = Effect.gen(function* () { ); } - const [defaultRef, worktreeList, remoteBranchResult, branchLastCommit] = yield* Effect.all( + const [defaultRef, worktreeList, remoteBranchResult, remoteNamesResult, branchLastCommit] = + yield* Effect.all( [ executeGit( "GitCore.listBranches.defaultRef", @@ -648,11 +693,17 @@ const makeGitCore = Effect.gen(function* () { allowNonZeroExit: true, }, ), + executeGit("GitCore.listBranches.remoteNames", input.cwd, ["remote"], { + timeoutMs: 5_000, + allowNonZeroExit: true, + }), branchRecencyPromise, ], { concurrency: "unbounded" }, ); + const remoteNames = remoteNamesResult.code === 0 ? parseRemoteNames(remoteNamesResult.stdout) : []; + const defaultBranch = defaultRef.code === 0 ? defaultRef.stdout.trim().replace(/^refs\/remotes\/origin\//, "") @@ -705,13 +756,27 @@ const makeGitCore = Effect.gen(function* () { .split("\n") .map(parseBranchLine) .filter((branch): branch is { name: string; current: boolean } => branch !== null) - .map((branch) => ({ - name: branch.name, - current: false, - isRemote: true, - isDefault: false, - worktreePath: null, - })) + .map((branch) => { + const parsedRemoteRef = parseRemoteRefWithRemoteNames(branch.name, remoteNames); + const remoteBranch: { + name: string; + current: boolean; + isRemote: boolean; + remoteName?: string; + isDefault: boolean; + worktreePath: string | null; + } = { + name: branch.name, + current: false, + isRemote: true, + isDefault: false, + worktreePath: null, + }; + if (parsedRemoteRef) { + remoteBranch.remoteName = parsedRemoteRef.remoteName; + } + return remoteBranch; + }) .toSorted((a, b) => { const aLastCommit = branchLastCommit.get(a.name) ?? 0; const bLastCommit = branchLastCommit.get(b.name) ?? 0; @@ -781,23 +846,7 @@ const makeGitCore = Effect.gen(function* () { const checkoutBranch: GitCoreShape["checkoutBranch"] = (input) => Effect.gen(function* () { - const trackingCandidate = parseRemoteTrackingCandidate(input.branch); - if (!trackingCandidate) { - yield* executeGit("GitCore.checkoutBranch.checkout", input.cwd, ["checkout", input.branch], { - timeoutMs: 10_000, - fallbackErrorMessage: "git checkout failed", - }); - - // Refresh upstream refs in the background so checkout remains responsive. - yield* Effect.sync(() => { - void Effect.runPromise( - refreshCheckedOutBranchUpstream(input.cwd).pipe(Effect.catch(() => Effect.void)), - ); - }); - return; - } - - const [localInputExists, remoteExists, localTrackingExists] = yield* Effect.all( + const [localInputExists, remoteExists] = yield* Effect.all( [ executeGit( "GitCore.checkoutBranch.localInputExists", @@ -811,16 +860,7 @@ const makeGitCore = Effect.gen(function* () { executeGit( "GitCore.checkoutBranch.remoteExists", input.cwd, - ["show-ref", "--verify", "--quiet", `refs/remotes/${trackingCandidate.remoteRef}`], - { - timeoutMs: 5_000, - allowNonZeroExit: true, - }, - ).pipe(Effect.map((result) => result.code === 0)), - executeGit( - "GitCore.checkoutBranch.localTrackingExists", - input.cwd, - ["show-ref", "--verify", "--quiet", `refs/heads/${trackingCandidate.localBranch}`], + ["show-ref", "--verify", "--quiet", `refs/remotes/${input.branch}`], { timeoutMs: 5_000, allowNonZeroExit: true, @@ -830,12 +870,33 @@ const makeGitCore = Effect.gen(function* () { { concurrency: "unbounded" }, ); + const localTrackingBranch = + remoteExists + ? yield* executeGit( + "GitCore.checkoutBranch.localTrackingBranch", + input.cwd, + ["for-each-ref", "--format=%(refname:short)\t%(upstream:short)", "refs/heads"], + { + timeoutMs: 5_000, + allowNonZeroExit: true, + }, + ).pipe( + Effect.map((result) => + result.code === 0 + ? parseTrackingBranchByUpstreamRef(result.stdout, input.branch) + : null, + ), + ) + : null; + const checkoutArgs = localInputExists ? ["checkout", input.branch] - : remoteExists && !localTrackingExists - ? ["checkout", "--track", trackingCandidate.remoteRef] - : ["checkout", localTrackingExists ? trackingCandidate.localBranch : input.branch]; + : remoteExists && !localTrackingBranch + ? ["checkout", "--track", input.branch] + : remoteExists && localTrackingBranch + ? ["checkout", localTrackingBranch] + : ["checkout", input.branch]; yield* executeGit("GitCore.checkoutBranch.checkout", input.cwd, checkoutArgs, { timeoutMs: 10_000, diff --git a/apps/web/src/components/BranchToolbar.logic.test.ts b/apps/web/src/components/BranchToolbar.logic.test.ts index f7a12d5ec68e4147baaf27def1f878eaae7736be..a5c10d7d725d2489b13c2ade45da09213b3defbf 100644 --- a/apps/web/src/components/BranchToolbar.logic.test.ts +++ b/apps/web/src/components/BranchToolbar.logic.test.ts @@ -67,6 +67,12 @@ describe("deriveLocalBranchNameFromRemoteRef", () => { expect(deriveLocalBranchNameFromRemoteRef("origin/feature/demo")).toBe("feature/demo"); }); + it("supports remote names that contain slashes", () => { + expect( + deriveLocalBranchNameFromRemoteRef("my-org/upstream/feature/demo", "my-org/upstream"), + ).toBe("feature/demo"); + }); + it("returns the original name when ref is malformed", () => { expect(deriveLocalBranchNameFromRemoteRef("origin/")).toBe("origin/"); expect(deriveLocalBranchNameFromRemoteRef("/feature/demo")).toBe("/feature/demo"); @@ -85,6 +91,7 @@ describe("dedupeRemoteBranchesWithLocalMatches", () => { { name: "origin/feature/demo", isRemote: true, + remoteName: "origin", current: false, isDefault: false, worktreePath: null, @@ -92,6 +99,7 @@ describe("dedupeRemoteBranchesWithLocalMatches", () => { { name: "origin/feature/remote-only", isRemote: true, + remoteName: "origin", current: false, isDefault: false, worktreePath: null, @@ -115,6 +123,7 @@ describe("dedupeRemoteBranchesWithLocalMatches", () => { { name: "origin/feature/remote-only", isRemote: true, + remoteName: "origin", current: false, isDefault: false, worktreePath: null, @@ -126,4 +135,50 @@ describe("dedupeRemoteBranchesWithLocalMatches", () => { "origin/feature/remote-only", ]); }); + + it("dedupes remote refs for remotes whose names contain slashes", () => { + const input: GitBranch[] = [ + { + name: "feature/demo", + current: false, + isDefault: false, + worktreePath: null, + }, + { + name: "my-org/upstream/feature/demo", + isRemote: true, + remoteName: "my-org/upstream", + current: false, + isDefault: false, + worktreePath: null, + }, + ]; + + expect(dedupeRemoteBranchesWithLocalMatches(input).map((branch) => branch.name)).toEqual([ + "feature/demo", + ]); + }); + + it("dedupes remote refs when git tracks with first-slash local naming", () => { + const input: GitBranch[] = [ + { + name: "upstream/feature", + current: false, + isDefault: false, + worktreePath: null, + }, + { + name: "my-org/upstream/feature", + isRemote: true, + remoteName: "my-org/upstream", + current: false, + isDefault: false, + worktreePath: null, + }, + ]; + + expect(dedupeRemoteBranchesWithLocalMatches(input).map((branch) => branch.name)).toEqual([ + "upstream/feature", + ]); + }); }); diff --git a/apps/web/src/components/BranchToolbar.logic.ts b/apps/web/src/components/BranchToolbar.logic.ts index 606f1ebd92821fc49ec5d57eadafba3c6a8c0ded..07d081396418760d2bd809fae557eaa142a0915c 100644 --- a/apps/web/src/components/BranchToolbar.logic.ts +++ b/apps/web/src/components/BranchToolbar.logic.ts @@ -8,12 +8,42 @@ interface DeriveSyncedLocalBranchInput { queryBranches: ReadonlyArray | undefined; } -export function deriveLocalBranchNameFromRemoteRef(branchName: string): string { - const separatorIndex = branchName.indexOf("/"); - if (separatorIndex <= 0 || separatorIndex === branchName.length - 1) { +export function deriveLocalBranchNameFromRemoteRef( + branchName: string, + remoteName?: string, +): string { + if (remoteName) { + const remotePrefix = `${remoteName}/`; + if (branchName.startsWith(remotePrefix) && branchName.length > remotePrefix.length) { + return branchName.slice(remotePrefix.length); + } + } + + const firstSeparatorIndex = branchName.indexOf("/"); + if (firstSeparatorIndex <= 0 || firstSeparatorIndex === branchName.length - 1) { return branchName; } - return branchName.slice(separatorIndex + 1); + return branchName.slice(firstSeparatorIndex + 1); +} + +function deriveLocalBranchNameCandidatesFromRemoteRef( + branchName: string, + remoteName?: string, +): ReadonlyArray { + const candidates = new Set(); + const firstSlashCandidate = deriveLocalBranchNameFromRemoteRef(branchName); + if (firstSlashCandidate.length > 0) { + candidates.add(firstSlashCandidate); + } + + if (remoteName) { + const remotePrefix = `${remoteName}/`; + if (branchName.startsWith(remotePrefix) && branchName.length > remotePrefix.length) { + candidates.add(branchName.slice(remotePrefix.length)); + } + } + + return [...candidates]; } export function dedupeRemoteBranchesWithLocalMatches( @@ -28,8 +58,11 @@ export function dedupeRemoteBranchesWithLocalMatches( return true; } - const localBranchName = deriveLocalBranchNameFromRemoteRef(branch.name); - return !localBranchNames.has(localBranchName); + const localBranchCandidates = deriveLocalBranchNameCandidatesFromRemoteRef( + branch.name, + branch.remoteName, + ); + return !localBranchCandidates.some((candidate) => localBranchNames.has(candidate)); }); } diff --git a/apps/web/src/components/BranchToolbar.tsx b/apps/web/src/components/BranchToolbar.tsx index 09feff6a96c0256e35e864939688eb419c11a99e..be5f05bb9a41c8cb43843a455de4953803305476 100644 --- a/apps/web/src/components/BranchToolbar.tsx +++ b/apps/web/src/components/BranchToolbar.tsx @@ -179,13 +179,20 @@ export default function BranchToolbar({ } const selectedBranchName = branch.isRemote - ? deriveLocalBranchNameFromRemoteRef(branch.name) + ? deriveLocalBranchNameFromRemoteRef(branch.name, branch.remoteName) : branch.name; checkoutMutation.mutate(branch.name, { - onSuccess: () => { + onSuccess: async () => { setThreadError(null); - setThreadBranch(selectedBranchName, activeWorktreePath); + let nextBranchName = selectedBranchName; + if (branch.isRemote) { + const status = await api.git.status({ cwd: branchCwd }).catch(() => null); + if (status?.branch) { + nextBranchName = status.branch; + } + } + setThreadBranch(nextBranchName, activeWorktreePath); setIsBranchMenuOpen(false); onComposerFocusRequest?.(); }, diff --git a/packages/contracts/src/git.ts b/packages/contracts/src/git.ts index 8a2b00d86bf365700ea41a243ee54bb0f098ff8c..e0441366ef5aaae7ed893f26f7c5b14a4e97eda2 100644 --- a/packages/contracts/src/git.ts +++ b/packages/contracts/src/git.ts @@ -24,6 +24,7 @@ export type GitStatusPrState = typeof GitStatusPrState.Type; export const GitBranch = Schema.Struct({ name: TrimmedNonEmptyStringSchema, isRemote: Schema.optional(Schema.Boolean), + remoteName: Schema.optional(TrimmedNonEmptyStringSchema), current: Schema.Boolean, isDefault: Schema.Boolean, worktreePath: TrimmedNonEmptyStringSchema.pipe(Schema.NullOr),