From c8022b129e45298f3aae29cea2db94892fb5cfbe Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Tue, 28 Apr 2026 15:02:10 +0300 Subject: [PATCH 1/2] fix(code-review): use stable GitHub PR checkout refs --- .../triggers/prepare-review-payload.test.ts | 79 +++++++++++++++++++ .../triggers/prepare-review-payload.ts | 4 +- .../pull-request-checkout-ref.ts | 12 +-- .../pull-request-handler.test.ts | 17 ++-- 4 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts diff --git a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts new file mode 100644 index 0000000000..ede326dcd3 --- /dev/null +++ b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts @@ -0,0 +1,79 @@ +import { db } from '@/lib/drizzle'; +import type { CodeReviewAgentConfig } from '@/lib/agent-config/core/types'; +import { insertTestUser } from '@/tests/helpers/user.helper'; +import { cloud_agent_code_reviews, kilocode_users, type User } from '@kilocode/db/schema'; +import { eq } from 'drizzle-orm'; +import { prepareReviewPayload } from './prepare-review-payload'; + +const REPO = `test-org/prepare-review-payload-${Date.now()}`; + +const codeReviewConfig = { + review_style: 'balanced', + focus_areas: ['bugs'], + custom_instructions: null, + max_review_time_minutes: 20, + model_slug: 'anthropic/claude-sonnet-4.5', + repository_selection_mode: 'all', + gate_threshold: 'off', +} satisfies CodeReviewAgentConfig; + +describe('prepareReviewPayload', () => { + let testUser: User; + + beforeAll(async () => { + testUser = await insertTestUser(); + }); + + afterEach(async () => { + await db + .delete(cloud_agent_code_reviews) + .where(eq(cloud_agent_code_reviews.owned_by_user_id, testUser.id)); + }); + + afterAll(async () => { + await db.delete(kilocode_users).where(eq(kilocode_users.id, testUser.id)); + }); + + it('uses the stable GitHub pull ref for agent checkout when the stored head_ref is a branch name', async () => { + const prNumber = 1234; + const [review] = await db + .insert(cloud_agent_code_reviews) + .values({ + owned_by_user_id: testUser.id, + repo_full_name: REPO, + pr_number: prNumber, + pr_url: `https://github.com/${REPO}/pull/${prNumber}`, + pr_title: 'Test PR with deleted source branch', + pr_author: 'octocat', + base_ref: 'main', + head_ref: 'feature/deleted-after-merge', + head_sha: 'sha-current', + platform: 'github', + status: 'pending', + }) + .returning({ id: cloud_agent_code_reviews.id }); + + if (!review) { + throw new Error('Expected inserted review'); + } + + const payload = await prepareReviewPayload({ + reviewId: review.id, + owner: { + type: 'user', + id: testUser.id, + userId: testUser.id, + }, + agentConfig: { + config: codeReviewConfig, + }, + platform: 'github', + }); + + expect(payload.sessionInput).toMatchObject({ + githubRepo: REPO, + platform: 'github', + upstreamBranch: 'refs/pull/1234/head', + }); + }); +}); diff --git a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts index 95928653fa..1c977c3ee7 100644 --- a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -44,6 +44,7 @@ import type { CodeReviewAgentConfig } from '@/lib/agent-config/core/types'; import { logExceptInTest, errorExceptInTest } from '@/lib/utils.server'; import type { CodeReviewPlatform } from '../core/schemas'; import { PLATFORM } from '@/lib/integrations/core/constants'; +import { getGitHubPullRequestCheckoutRef } from '@/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref'; export type PreparePayloadParams = { reviewId: string; @@ -385,6 +386,7 @@ export async function prepareReviewPayload( // GitLab: uses gitUrl (full HTTPS URL) + gitToken const variant = config.thinking_effort ?? undefined; const gateThreshold = config.gate_threshold ?? 'off'; + const githubCheckoutRef = getGitHubPullRequestCheckoutRef(review.pr_number); const sessionInput: SessionInput = platform === PLATFORM.GITLAB ? { @@ -410,7 +412,7 @@ export async function prepareReviewPayload( mode: DEFAULT_CODE_REVIEW_MODE as 'code', model: config.model_slug || DEFAULT_CODE_REVIEW_MODEL, variant, - upstreamBranch: review.head_ref, + upstreamBranch: githubCheckoutRef, ...(gateThreshold !== 'off' ? { gateThreshold } : {}), }; diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref.ts index 6321f70d4e..05468aa478 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref.ts @@ -19,20 +19,22 @@ export type PullRequestCheckoutRefInput = { }; }; +export function getGitHubPullRequestCheckoutRef(prNumber: number): string { + return `refs/pull/${prNumber}/head`; +} + /** * 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") + * GitHub keeps refs/pull//head available even when the source + * branch is deleted after merge/close, so use it for both same-repo and fork PRs. */ 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; + const checkoutRef = getGitHubPullRequestCheckoutRef(payload.pull_request.number); return { checkoutRef, diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts index 916c2640fe..b1b7eaa202 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts @@ -1,8 +1,15 @@ -import { resolvePullRequestCheckoutRef } from '@/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref'; +import { + getGitHubPullRequestCheckoutRef, + resolvePullRequestCheckoutRef, +} from '@/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref'; import { shouldSkipSynchronizeForMergeCommit } from '@/lib/integrations/platforms/github/webhook-handlers/pull-request-handler'; describe('resolvePullRequestCheckoutRef', () => { - it('uses head.ref for same-repo PRs', () => { + it('builds GitHub synthetic pull refs', () => { + expect(getGitHubPullRequestCheckoutRef(123)).toBe('refs/pull/123/head'); + }); + + it('uses refs/pull//head for same-repo PRs', () => { const result = resolvePullRequestCheckoutRef({ pull_request: { number: 123, @@ -17,7 +24,7 @@ describe('resolvePullRequestCheckoutRef', () => { }); expect(result).toEqual({ - checkoutRef: 'feature/same-repo', + checkoutRef: 'refs/pull/123/head', isForkPr: false, headRepoFullName: 'acme/widgets', }); @@ -44,7 +51,7 @@ describe('resolvePullRequestCheckoutRef', () => { }); }); - it('falls back to head.ref when head.repo is missing', () => { + it('uses refs/pull//head when head.repo is missing', () => { const result = resolvePullRequestCheckoutRef({ pull_request: { number: 789, @@ -58,7 +65,7 @@ describe('resolvePullRequestCheckoutRef', () => { }); expect(result).toEqual({ - checkoutRef: 'feature/missing-head-repo', + checkoutRef: 'refs/pull/789/head', isForkPr: false, headRepoFullName: null, }); From ae457a77474e91a83df37b093df6230aa2d9d6d9 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Tue, 28 Apr 2026 16:40:43 +0300 Subject: [PATCH 2/2] fix(code-review): disable GitHub pull-ref session reuse --- .../triggers/prepare-review-payload.test.ts | 61 +++++++++++++++++++ .../triggers/prepare-review-payload.ts | 23 +++++-- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts index ede326dcd3..c4289d4b7f 100644 --- a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts +++ b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts @@ -76,4 +76,65 @@ describe('prepareReviewPayload', () => { upstreamBranch: 'refs/pull/1234/head', }); }); + + it('does not continue previous cloud-agent sessions for GitHub pull-ref reviews', async () => { + const prNumber = 1235; + const repo = `${REPO}-session-continuation`; + + await db.insert(cloud_agent_code_reviews).values({ + owned_by_user_id: testUser.id, + repo_full_name: repo, + pr_number: prNumber, + pr_url: `https://github.com/${repo}/pull/${prNumber}`, + pr_title: 'Previous completed review', + pr_author: 'octocat', + base_ref: 'main', + head_ref: 'feature/old-head', + head_sha: 'sha-previous', + platform: 'github', + status: 'completed', + session_id: 'previous-cloud-agent-session', + }); + + const [review] = await db + .insert(cloud_agent_code_reviews) + .values({ + owned_by_user_id: testUser.id, + repo_full_name: repo, + pr_number: prNumber, + pr_url: `https://github.com/${repo}/pull/${prNumber}`, + pr_title: 'Current review', + pr_author: 'octocat', + base_ref: 'main', + head_ref: 'feature/current-head', + head_sha: 'sha-current', + platform: 'github', + status: 'pending', + }) + .returning({ id: cloud_agent_code_reviews.id }); + + if (!review) { + throw new Error('Expected inserted review'); + } + + const payload = await prepareReviewPayload({ + reviewId: review.id, + owner: { + type: 'user', + id: testUser.id, + userId: testUser.id, + }, + agentConfig: { + config: codeReviewConfig, + }, + platform: 'github', + }); + + expect(payload.previousCloudAgentSessionId).toBeUndefined(); + expect(payload.sessionInput).toMatchObject({ + githubRepo: repo, + platform: 'github', + upstreamBranch: 'refs/pull/1235/head', + }); + }); }); diff --git a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts index 1c977c3ee7..77d676d292 100644 --- a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -310,9 +310,9 @@ export async function prepareReviewPayload( } } - // 4. Check for previous completed review (incremental review optimization) - // Both previousHeadSha (for diff base) and previousCloudAgentSessionId (for session - // continuation) are derived from the same review row to avoid mismatches. + // 4. Check for previous completed review (incremental review optimization). + // Keep previousHeadSha for prompt diff context, but disable GitHub session + // continuation because sendMessageV2 does not refetch refs/pull//head. let previousHeadSha: string | null = null; let previousCloudAgentSessionId: string | undefined; try { @@ -323,7 +323,21 @@ export async function prepareReviewPayload( platform ); previousHeadSha = previousReview?.head_sha ?? null; - previousCloudAgentSessionId = previousReview?.session_id ?? undefined; + + if (previousReview?.session_id) { + if (platform === PLATFORM.GITHUB) { + logExceptInTest( + '[prepareReviewPayload] Disabling GitHub session continuation for pull-ref checkout safety', + { + reviewId, + previousCloudAgentSessionId: previousReview.session_id, + upstreamBranch: getGitHubPullRequestCheckoutRef(review.pr_number), + } + ); + } else { + previousCloudAgentSessionId = previousReview.session_id; + } + } if (previousHeadSha) { logExceptInTest( @@ -332,6 +346,7 @@ export async function prepareReviewPayload( reviewId, previousHeadSha: previousHeadSha.substring(0, 8), currentHeadSha: review.head_sha.substring(0, 8), + previousSessionIdAvailable: !!previousReview?.session_id, previousCloudAgentSessionId, } );