From dd98ae935912c9d1e199d4719f2b84550e8d0eb1 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 22 Apr 2026 17:51:41 +0700 Subject: [PATCH] fix(server): checkout fork PRs into owner branches --- .../resolve-worktree-creation-intent.ts | 122 +++++++++---- .../server/src/server/worktree-core.test.ts | 168 ++++++++++++++++++ packages/server/src/server/worktree-core.ts | 4 +- .../src/services/github-service.test.ts | 60 +++++++ .../server/src/services/github-service.ts | 111 ++++++++++++ packages/server/src/utils/worktree.ts | 59 +++++- 6 files changed, 481 insertions(+), 43 deletions(-) diff --git a/packages/server/src/server/resolve-worktree-creation-intent.ts b/packages/server/src/server/resolve-worktree-creation-intent.ts index f258c6ebc..aa962b57d 100644 --- a/packages/server/src/server/resolve-worktree-creation-intent.ts +++ b/packages/server/src/server/resolve-worktree-creation-intent.ts @@ -1,4 +1,4 @@ -import type { GitHubService } from "../services/github-service.js"; +import type { GitHubPullRequestCheckoutTarget, GitHubService } from "../services/github-service.js"; import type { AgentAttachment } from "./messages.js"; import { findGitHubPrAttachment } from "./agent/prompt-attachments.js"; import type { WorktreeSource } from "../utils/worktree.js"; @@ -63,18 +63,13 @@ export async function resolveWorktreeCreationIntent( if (input.action === "checkout") { if (input.githubPrNumber !== undefined) { - return { - kind: "checkout-github-pr", + return resolveGitHubPrCheckoutIntent({ + refName: input.refName, githubPrNumber: input.githubPrNumber, - headRef: await resolveGitHubPrHeadRef({ - refName: input.refName, - githubPrNumber: input.githubPrNumber, - githubPrAttachment, - repoRoot, - deps, - }), - baseRefName: await resolveGitHubPrBaseRefName(githubPrAttachment, repoRoot, deps), - }; + githubPrAttachment, + repoRoot, + deps, + }); } const branchName = input.refName?.trim(); @@ -89,18 +84,13 @@ export async function resolveWorktreeCreationIntent( } if (input.githubPrNumber !== undefined) { - return { - kind: "checkout-github-pr", + return resolveGitHubPrCheckoutIntent({ + refName: input.refName, githubPrNumber: input.githubPrNumber, - headRef: await resolveGitHubPrHeadRef({ - refName: input.refName, - githubPrNumber: input.githubPrNumber, - githubPrAttachment, - repoRoot, - deps, - }), - baseRefName: await resolveGitHubPrBaseRefName(githubPrAttachment, repoRoot, deps), - }; + githubPrAttachment, + repoRoot, + deps, + }); } if (input.refName?.trim()) { @@ -112,12 +102,12 @@ export async function resolveWorktreeCreationIntent( } if (githubPrAttachment) { - return { - kind: "checkout-github-pr", + return resolveGitHubPrCheckoutIntent({ githubPrNumber: githubPrAttachment.number, - headRef: await resolveGitHubPrAttachmentHeadRef(githubPrAttachment, repoRoot, deps), - baseRefName: await resolveGitHubPrBaseRefName(githubPrAttachment, repoRoot, deps), - }; + githubPrAttachment, + repoRoot, + deps, + }); } return { @@ -127,6 +117,54 @@ export async function resolveWorktreeCreationIntent( }; } +async function resolveGitHubPrCheckoutIntent(params: { + refName?: string; + githubPrNumber: number; + githubPrAttachment: Extract | null; + repoRoot: string; + deps: ResolveWorktreeCreationIntentDeps; +}): Promise> { + const checkoutTarget = await resolveGitHubPrCheckoutTarget(params); + const headRef = await resolveGitHubPrHeadRef({ + refName: params.refName, + githubPrNumber: params.githubPrNumber, + githubPrAttachment: params.githubPrAttachment, + checkoutTarget, + repoRoot: params.repoRoot, + deps: params.deps, + }); + const baseRefName = + checkoutTarget?.baseRefName?.trim() || + (await resolveGitHubPrBaseRefName(params.githubPrAttachment, params.repoRoot, params.deps)); + const localBranchName = buildGitHubPrLocalBranchName({ headRef, checkoutTarget }); + const pushRemoteUrl = checkoutTarget + ? checkoutTarget.headRepositorySshUrl || checkoutTarget.headRepositoryUrl || undefined + : undefined; + + return { + kind: "checkout-github-pr", + githubPrNumber: params.githubPrNumber, + headRef, + baseRefName, + ...(localBranchName !== headRef ? { localBranchName } : {}), + ...(pushRemoteUrl ? { pushRemoteUrl } : {}), + }; +} + +async function resolveGitHubPrCheckoutTarget(params: { + githubPrNumber: number; + repoRoot: string; + deps: ResolveWorktreeCreationIntentDeps; +}): Promise { + if (!params.deps.github.getPullRequestCheckoutTarget) { + return null; + } + return params.deps.github.getPullRequestCheckoutTarget({ + cwd: params.repoRoot, + number: params.githubPrNumber, + }); +} + async function resolveDefaultBranch( repoRoot: string, deps: ResolveWorktreeCreationIntentDeps, @@ -158,6 +196,7 @@ async function resolveGitHubPrHeadRef(params: { refName?: string; githubPrNumber: number; githubPrAttachment: Extract | null; + checkoutTarget?: GitHubPullRequestCheckoutTarget | null; repoRoot: string; deps: ResolveWorktreeCreationIntentDeps; }): Promise { @@ -165,6 +204,10 @@ async function resolveGitHubPrHeadRef(params: { if (trimmedRefName) { return trimmedRefName; } + const checkoutTargetHeadRef = params.checkoutTarget?.headRefName.trim(); + if (checkoutTargetHeadRef) { + return checkoutTargetHeadRef; + } if (params.githubPrAttachment) { const attachmentHeadRef = params.githubPrAttachment.headRefName?.trim(); if (attachmentHeadRef) { @@ -177,16 +220,19 @@ async function resolveGitHubPrHeadRef(params: { }); } -async function resolveGitHubPrAttachmentHeadRef( - attachment: Extract, - repoRoot: string, - deps: ResolveWorktreeCreationIntentDeps, -): Promise { - const trimmed = attachment.headRefName?.trim(); - if (trimmed && trimmed.length > 0) { - return trimmed; - } - return deps.github.getPullRequestHeadRef({ cwd: repoRoot, number: attachment.number }); +function buildGitHubPrLocalBranchName(params: { + headRef: string; + checkoutTarget: GitHubPullRequestCheckoutTarget | null; +}): string { + const owner = params.checkoutTarget?.isCrossRepository + ? normalizeGitHubOwnerForBranch(params.checkoutTarget.headOwnerLogin) + : null; + return owner ? `${owner}/${params.headRef}` : params.headRef; +} + +function normalizeGitHubOwnerForBranch(owner: string | null): string | null { + const normalized = owner?.trim().toLowerCase() ?? ""; + return /^[a-z0-9-]+$/.test(normalized) ? normalized : null; } async function resolveGitHubPrBaseRefName( diff --git a/packages/server/src/server/worktree-core.test.ts b/packages/server/src/server/worktree-core.test.ts index 647a619db..636fad844 100644 --- a/packages/server/src/server/worktree-core.test.ts +++ b/packages/server/src/server/worktree-core.test.ts @@ -153,6 +153,56 @@ function createGitHubPrRemoteRepo(): { tempDir: string; repoDir: string; paseoHo return { tempDir, repoDir, paseoHome }; } +function createForkGitHubPrRemoteRepo(): { + tempDir: string; + repoDir: string; + headRemoteDir: string; + paseoHome: string; +} { + const { tempDir, repoDir, paseoHome } = createGitRepo(); + const baseRemoteDir = path.join(tempDir, "base.git"); + const headRemoteDir = path.join(tempDir, "therainisme.git"); + const headCloneDir = path.join(tempDir, "therainisme-clone"); + + execSync(`git clone --bare ${JSON.stringify(repoDir)} ${JSON.stringify(baseRemoteDir)}`, { + stdio: "pipe", + }); + execSync(`git clone --bare ${JSON.stringify(repoDir)} ${JSON.stringify(headRemoteDir)}`, { + stdio: "pipe", + }); + execSync(`git remote add origin ${JSON.stringify(baseRemoteDir)}`, { + cwd: repoDir, + stdio: "pipe", + }); + + execSync(`git clone ${JSON.stringify(headRemoteDir)} ${JSON.stringify(headCloneDir)}`, { + stdio: "pipe", + }); + execSync("git config user.email 'test@test.com'", { cwd: headCloneDir, stdio: "pipe" }); + execSync("git config user.name 'Test'", { cwd: headCloneDir, stdio: "pipe" }); + writeFileSync(path.join(headCloneDir, "README.md"), "fork pr main branch\n"); + execSync("git add README.md", { cwd: headCloneDir, stdio: "pipe" }); + execSync("git -c commit.gpgsign=false commit -m 'fork pr main branch'", { + cwd: headCloneDir, + stdio: "pipe", + }); + const prHead = execSync("git rev-parse HEAD", { cwd: headCloneDir, stdio: "pipe" }) + .toString() + .trim(); + execSync("git push origin main", { cwd: headCloneDir, stdio: "pipe" }); + execSync( + `git --git-dir=${JSON.stringify(baseRemoteDir)} fetch ${JSON.stringify(headRemoteDir)} main`, + { stdio: "pipe" }, + ); + execSync( + `git --git-dir=${JSON.stringify(baseRemoteDir)} update-ref refs/pull/526/head ${prHead}`, + { stdio: "pipe" }, + ); + execSync("git fetch origin", { cwd: repoDir, stdio: "pipe" }); + + return { tempDir, repoDir, headRemoteDir, paseoHome }; +} + describe.skipIf(process.platform === "win32")("createWorktreeCore", () => { const cleanupPaths: string[] = []; @@ -354,6 +404,124 @@ describe.skipIf(process.platform === "win32")("createWorktreeCore", () => { expect(result.worktree.branchName).toBe("pr-123"); }); + test("checks out a fork PR whose head branch collides with local main", async () => { + const { tempDir, repoDir, headRemoteDir, paseoHome } = createForkGitHubPrRemoteRepo(); + cleanupPaths.push(tempDir); + const github = { + ...createGitHubServiceStub(), + getPullRequestCheckoutTarget: async () => ({ + number: 526, + baseRefName: "main", + headRefName: "main", + headOwnerLogin: "therainisme", + headRepositorySshUrl: headRemoteDir, + headRepositoryUrl: headRemoteDir, + isCrossRepository: true, + }), + }; + + const result = await createCoreWorktree( + { + cwd: repoDir, + action: "checkout", + githubPrNumber: 526, + refName: "main", + paseoHome, + runSetup: false, + }, + createCoreDeps({ github }), + ); + + const sourceBranch = execSync("git branch --show-current", { cwd: repoDir, stdio: "pipe" }) + .toString() + .trim(); + const worktreeBranch = execSync("git branch --show-current", { + cwd: result.worktree.worktreePath, + stdio: "pipe", + }) + .toString() + .trim(); + const readme = readFileSync(path.join(result.worktree.worktreePath, "README.md"), "utf8"); + writeFileSync(path.join(result.worktree.worktreePath, "FOLLOWUP.md"), "maintainer edit\n"); + execSync("git add FOLLOWUP.md", { cwd: result.worktree.worktreePath, stdio: "pipe" }); + execSync("git -c commit.gpgsign=false commit -m 'maintainer edit'", { + cwd: result.worktree.worktreePath, + stdio: "pipe", + }); + const pushDryRun = execSync("git push --dry-run 2>&1", { + cwd: result.worktree.worktreePath, + stdio: "pipe", + }).toString(); + + expect(sourceBranch).toBe("main"); + expect(result.intent).toEqual({ + kind: "checkout-github-pr", + githubPrNumber: 526, + headRef: "main", + baseRefName: "main", + localBranchName: "therainisme/main", + pushRemoteUrl: headRemoteDir, + }); + expect(result.worktree.branchName).toBe("therainisme/main"); + expect(path.basename(result.worktree.worktreePath)).toBe("therainisme-main"); + expect(worktreeBranch).toBe("therainisme/main"); + expect(readme).toBe("fork pr main branch\n"); + expect(pushDryRun).toContain("HEAD -> main"); + }); + + test("uses a unique local branch when the same fork PR branch already exists", async () => { + const { tempDir, repoDir, headRemoteDir, paseoHome } = createForkGitHubPrRemoteRepo(); + cleanupPaths.push(tempDir); + const github = { + ...createGitHubServiceStub(), + getPullRequestCheckoutTarget: async () => ({ + number: 526, + baseRefName: "main", + headRefName: "main", + headOwnerLogin: "therainisme", + headRepositorySshUrl: headRemoteDir, + headRepositoryUrl: headRemoteDir, + isCrossRepository: true, + }), + }; + + const first = await createCoreWorktree( + { + cwd: repoDir, + worktreeSlug: "first-pr-worktree", + action: "checkout", + githubPrNumber: 526, + refName: "main", + paseoHome, + runSetup: false, + }, + createCoreDeps({ github }), + ); + const second = await createCoreWorktree( + { + cwd: repoDir, + worktreeSlug: "second-pr-worktree", + action: "checkout", + githubPrNumber: 526, + refName: "main", + paseoHome, + runSetup: false, + }, + createCoreDeps({ github }), + ); + + expect(first.worktree.branchName).toBe("therainisme/main"); + expect(second.worktree.branchName).toBe("therainisme/main-1"); + expect( + execSync("git config --get remote.paseo-pr-526.push", { + cwd: second.worktree.worktreePath, + stdio: "pipe", + }) + .toString() + .trim(), + ).toBe("HEAD:refs/heads/main"); + }); + test("throws a typed error for an unknown checkout branch", async () => { const { tempDir, repoDir, paseoHome } = createGitRepo(); cleanupPaths.push(tempDir); diff --git a/packages/server/src/server/worktree-core.ts b/packages/server/src/server/worktree-core.ts index 5d96dc635..fc6b35714 100644 --- a/packages/server/src/server/worktree-core.ts +++ b/packages/server/src/server/worktree-core.ts @@ -62,7 +62,9 @@ export async function createWorktreeCore( break; } case "checkout-github-pr": { - normalizedSlug = validateWorktreeSlug(requestedSlug ?? slugify(intent.headRef)); + normalizedSlug = validateWorktreeSlug( + requestedSlug ?? slugify(intent.localBranchName ?? intent.headRef), + ); break; } } diff --git a/packages/server/src/services/github-service.test.ts b/packages/server/src/services/github-service.test.ts index f29f84498..a98732a69 100644 --- a/packages/server/src/services/github-service.test.ts +++ b/packages/server/src/services/github-service.test.ts @@ -161,6 +161,34 @@ function pullRequestJson(title: string): string { ]); } +function pullRequestCheckoutTargetJson(): string { + return JSON.stringify({ + data: { + repository: { + pullRequest: { + number: 526, + baseRefName: "main", + headRefName: "main", + isCrossRepository: true, + headRepositoryOwner: { login: "therainisme" }, + headRepository: { + sshUrl: "git@github.com:therainisme/paseo.git", + url: "https://github.com/therainisme/paseo", + }, + }, + }, + }, + }); +} + +function repoViewJson(): string { + return JSON.stringify({ + owner: { login: "getpaseo" }, + name: "paseo", + parent: null, + }); +} + function issueJson(title: string): string { return JSON.stringify([ { @@ -275,6 +303,38 @@ describe("GitHubService", () => { expect(computeGithubNextInterval(stableStatus, 4)).toBe(EXPECTED_GITHUB_ERROR_BACKOFF_CAP_MS); }); + it("loads pull request checkout target details through GraphQL", async () => { + const runner = createRunner([repoViewJson(), pullRequestCheckoutTargetJson()]); + const service = createGitHubService({ + runner: runner.runner, + resolveGhPath: async () => "/usr/bin/gh", + now: () => 100, + }); + + await expect( + service.getPullRequestCheckoutTarget?.({ cwd: "/repo", number: 526 }), + ).resolves.toEqual({ + number: 526, + baseRefName: "main", + headRefName: "main", + headOwnerLogin: "therainisme", + headRepositorySshUrl: "git@github.com:therainisme/paseo.git", + headRepositoryUrl: "https://github.com/therainisme/paseo", + isCrossRepository: true, + }); + + expect(runner.calls).toHaveLength(2); + expect(runner.calls[0]).toEqual({ + cwd: "/repo", + args: ["repo", "view", "--json", "owner,name,parent"], + }); + expect(runner.calls[1]?.cwd).toBe("/repo"); + expect(runner.calls[1]?.args.slice(0, 3)).toEqual(["api", "graphql", "-f"]); + expect(runner.calls[1]?.args).toContain("owner=getpaseo"); + expect(runner.calls[1]?.args).toContain("name=paseo"); + expect(runner.calls[1]?.args).toContain("number=526"); + }); + it("polls PR status at fast cadence while checks are pending", async () => { let now = 0; const runner = createRunner([ diff --git a/packages/server/src/services/github-service.ts b/packages/server/src/services/github-service.ts index 7f3e06817..bc5f993a6 100644 --- a/packages/server/src/services/github-service.ts +++ b/packages/server/src/services/github-service.ts @@ -185,6 +185,53 @@ const GitHubRepoViewSchema = z.object({ .optional(), }); +const PullRequestCheckoutTargetSchema = z.object({ + data: z.object({ + repository: z.object({ + pullRequest: z + .object({ + number: z.number(), + baseRefName: z.string().catch(""), + headRefName: z.string().catch(""), + isCrossRepository: z.boolean().catch(false), + headRepositoryOwner: z + .object({ + login: z.string().catch(""), + }) + .nullable() + .optional(), + headRepository: z + .object({ + sshUrl: z.string().nullable().optional(), + url: z.string().nullable().optional(), + }) + .nullable() + .optional(), + }) + .nullable(), + }), + }), +}); + +const PULL_REQUEST_CHECKOUT_TARGET_QUERY = ` +query PullRequestCheckoutTarget($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + number + baseRefName + headRefName + isCrossRepository + headRepositoryOwner { + login + } + headRepository { + sshUrl + url + } + } + } +}`; + const CURRENT_PR_STATUS_FIELDS = "number,url,title,state,isDraft,baseRefName,headRefName,mergedAt,statusCheckRollup,reviewDecision,headRepositoryOwner"; @@ -266,6 +313,16 @@ export interface GitHubPullRequestSummary { updatedAt: string; } +export interface GitHubPullRequestCheckoutTarget { + number: number; + baseRefName: string; + headRefName: string; + headOwnerLogin: string | null; + headRepositorySshUrl: string | null; + headRepositoryUrl: string | null; + isCrossRepository: boolean; +} + export interface GitHubIssueSummary { number: number; title: string; @@ -417,6 +474,9 @@ export interface GitHubService { listIssues(options: ListGitHubIssuesOptions): Promise; getPullRequest(options: GetGitHubPullRequestOptions): Promise; getPullRequestHeadRef(options: GetGitHubPullRequestOptions): Promise; + getPullRequestCheckoutTarget?( + options: GetGitHubPullRequestOptions, + ): Promise; getCurrentPullRequestStatus( options: { cwd: string; @@ -756,6 +816,40 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G return pullRequest.headRefName; }, + getPullRequestCheckoutTarget(options) { + return cached({ + cwd: options.cwd, + method: "getPullRequestCheckoutTarget", + args: { number: options.number }, + readOptions: options, + load: async () => { + const repo = await getGitHubRepoView({ cwd: options.cwd, run }); + const owner = repo?.owner?.login; + const name = repo?.name; + if (!owner || !name) { + throw new Error("Unable to resolve GitHub repository for pull request checkout"); + } + + const stdout = await run( + [ + "api", + "graphql", + "-f", + `query=${PULL_REQUEST_CHECKOUT_TARGET_QUERY}`, + "-F", + `owner=${owner}`, + "-F", + `name=${name}`, + "-F", + `number=${options.number}`, + ], + { cwd: options.cwd }, + ); + return parsePullRequestCheckoutTarget(stdout); + }, + }); + }, + getCurrentPullRequestStatus(options) { return cached({ cwd: options.cwd, @@ -1364,6 +1458,23 @@ function parsePullRequestSummary(stdout: string): GitHubPullRequestSummary { return toPullRequestSummary(GitHubPullRequestSummarySchema.parse(JSON.parse(stdout || "{}"))); } +function parsePullRequestCheckoutTarget(stdout: string): GitHubPullRequestCheckoutTarget { + const parsed = PullRequestCheckoutTargetSchema.parse(JSON.parse(stdout || "{}")); + const pullRequest = parsed.data.repository.pullRequest; + if (!pullRequest) { + throw new Error("Pull request not found"); + } + return { + number: pullRequest.number, + baseRefName: pullRequest.baseRefName, + headRefName: pullRequest.headRefName, + headOwnerLogin: pullRequest.headRepositoryOwner?.login || null, + headRepositorySshUrl: pullRequest.headRepository?.sshUrl || null, + headRepositoryUrl: pullRequest.headRepository?.url || null, + isCrossRepository: pullRequest.isCrossRepository, + }; +} + function toPullRequestSummary( item: z.infer, ): GitHubPullRequestSummary { diff --git a/packages/server/src/utils/worktree.ts b/packages/server/src/utils/worktree.ts index 33590109d..bdb3a0aba 100644 --- a/packages/server/src/utils/worktree.ts +++ b/packages/server/src/utils/worktree.ts @@ -160,6 +160,8 @@ export type WorktreeSource = githubPrNumber: number; headRef: string; baseRefName: string; + localBranchName?: string; + pushRemoteUrl?: string; }; export interface CreateWorktreeOptions { @@ -1227,6 +1229,14 @@ export const createWorktree = async ({ }); worktreePath = normalizePathForOwnership(finalWorktreePath); + if (sourcePlan.pushRemote) { + await configureWorktreePushRemote({ + cwd, + branchName: sourcePlan.branchName, + remote: sourcePlan.pushRemote, + }); + } + writePaseoWorktreeMetadata(worktreePath, { baseRefName: sourcePlan.metadataBaseRefName }); if (runSetup) { @@ -1253,6 +1263,11 @@ interface WorktreeSourcePlan { branchName: string; metadataBaseRefName: string; addArguments: string[]; + pushRemote?: { + name: string; + url: string; + headRef: string; + }; } async function resolveWorktreeSourcePlan({ @@ -1300,13 +1315,15 @@ async function resolveWorktreeSourcePlan({ }; } case "checkout-github-pr": { - validateWorktreeBranchName(source.headRef); + const localBranchCandidate = source.localBranchName ?? source.headRef; + validateWorktreeBranchName(localBranchCandidate); + const localBranchName = await resolveUniqueLocalBranchName(cwd, localBranchCandidate); const normalizedBaseRefName = normalizeRequiredBaseBranch(source.baseRefName); await runGitCommand( [ "fetch", "origin", - `refs/pull/${source.githubPrNumber}/head:refs/heads/${source.headRef}`, + `refs/pull/${source.githubPrNumber}/head:refs/heads/${localBranchName}`, "--force", ], { @@ -1316,14 +1333,48 @@ async function resolveWorktreeSourcePlan({ ); return { - branchName: source.headRef, + branchName: localBranchName, metadataBaseRefName: normalizedBaseRefName, - addArguments: [source.headRef], + addArguments: [localBranchName], + ...(source.pushRemoteUrl + ? { + pushRemote: { + name: `paseo-pr-${source.githubPrNumber}`, + url: source.pushRemoteUrl, + headRef: source.headRef, + }, + } + : {}), }; } } } +async function configureWorktreePushRemote(options: { + cwd: string; + branchName: string; + remote: { + name: string; + url: string; + headRef: string; + }; +}): Promise { + await runGitCommand(["config", `remote.${options.remote.name}.url`, options.remote.url], { + cwd: options.cwd, + }); + await runGitCommand( + ["config", `remote.${options.remote.name}.push`, `HEAD:refs/heads/${options.remote.headRef}`], + { cwd: options.cwd }, + ); + await runGitCommand(["config", `branch.${options.branchName}.remote`, options.remote.name], { + cwd: options.cwd, + }); + await runGitCommand( + ["config", `branch.${options.branchName}.merge`, `refs/heads/${options.remote.headRef}`], + { cwd: options.cwd }, + ); +} + function validateWorktreeBranchName(branchName: string): void { const validation = validateBranchSlug(branchName); if (!validation.valid) {