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;