From f9ff668a7119f7698cc83c1d65af6d85a6726520 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 25 Jun 2026 13:05:32 +0800 Subject: [PATCH] Link worktrees to PRs from differently named tracked branches (#1718) * fix(server): link PRs for differently named tracked branches PR lookup now uses the configured tracked head ref when it differs from the local branch, while keeping fork owner scoping only for cross-repo remotes. * fix(server): avoid extra PR lookup git reads Use the shared lookup-target builder for an early local-branch result before reading origin and base metadata on on-demand PR status refreshes. --- .../server/src/utils/checkout-git.test.ts | 176 +++++++++++++++--- packages/server/src/utils/checkout-git.ts | 98 ++++++---- 2 files changed, 218 insertions(+), 56 deletions(-) diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index b6b155e07..2a28cdeaa 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -149,6 +149,44 @@ function createPullRequestStatus(overrides?: Partial; +} + +function createGitHubServiceRecordingPullRequestTargets( + options: RecordingPullRequestTargetsOptions, +): GitHubService { + const github = createGitHubServiceForStatus(null); + github.getCurrentPullRequestStatus = async (request) => { + options.requestedTargets.push({ + headRef: request.headRef, + ...(request.headRepositoryOwner ? { headRepositoryOwner: request.headRepositoryOwner } : {}), + }); + return createPullRequestStatus({ + ...options.statusOverrides, + headRefName: request.headRef, + }); + }; + return github; +} + +async function readPullRequestLookupTargetFromFacts( + repoDir: string, + paseoHome: string, +): Promise { + const facts = await getCheckoutSnapshotFacts(repoDir, { paseoHome }); + if (!facts.isGit) { + throw new Error("Expected git checkout facts"); + } + return facts.pullRequestLookupTarget; +} + function setupRemoteTrackingMain( repoDir: string, tempDir: string, @@ -2000,6 +2038,118 @@ const x = 1; }); }); + it("uses an origin tracked head when the local branch name differs", async () => { + execFileSync("git", ["checkout", "-b", "tender-parrot"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], { + cwd: repoDir, + }); + execFileSync("git", ["config", "branch.tender-parrot.remote", "origin"], { cwd: repoDir }); + execFileSync( + "git", + ["config", "branch.tender-parrot.merge", "refs/heads/refactor/workspace-scripts"], + { cwd: repoDir }, + ); + + const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); + + expect(lookupTarget).toEqual({ headRef: "refactor/workspace-scripts" }); + }); + + it("keeps the local branch lookup when origin tracking uses the same head name", async () => { + execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], { + cwd: repoDir, + }); + execFileSync("git", ["config", "branch.feature.remote", "origin"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.feature.merge", "refs/heads/feature"], { + cwd: repoDir, + }); + + const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); + + expect(lookupTarget).toEqual({ headRef: "feature" }); + }); + + it("does not attach an owner when the tracked remote is the same GitHub repository", async () => { + execFileSync("git", ["checkout", "-b", "local-feature"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "git@github.com:getpaseo/paseo.git"], { + cwd: repoDir, + }); + execFileSync("git", ["remote", "add", "upstream", "https://github.com/getpaseo/paseo.git"], { + cwd: repoDir, + }); + execFileSync("git", ["config", "branch.local-feature.remote", "upstream"], { + cwd: repoDir, + }); + execFileSync( + "git", + ["config", "branch.local-feature.merge", "refs/heads/refactor/workspace-scripts"], + { cwd: repoDir }, + ); + + const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); + + expect(lookupTarget).toEqual({ headRef: "refactor/workspace-scripts" }); + }); + + it("keeps the fork owner when same-repo comparison is indeterminate", async () => { + execFileSync("git", ["checkout", "-b", "chethanuk/main"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "not-a-github-remote"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "paseo-pr-345", "git@github.com:chethanuk/paseo.git"], { + cwd: repoDir, + }); + execFileSync("git", ["config", "branch.chethanuk/main.remote", "paseo-pr-345"], { + cwd: repoDir, + }); + execFileSync("git", ["config", "branch.chethanuk/main.merge", "refs/heads/main"], { + cwd: repoDir, + }); + + const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); + + expect(lookupTarget).toEqual({ headRef: "main", headRepositoryOwner: "chethanuk" }); + }); + + it("keeps the local branch lookup when same-repo tracking points at the base branch", async () => { + execFileSync("git", ["checkout", "-b", "tender-parrot"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], { + cwd: repoDir, + }); + execFileSync("git", ["config", "branch.tender-parrot.remote", "origin"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.tender-parrot.merge", "refs/heads/main"], { + cwd: repoDir, + }); + + const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); + + expect(lookupTarget).toEqual({ headRef: "tender-parrot" }); + }); + + it("derives the same origin tracked head for on-demand PR status reads", async () => { + execFileSync("git", ["checkout", "-b", "tender-parrot"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], { + cwd: repoDir, + }); + execFileSync("git", ["config", "branch.tender-parrot.remote", "origin"], { cwd: repoDir }); + execFileSync( + "git", + ["config", "branch.tender-parrot.merge", "refs/heads/refactor/workspace-scripts"], + { cwd: repoDir }, + ); + const factsTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome); + const requestedTargets: RequestedPullRequestTarget[] = []; + const github = createGitHubServiceRecordingPullRequestTargets({ requestedTargets }); + + await getPullRequestStatus( + repoDir, + github, + { force: true, reason: "tracked-head-parity" }, + { paseoHome }, + ); + + expect(requestedTargets).toEqual([factsTarget]); + }); + it("uses the tracked fork branch for PR worktree status lookup", async () => { execFileSync("git", ["checkout", "-b", "chethanuk/main"], { cwd: repoDir }); execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], { @@ -2015,30 +2165,14 @@ const x = 1; cwd: repoDir, }); - const requestedTargets: Array<{ headRef: string; headRepositoryOwner?: string }> = []; - const github = createGitHubServiceForStatus( - createPullRequestStatus({ + const requestedTargets: RequestedPullRequestTarget[] = []; + const github = createGitHubServiceRecordingPullRequestTargets({ + requestedTargets, + statusOverrides: { number: 345, url: "https://github.com/getpaseo/paseo/pull/345", - headRefName: "main", - }), - { - onStatus: () => {}, }, - ); - github.getCurrentPullRequestStatus = async (options) => { - requestedTargets.push({ - headRef: options.headRef, - ...(options.headRepositoryOwner - ? { headRepositoryOwner: options.headRepositoryOwner } - : {}), - }); - return createPullRequestStatus({ - number: 345, - url: "https://github.com/getpaseo/paseo/pull/345", - headRefName: options.headRef, - }); - }; + }); const status = await getPullRequestStatus(repoDir, github); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index c41c78049..61dab9c79 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -70,6 +70,15 @@ interface PullRequestStatusLookupTarget { headRepositoryOwner?: string; } +interface PullRequestLookupTargetBranchConfig { + currentBranch: string; + branchRemoteName: string | null; + branchMergeRef: string | null; + branchRemoteUrl: string | null; + originRemoteUrl: string | null; + resolvedBaseRef: string | null; +} + function getErrorStderr(error: Error): string { return "stderr" in error && typeof error.stderr === "string" ? error.stderr : ""; } @@ -1132,24 +1141,37 @@ async function resolvePullRequestStatusLookupTarget( if (context?.facts?.isGit && context.facts.pullRequestLookupTarget) { return context.facts.pullRequestLookupTarget; } - const remoteName = await getGitConfigValue(cwd, `branch.${currentBranch}.remote`); - if (!remoteName?.startsWith("paseo-pr-")) { - return { headRef: currentBranch }; + const branchRemoteName = await getGitConfigValue(cwd, `branch.${currentBranch}.remote`, context); + let branchMergeRef: string | null = null; + if (branchRemoteName) { + branchMergeRef = await getGitConfigValue(cwd, `branch.${currentBranch}.merge`, context); } - const mergeRef = await getGitConfigValue(cwd, `branch.${currentBranch}.merge`); - const trackedHeadRef = parseBranchMergeHeadRef(mergeRef); - if (!trackedHeadRef) { - return { headRef: currentBranch }; + const localBranchTarget = buildPullRequestLookupTargetFromBranchConfig({ + currentBranch, + branchRemoteName, + branchMergeRef, + branchRemoteUrl: null, + originRemoteUrl: null, + resolvedBaseRef: null, + }); + if (localBranchTarget.headRef === currentBranch) { + return localBranchTarget; } - const remoteUrl = await getGitConfigValue(cwd, `remote.${remoteName}.url`); - const remoteRepo = remoteUrl ? parseGitHubRepoFromRemote(remoteUrl) : null; - const headRepositoryOwner = remoteRepo?.split("/")[0]; - return { - headRef: trackedHeadRef, - ...(headRepositoryOwner ? { headRepositoryOwner } : {}), - }; + const [branchRemoteUrl, originRemoteUrl, resolvedBaseRef] = await Promise.all([ + branchRemoteName ? getGitConfigValue(cwd, `remote.${branchRemoteName}.url`, context) : null, + getGitConfigValue(cwd, "remote.origin.url", context), + getResolvedBaseRefForCwd(cwd, context), + ]); + return buildPullRequestLookupTargetFromBranchConfig({ + currentBranch, + branchRemoteName, + branchMergeRef, + branchRemoteUrl, + originRemoteUrl, + resolvedBaseRef, + }); } export async function resolveAbsoluteGitDir(cwd: string): Promise { @@ -1498,25 +1520,33 @@ async function inspectCheckoutContext( } } -function buildPullRequestLookupTargetFromBranchConfig(input: { - currentBranch: string; - branchRemoteName: string | null; - branchMergeRef: string | null; - branchRemoteUrl: string | null; -}): PullRequestStatusLookupTarget { - if (!input.branchRemoteName?.startsWith("paseo-pr-")) { - return { headRef: input.currentBranch }; - } - +function buildPullRequestLookupTargetFromBranchConfig( + input: PullRequestLookupTargetBranchConfig, +): PullRequestStatusLookupTarget { const trackedHeadRef = parseBranchMergeHeadRef(input.branchMergeRef); - if (!trackedHeadRef) { + if (!input.branchRemoteName || !trackedHeadRef || trackedHeadRef === input.currentBranch) { return { headRef: input.currentBranch }; } const remoteRepo = input.branchRemoteUrl ? parseGitHubRepoFromRemote(input.branchRemoteUrl) : null; - const headRepositoryOwner = remoteRepo?.split("/")[0]; + const originRepo = input.originRemoteUrl + ? parseGitHubRepoFromRemote(input.originRemoteUrl) + : null; + const isSameRepo = Boolean(remoteRepo && originRepo && remoteRepo === originRepo); + const headRepositoryOwner = remoteRepo && !isSameRepo ? remoteRepo.split("/")[0] : null; + const normalizedBaseRef = input.resolvedBaseRef + ? normalizeLocalBranchRefName(input.resolvedBaseRef) + : null; + if (trackedHeadRef === normalizedBaseRef && !headRepositoryOwner) { + return { headRef: input.currentBranch }; + } + + if (isSameRepo) { + return { headRef: trackedHeadRef }; + } + return { headRef: trackedHeadRef, ...(headRepositoryOwner ? { headRepositoryOwner } : {}), @@ -1559,21 +1589,17 @@ export async function getCheckoutSnapshotFacts( let branchRemoteName: string | null = null; let branchMergeRef: string | null = null; let branchRemoteUrl: string | null = null; - if (inspected.remoteUrl && inspected.currentBranch) { + if (inspected.currentBranch) { branchRemoteName = await getGitConfigValue( cwd, `branch.${inspected.currentBranch}.remote`, context, ); if (branchRemoteName) { - branchMergeRef = await getGitConfigValue( - cwd, - `branch.${inspected.currentBranch}.merge`, - context, - ); - if (branchRemoteName.startsWith("paseo-pr-")) { - branchRemoteUrl = await getGitConfigValue(cwd, `remote.${branchRemoteName}.url`, context); - } + [branchMergeRef, branchRemoteUrl] = await Promise.all([ + getGitConfigValue(cwd, `branch.${inspected.currentBranch}.merge`, context), + getGitConfigValue(cwd, `remote.${branchRemoteName}.url`, context), + ]); } } const pullRequestLookupTarget = inspected.currentBranch @@ -1582,6 +1608,8 @@ export async function getCheckoutSnapshotFacts( branchRemoteName, branchMergeRef, branchRemoteUrl, + originRemoteUrl: inspected.remoteUrl, + resolvedBaseRef, }) : null;