From d00a40b2e405658c73d5ba3083227752c059b593 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Thu, 19 Feb 2026 12:59:08 +0200 Subject: [PATCH] fix: Setup Cloud Code Reviewer to work for git forks --- cloud-agent-next/src/workspace.test.ts | 28 ++++++++ cloud-agent-next/src/workspace.ts | 35 ++++++++++ cloud-agent/src/workspace.test.ts | 28 ++++++++ cloud-agent/src/workspace.ts | 35 ++++++++++ .../pull-request-checkout-ref.ts | 42 ++++++++++++ .../pull-request-handler.test.ts | 65 +++++++++++++++++++ .../webhook-handlers/pull-request-handler.ts | 12 +++- .../platforms/github/webhook-schemas.ts | 6 ++ 8 files changed, 250 insertions(+), 1 deletion(-) create mode 100644 src/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref.ts create mode 100644 src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts diff --git a/cloud-agent-next/src/workspace.test.ts b/cloud-agent-next/src/workspace.test.ts index d39786451..44f4b8ed5 100644 --- a/cloud-agent-next/src/workspace.test.ts +++ b/cloud-agent-next/src/workspace.test.ts @@ -107,6 +107,34 @@ describe('manageBranch', () => { }); describe('and it is an upstream branch', () => { + it('should fetch and checkout GitHub pull refs', async () => { + mockExec + .mockResolvedValueOnce({ exitCode: 0 }) // git fetch + .mockResolvedValueOnce({ exitCode: 1 }) // local check (does not exist) + .mockResolvedValueOnce({ exitCode: 1 }) // remote check (does not exist) + .mockResolvedValueOnce({ exitCode: 0 }) // fetch pull ref + .mockResolvedValueOnce({ exitCode: 0 }); // checkout from FETCH_HEAD + + const result = await manageBranch(fakeSession, '/workspace', 'refs/pull/42/head', true); + + const execCalls = mockExec.mock.calls; + expect(execCalls[3]?.[0]).toContain("git fetch origin 'refs/pull/42/head'"); + expect(execCalls[4]?.[0]).toContain("git checkout -B 'refs/pull/42/head' FETCH_HEAD"); + expect(result).toBe('refs/pull/42/head'); + }); + + it('should throw when pull ref fetch fails', async () => { + mockExec + .mockResolvedValueOnce({ exitCode: 0 }) // git fetch + .mockResolvedValueOnce({ exitCode: 1 }) // local check (does not exist) + .mockResolvedValueOnce({ exitCode: 1 }) // remote check (does not exist) + .mockResolvedValueOnce({ exitCode: 1, stderr: 'fetch pull ref error' }); // fetch pull ref fails + + await expect( + manageBranch(fakeSession, '/workspace', 'refs/pull/42/head', true) + ).rejects.toThrow('Failed to fetch pull ref refs/pull/42/head'); + }); + it('should throw error', async () => { mockExec .mockResolvedValueOnce({ exitCode: 0 }) // git fetch diff --git a/cloud-agent-next/src/workspace.ts b/cloud-agent-next/src/workspace.ts index f421e17db..16b838a9c 100644 --- a/cloud-agent-next/src/workspace.ts +++ b/cloud-agent-next/src/workspace.ts @@ -485,6 +485,34 @@ async function createNewBranch( } } +const GITHUB_PULL_REF_PATTERN = /^refs\/pull\/\d+\/head$/; + +async function fetchPullRefAndCheckout( + session: ExecutionSession, + workspacePath: string, + pullRef: string +): Promise { + if (!GITHUB_PULL_REF_PATTERN.test(pullRef)) { + throw new Error(`Invalid pull ref format: ${pullRef}`); + } + + const fetchResult = await session.exec(`cd ${workspacePath} && git fetch origin '${pullRef}'`); + if (fetchResult.exitCode !== 0) { + throw new Error( + `Failed to fetch pull ref ${pullRef}: ${fetchResult.stderr || fetchResult.stdout}` + ); + } + + const checkoutResult = await session.exec( + `cd ${workspacePath} && git checkout -B '${pullRef}' FETCH_HEAD` + ); + if (checkoutResult.exitCode !== 0) { + throw new Error( + `Failed to checkout pull ref ${pullRef}: ${checkoutResult.stderr || checkoutResult.stdout}` + ); + } +} + /** * Manage branch checkout/creation. * @@ -542,6 +570,13 @@ export async function manageBranch( } else { // Case 4: Doesn't exist anywhere if (isUpstreamBranch) { + if (GITHUB_PULL_REF_PATTERN.test(branchName)) { + await fetchPullRefAndCheckout(session, workspacePath, branchName); + logger.withTags({ pullRef: branchName }).info('Checked out GitHub pull ref'); + logger.debug('Successfully on branch'); + return branchName; + } + throw new Error( `Branch "${branchName}" not found in repository. Please ensure the branch exists remotely.` ); diff --git a/cloud-agent/src/workspace.test.ts b/cloud-agent/src/workspace.test.ts index 2930d42e3..14ea2ba91 100644 --- a/cloud-agent/src/workspace.test.ts +++ b/cloud-agent/src/workspace.test.ts @@ -107,6 +107,34 @@ describe('manageBranch', () => { }); describe('and it is an upstream branch', () => { + it('should fetch and checkout GitHub pull refs', async () => { + mockExec + .mockResolvedValueOnce({ exitCode: 0 }) // git fetch + .mockResolvedValueOnce({ exitCode: 1 }) // local check (does not exist) + .mockResolvedValueOnce({ exitCode: 1 }) // remote check (does not exist) + .mockResolvedValueOnce({ exitCode: 0 }) // fetch pull ref + .mockResolvedValueOnce({ exitCode: 0 }); // checkout from FETCH_HEAD + + const result = await manageBranch(fakeSession, '/workspace', 'refs/pull/42/head', true); + + const execCalls = mockExec.mock.calls; + expect(execCalls[3]?.[0]).toContain("git fetch origin 'refs/pull/42/head'"); + expect(execCalls[4]?.[0]).toContain("git checkout -B 'refs/pull/42/head' FETCH_HEAD"); + expect(result).toBe('refs/pull/42/head'); + }); + + it('should throw when pull ref fetch fails', async () => { + mockExec + .mockResolvedValueOnce({ exitCode: 0 }) // git fetch + .mockResolvedValueOnce({ exitCode: 1 }) // local check (does not exist) + .mockResolvedValueOnce({ exitCode: 1 }) // remote check (does not exist) + .mockResolvedValueOnce({ exitCode: 1, stderr: 'fetch pull ref error' }); // fetch pull ref fails + + await expect( + manageBranch(fakeSession, '/workspace', 'refs/pull/42/head', true) + ).rejects.toThrow('Failed to fetch pull ref refs/pull/42/head'); + }); + it('should throw error', async () => { mockExec .mockResolvedValueOnce({ exitCode: 0 }) // git fetch diff --git a/cloud-agent/src/workspace.ts b/cloud-agent/src/workspace.ts index 1a39cf33a..0dea68a82 100644 --- a/cloud-agent/src/workspace.ts +++ b/cloud-agent/src/workspace.ts @@ -585,6 +585,34 @@ async function createNewBranch( } } +const GITHUB_PULL_REF_PATTERN = /^refs\/pull\/\d+\/head$/; + +async function fetchPullRefAndCheckout( + session: ExecutionSession, + workspacePath: string, + pullRef: string +): Promise { + if (!GITHUB_PULL_REF_PATTERN.test(pullRef)) { + throw new Error(`Invalid pull ref format: ${pullRef}`); + } + + const fetchResult = await session.exec(`cd ${workspacePath} && git fetch origin '${pullRef}'`); + if (fetchResult.exitCode !== 0) { + throw new Error( + `Failed to fetch pull ref ${pullRef}: ${fetchResult.stderr || fetchResult.stdout}` + ); + } + + const checkoutResult = await session.exec( + `cd ${workspacePath} && git checkout -B '${pullRef}' FETCH_HEAD` + ); + if (checkoutResult.exitCode !== 0) { + throw new Error( + `Failed to checkout pull ref ${pullRef}: ${checkoutResult.stderr || checkoutResult.stdout}` + ); + } +} + /** * Manage branch checkout/creation. * @@ -642,6 +670,13 @@ export async function manageBranch( } else { // Case 4: Doesn't exist anywhere if (isUpstreamBranch) { + if (GITHUB_PULL_REF_PATTERN.test(branchName)) { + await fetchPullRefAndCheckout(session, workspacePath, branchName); + logger.withTags({ pullRef: branchName }).info('Checked out GitHub pull ref'); + logger.debug('Successfully on branch'); + return branchName; + } + throw new Error( `Branch "${branchName}" not found in repository. Please ensure the branch exists remotely.` ); diff --git a/src/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref.ts b/src/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref.ts new file mode 100644 index 000000000..6321f70d4 --- /dev/null +++ b/src/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref.ts @@ -0,0 +1,42 @@ +export type PullRequestCheckoutRef = { + checkoutRef: string; + isForkPr: boolean; + headRepoFullName: string | null; +}; + +export type PullRequestCheckoutRefInput = { + pull_request: { + number: number; + head: { + ref: string; + repo?: { + full_name: string; + } | null; + }; + }; + repository: { + full_name: string; + }; +}; + +/** + * Resolve which git ref should be checked out for a PR review. + * + * - Same-repo PRs: use head.ref (e.g. "feature/my-change") + * - Fork PRs: use GitHub's synthetic pull ref (e.g. "refs/pull/123/head") + */ +export function resolvePullRequestCheckoutRef( + payload: PullRequestCheckoutRefInput +): PullRequestCheckoutRef { + const headRepoFullName = payload.pull_request.head.repo?.full_name ?? null; + const isForkPr = headRepoFullName !== null && headRepoFullName !== payload.repository.full_name; + const checkoutRef = isForkPr + ? `refs/pull/${payload.pull_request.number}/head` + : payload.pull_request.head.ref; + + return { + checkoutRef, + isForkPr, + headRepoFullName, + }; +} diff --git a/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts b/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts new file mode 100644 index 000000000..be1f9daca --- /dev/null +++ b/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts @@ -0,0 +1,65 @@ +import { resolvePullRequestCheckoutRef } from '@/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref'; + +describe('resolvePullRequestCheckoutRef', () => { + it('uses head.ref for same-repo PRs', () => { + const result = resolvePullRequestCheckoutRef({ + pull_request: { + number: 123, + head: { + ref: 'feature/same-repo', + repo: { full_name: 'acme/widgets' }, + }, + }, + repository: { + full_name: 'acme/widgets', + }, + }); + + expect(result).toEqual({ + checkoutRef: 'feature/same-repo', + isForkPr: false, + headRepoFullName: 'acme/widgets', + }); + }); + + it('uses refs/pull//head for fork PRs', () => { + const result = resolvePullRequestCheckoutRef({ + pull_request: { + number: 456, + head: { + ref: 'feature/fork-branch', + repo: { full_name: 'external/widgets-fork' }, + }, + }, + repository: { + full_name: 'acme/widgets', + }, + }); + + expect(result).toEqual({ + checkoutRef: 'refs/pull/456/head', + isForkPr: true, + headRepoFullName: 'external/widgets-fork', + }); + }); + + it('falls back to head.ref when head.repo is missing', () => { + const result = resolvePullRequestCheckoutRef({ + pull_request: { + number: 789, + head: { + ref: 'feature/missing-head-repo', + }, + }, + repository: { + full_name: 'acme/widgets', + }, + }); + + expect(result).toEqual({ + checkoutRef: 'feature/missing-head-repo', + isForkPr: false, + headRepoFullName: null, + }); + }); +}); diff --git a/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts b/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts index 68545d6c0..fb2328ef1 100644 --- a/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts +++ b/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts @@ -16,6 +16,7 @@ import { getBotUserId } from '@/lib/bot-users/bot-user-service'; import type { CodeReviewAgentConfig } from '@/lib/agent-config/core/types'; import { addReactionToPR } from '../adapter'; import { codeReviewWorkerClient } from '@/lib/code-reviews/client/code-review-worker-client'; +import { resolvePullRequestCheckoutRef } from './pull-request-checkout-ref'; /** * GitHub Pull Request Event Handler @@ -33,6 +34,8 @@ export async function handlePullRequestCodeReview( const { pull_request, repository } = payload; try { + const checkoutRef = resolvePullRequestCheckoutRef(payload); + logExceptInTest('Pull request event received:', { action: payload.action, pr_number: pull_request.number, @@ -40,6 +43,13 @@ export async function handlePullRequestCodeReview( title: pull_request.title, author: pull_request.user?.login, }); + logExceptInTest('Resolved pull request checkout ref:', { + pr_number: pull_request.number, + repo: repository.full_name, + isForkPr: checkoutRef.isForkPr, + headRepoFullName: checkoutRef.headRepoFullName, + checkoutRef: checkoutRef.checkoutRef, + }); // Skip draft PRs - only trigger code review for ready PRs if (pull_request.draft === true) { @@ -184,7 +194,7 @@ export async function handlePullRequestCodeReview( prAuthor: pull_request.user.login, prAuthorGithubId: String(pull_request.user.id), baseRef: pull_request.base.ref, - headRef: pull_request.head.ref, + headRef: checkoutRef.checkoutRef, headSha: pull_request.head.sha, platform: 'github', }); diff --git a/src/lib/integrations/platforms/github/webhook-schemas.ts b/src/lib/integrations/platforms/github/webhook-schemas.ts index 179358cf3..4c7b7fd8c 100644 --- a/src/lib/integrations/platforms/github/webhook-schemas.ts +++ b/src/lib/integrations/platforms/github/webhook-schemas.ts @@ -130,6 +130,12 @@ export const PullRequestPayloadSchema = z.object({ head: z.object({ sha: z.string(), ref: z.string(), + repo: z + .object({ + full_name: z.string(), + }) + .nullable() + .optional(), }), base: z.object({ sha: z.string(),