From 3121f87e87c261a9f67b4bf794a4912d874c4665 Mon Sep 17 00:00:00 2001 From: avallete Date: Mon, 7 Sep 2026 13:56:23 +0200 Subject: [PATCH] ci: generate large AI review diffs locally --- .github/ai-review/README.md | 13 ++- .../ai-review/generate-pr-diff.test.ts | 90 +++++++++++++++++ .github/scripts/ai-review/generate-pr-diff.ts | 99 +++++++++++++++++++ .github/workflows/ai-review.yml | 24 +++-- .github/workflows/github-scripts-ci.yml | 1 + 5 files changed, 210 insertions(+), 17 deletions(-) create mode 100644 .github/scripts/ai-review/generate-pr-diff.test.ts create mode 100644 .github/scripts/ai-review/generate-pr-diff.ts diff --git a/.github/ai-review/README.md b/.github/ai-review/README.md index 852d51ddb8..10f9848fa7 100644 --- a/.github/ai-review/README.md +++ b/.github/ai-review/README.md @@ -37,10 +37,9 @@ resolve ──────>┤ ├──> adjudicate ──> po models review agentically — reading the diff and the changed files via their own tools over many turns, like the local CLI — so PRs of any size are reviewed (very large diffs best-effort, within the model's context/turn - budget). One caveat: the diff is fetched with `gh pr diff`, which GitHub - itself caps (≈300 files / 20k lines / 1 MB); a PR beyond those limits gets - a truncated diff, so the review is truncated with it. Generating the diff - from the base/head refs instead is a possible follow-up. + budget). Each model job fetches the PR head and exact base branch into its + full-history trusted checkout, then generates the diff locally with + triple-dot merge-base semantics. This avoids GitHub's API diff-size limits. - **`claude-review`** and **`codex-review`** run **in parallel** — each gives its model an independent, exhaustive pass and produces structured JSON findings validated against `findings.schema.json`. Claude reads the PR's @@ -133,9 +132,9 @@ run 400s on the output schema because of this, drop `pattern`/`minItems` from instructions. The npm install of the Claude CLI runs with an isolated, pinned-registry npm config (`--userconfig /dev/null --globalconfig /dev/null --registry=...`) so a PR-supplied `.npmrc` cannot redirect it. - `codex-review` goes further and checks out no PR code at all — it works - purely from `pr.diff` and `claude-findings.json` under `/tmp`, both - regenerated from the GitHub API. Neither job can push, comment, or + `codex-review` goes further and checks out no PR worktree at all — it works + purely from `pr.diff` under `/tmp`, generated by trusted code from fetched + Git objects. Neither job can push, comment, or otherwise mutate anything. - **`bun` never runs with a cwd inside the untrusted `pr` checkout.** `bun` auto-loads `bunfig.toml` (whose `preload` runs arbitrary code) and `.env` diff --git a/.github/scripts/ai-review/generate-pr-diff.test.ts b/.github/scripts/ai-review/generate-pr-diff.test.ts new file mode 100644 index 0000000000..471b00691c --- /dev/null +++ b/.github/scripts/ai-review/generate-pr-diff.test.ts @@ -0,0 +1,90 @@ +import { execFileSync } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, describe, expect, test } from "bun:test"; + +import { generatePrDiff } from "./generate-pr-diff.ts"; + +const temporaryDirectories: string[] = []; + +function git(cwd: string, ...args: string[]): string { + return execFileSync("git", ["-c", "core.hooksPath=/dev/null", ...args], { + cwd, + encoding: "utf8", + }).trim(); +} + +function setupRepository(): { seed: string; checkout: string } { + const root = mkdtempSync(join(tmpdir(), "ai-review-diff-")); + temporaryDirectories.push(root); + const remote = join(root, "remote.git"); + const seed = join(root, "seed"); + const checkout = join(root, "checkout"); + + git(root, "init", "--bare", remote); + git(root, "init", "-b", "develop", seed); + git(seed, "config", "user.name", "AI Review Test"); + git(seed, "config", "user.email", "ai-review@example.test"); + writeFileSync(join(seed, "shared.txt"), "common\n"); + git(seed, "add", "shared.txt"); + git(seed, "commit", "-m", "common base"); + git(seed, "remote", "add", "origin", remote); + git(seed, "push", "-u", "origin", "develop"); + git(root, "clone", "--branch", "develop", remote, checkout); + return { seed, checkout }; +} + +function publishPullRequest(seed: string, prNumber: number): void { + git(seed, "push", "--force", "origin", `HEAD:refs/pull/${prNumber}/head`); +} + +afterEach(() => { + for (const directory of temporaryDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } +}); + +describe("generatePrDiff", () => { + test("writes a complete diff beyond GitHub's 20,000-line API limit", () => { + const { seed, checkout } = setupRepository(); + git(seed, "switch", "-c", "large-pr"); + const lines = Array.from({ length: 20_001 }, (_, index) => `added line ${index + 1}`); + writeFileSync(join(seed, "large.txt"), `${lines.join("\n")}\n`); + git(seed, "add", "large.txt"); + git(seed, "commit", "-m", "large change"); + publishPullRequest(seed, 42); + + const outputPath = join(checkout, "pr.diff"); + generatePrDiff({ repositoryPath: checkout, prNumber: 42, baseRef: "develop", outputPath }); + + const diff = readFileSync(outputPath, "utf8"); + expect(diff).toContain("+added line 1\n"); + expect(diff).toContain("+added line 20001\n"); + expect(diff.match(/^\+added line /gm)).toHaveLength(20_001); + }); + + test("uses the merge base when the base and PR branches diverge", () => { + const { seed, checkout } = setupRepository(); + git(seed, "switch", "-c", "feature"); + writeFileSync(join(seed, "shared.txt"), "feature\n"); + git(seed, "add", "shared.txt"); + git(seed, "commit", "-m", "feature change"); + publishPullRequest(seed, 77); + + git(seed, "switch", "develop"); + writeFileSync(join(seed, "base-only.txt"), "base progressed\n"); + git(seed, "add", "base-only.txt"); + git(seed, "commit", "-m", "base change"); + git(seed, "push", "origin", "develop"); + + const outputPath = join(checkout, "pr.diff"); + generatePrDiff({ repositoryPath: checkout, prNumber: 77, baseRef: "develop", outputPath }); + + const diff = readFileSync(outputPath, "utf8"); + expect(diff).toContain("diff --git a/shared.txt b/shared.txt"); + expect(diff).toContain("+feature"); + expect(diff).not.toContain("base-only.txt"); + }); +}); diff --git a/.github/scripts/ai-review/generate-pr-diff.ts b/.github/scripts/ai-review/generate-pr-diff.ts new file mode 100644 index 0000000000..dc6b84d54f --- /dev/null +++ b/.github/scripts/ai-review/generate-pr-diff.ts @@ -0,0 +1,99 @@ +import { spawnSync } from "node:child_process"; +import { closeSync, mkdirSync, openSync, renameSync, rmSync } from "node:fs"; +import { dirname, resolve } from "node:path"; + +export interface GeneratePrDiffOptions { + repositoryPath: string; + prNumber: number; + baseRef: string; + outputPath: string; +} + +function runGit(repositoryPath: string, args: string[]): void { + const result = spawnSync("git", args, { + cwd: repositoryPath, + encoding: "utf8", + }); + if (result.error) { + throw result.error; + } + if (result.status !== 0) { + throw new Error(`git ${args[0]} failed: ${result.stderr.trim()}`); + } +} + +function validateInputs(prNumber: number, baseRef: string): void { + if (!Number.isSafeInteger(prNumber) || prNumber <= 0) { + throw new Error(`Invalid PR number: ${prNumber}`); + } + if (baseRef.length === 0) { + throw new Error("Base ref must not be empty"); + } +} + +export function generatePrDiff(options: GeneratePrDiffOptions): void { + const repositoryPath = resolve(options.repositoryPath); + const outputPath = resolve(options.outputPath); + validateInputs(options.prNumber, options.baseRef); + runGit(repositoryPath, ["check-ref-format", `refs/heads/${options.baseRef}`]); + + const baseRef = "refs/ai-review/base"; + const headRef = "refs/ai-review/head"; + runGit(repositoryPath, [ + "fetch", + "--force", + "--no-tags", + "origin", + `+refs/heads/${options.baseRef}:${baseRef}`, + `+refs/pull/${options.prNumber}/head:${headRef}`, + ]); + + mkdirSync(dirname(outputPath), { recursive: true }); + const temporaryPath = `${outputPath}.${process.pid}.${crypto.randomUUID()}.tmp`; + const output = openSync(temporaryPath, "wx"); + try { + try { + const result = spawnSync( + "git", + ["diff", "--no-ext-diff", "--no-textconv", `${baseRef}...${headRef}`, "--"], + { + cwd: repositoryPath, + stdio: ["ignore", output, "pipe"], + encoding: "utf8", + }, + ); + if (result.error) { + throw result.error; + } + if (result.status !== 0) { + throw new Error(`git diff failed: ${result.stderr.trim()}`); + } + } finally { + closeSync(output); + } + renameSync(temporaryPath, outputPath); + } finally { + rmSync(temporaryPath, { force: true }); + } +} + +function parseArguments(args: string[]): GeneratePrDiffOptions { + if (args.length !== 2) { + throw new Error("Usage: generate-pr-diff.ts "); + } + return { + repositoryPath: process.cwd(), + prNumber: Number(args[0]), + baseRef: args[1] ?? "", + outputPath: "/tmp/ai-review/pr.diff", + }; +} + +if (import.meta.main) { + try { + generatePrDiff(parseArguments(process.argv.slice(2))); + } catch (error) { + console.error(error); + process.exit(1); + } +} diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml index 2c82cc1e7e..879b8fcf1f 100644 --- a/.github/workflows/ai-review.yml +++ b/.github/workflows/ai-review.yml @@ -144,6 +144,7 @@ jobs: with: ref: ${{ github.event.repository.default_branch }} path: trusted + fetch-depth: 0 persist-credentials: false - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 @@ -155,19 +156,18 @@ jobs: # must never be able to write to it. no-cache: true - - name: Fetch PR diff and metadata + - name: Generate PR diff and fetch metadata + working-directory: trusted env: GH_TOKEN: ${{ github.token }} PR: ${{ needs.resolve.outputs.pr_number }} run: | mkdir -p /tmp/ai-review - # `gh` infers the repo from the current directory's git remote, but this - # job checks out into `pr/` and `trusted/` subdirs, so $GITHUB_WORKSPACE - # itself is not a git repo — pass --repo explicitly. - gh pr diff "$PR" --repo "$GITHUB_REPOSITORY" > /tmp/ai-review/pr.diff gh pr view "$PR" --repo "$GITHUB_REPOSITORY" \ --json number,title,body,baseRefName,headRefName,additions,deletions,changedFiles \ > /tmp/ai-review/pr.json + base_ref=$(jq -r '.baseRefName' /tmp/ai-review/pr.json) + bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" # Pin the exact published version so a new Claude Code release can't # silently change review behavior mid-rollout; bump deliberately. @@ -299,6 +299,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: ref: ${{ github.event.repository.default_branch }} + fetch-depth: 0 persist-credentials: false - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 @@ -306,14 +307,14 @@ jobs: bun-version-file: ".bun-version" no-cache: true - - name: Fetch PR diff + - name: Generate PR diff env: GH_TOKEN: ${{ github.token }} PR: ${{ needs.resolve.outputs.pr_number }} run: | mkdir -p /tmp/ai-review - # Pass --repo explicitly so `gh` never depends on cwd being a git repo. - gh pr diff "$PR" --repo "$GITHUB_REPOSITORY" > /tmp/ai-review/pr.diff + base_ref=$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json baseRefName --jq '.baseRefName') + bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" - name: Prepare findings output schema run: | @@ -410,6 +411,7 @@ jobs: with: ref: ${{ github.event.repository.default_branch }} path: trusted + fetch-depth: 0 persist-credentials: false - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 @@ -455,13 +457,15 @@ jobs: > /tmp/ai-review/codex-findings.json fi - - name: Fetch PR diff + - name: Generate PR diff + working-directory: trusted env: GH_TOKEN: ${{ github.token }} PR: ${{ needs.resolve.outputs.pr_number }} run: | mkdir -p /tmp/ai-review - gh pr diff "$PR" --repo "$GITHUB_REPOSITORY" > /tmp/ai-review/pr.diff + base_ref=$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json baseRefName --jq '.baseRefName') + bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" - name: Prepare merged-review output schema working-directory: trusted diff --git a/.github/workflows/github-scripts-ci.yml b/.github/workflows/github-scripts-ci.yml index 237e718ab1..ada4560ee1 100644 --- a/.github/workflows/github-scripts-ci.yml +++ b/.github/workflows/github-scripts-ci.yml @@ -9,6 +9,7 @@ on: pull_request: paths: - ".github/scripts/**" + - ".github/workflows/ai-review.yml" - ".github/workflows/github-scripts-ci.yml" permissions: {}