Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/actions/claude-pr-review/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Family: [Claude CI](../CLAUDE-CI.md).
| Input | Required | Default | Description |
|---|---|---|---|
| `claude_code_oauth_token` | yes | | OAuth token for Claude Code. |
| `github_identity_token` | no | | Optional GitHub token used for review publication and state updates. When supplied, it is also used to resolve addressed automated review threads. Empty uses the job token and skips thread resolution. |
| `github_identity_token` | no | | Optional GitHub token used for review publication and state updates. When supplied, it also authenticates model analysis and resolves addressed automated review threads. Empty uses OIDC for model analysis and the job token for publication, and skips thread resolution. |
| `allowed_bots` | no | `mega-putin` | Bot accounts allowed to trigger Claude Code. |
| `extra_allowed_tools` | no | | Additional read-only tools to append to the canonical allowedTools list. |
| `extra_prompt` | no | | Additional analysis instructions appended after the canonical prompt. |
Expand Down
7 changes: 5 additions & 2 deletions .github/actions/claude-pr-review/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ inputs:
default: ""
description: >
Optional GitHub token used for review publication and state updates.
When supplied, it is also used to resolve addressed automated review
threads. Empty uses the job token and skips thread resolution.
When supplied, it also authenticates model analysis and resolves addressed
automated review threads. Empty uses OIDC for model analysis and the job
token for publication, and skips thread resolution.
allowed_bots:
required: false
default: mega-putin
Expand Down Expand Up @@ -213,6 +214,7 @@ runs:
uses: anthropics/claude-code-action@5ef2e550a465a721f4f45e4a7d3c340c873e1dcc # v1
with:
claude_code_oauth_token: ${{ inputs.claude_code_oauth_token }}
github_token: ${{ inputs.github_identity_token }}
allowed_bots: ${{ inputs.allowed_bots }}
display_report: false
# Always on. The review body says what the model concluded; only this
Expand Down Expand Up @@ -357,6 +359,7 @@ runs:
uses: anthropics/claude-code-action@5ef2e550a465a721f4f45e4a7d3c340c873e1dcc # v1
with:
claude_code_oauth_token: ${{ inputs.claude_code_oauth_token }}
github_token: ${{ inputs.github_identity_token }}
allowed_bots: ${{ inputs.allowed_bots }}
display_report: false
show_full_output: true
Expand Down
60 changes: 50 additions & 10 deletions .github/actions/claude-pr-review/review_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,53 @@ def prepare_trace(
return lines


def pull_request_diff(
repository: str, pull_request: int, base_sha: str, head_sha: str
) -> str:
try:
return run(
["gh", "pr", "diff", str(pull_request), "--repo", repository]
).stdout
except PipelineError as error:
if (
error.code != "GITHUB_API_FAILED"
or "HTTP 406" not in str(error)
or "maximum number of files" not in str(error)
):
raise

# GitHub's diff endpoint rejects PRs with more than 300 files. Resolve the
# frozen merge base through the compare API, then diff the complete trees.
# Fetching only those two snapshots also works with a depth-one checkout.
comparison = gh_json(
["api", f"repos/{repository}/compare/{base_sha}...{head_sha}"]
)
merge_base = str((comparison.get("merge_base_commit") or {}).get("sha") or "")
if not re.fullmatch(r"[0-9a-f]{40}", merge_base):
raise PipelineError("GitHub compare did not return a valid merge base")
missing = [
sha
for sha in (merge_base, head_sha)
if run(["git", "cat-file", "-e", f"{sha}^{{commit}}"], check=False).returncode
]
if missing:
# checkout may remove its credentials before the shared action runs.
# gh's helper reads GH_TOKEN from the environment; no token enters argv.
run(
[
"git", "-c", "credential.helper=",
"-c", "credential.helper=!gh auth git-credential",
"fetch", "--no-tags", "--depth=1", "origin", merge_base, head_sha,
]
)
return run(
[
"git", "diff", "--no-ext-diff", "--no-textconv", "--no-color",
"--binary", merge_base, head_sha, "--",
]
).stdout


def prepare(args: argparse.Namespace) -> None:

action_dir = Path(__file__).resolve().parent
Expand Down Expand Up @@ -1487,16 +1534,9 @@ def prepare(args: argparse.Namespace) -> None:
else:
run_premortem = mode == "full" or high_risk

full_diff = run(
[
"gh",
"pr",
"diff",
str(args.pull_request),
"--repo",
args.repository,
]
).stdout
full_diff = pull_request_diff(
args.repository, args.pull_request, expected_base, expected_head
)
require_expected_pull(
gh_json(
["api", f"repos/{args.repository}/pulls/{args.pull_request}"]
Expand Down
115 changes: 114 additions & 1 deletion .github/actions/claude-pr-review/test_review_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import copy
import json
import subprocess
import tempfile
import unittest
from pathlib import Path
Expand Down Expand Up @@ -220,6 +221,13 @@ def test_action_exposes_optional_github_identity_token(self):
)
# prepare (announce), publish, and report (retire on failure).
self.assertEqual(action.count(identity_env), 3)
model_steps = [
step for step in action.split("\n - name:")
if "uses: anthropics/claude-code-action@" in step
]
self.assertEqual(len(model_steps), 2)
for step in model_steps:
self.assertIn("github_token: ${{ inputs.github_identity_token }}", step)
self.assertIn(
"GH_RESOLVE_THREADS: "
"${{ inputs.github_identity_token != '' }}",
Expand Down Expand Up @@ -354,6 +362,112 @@ def test_failed_github_command_hides_verbose_arguments(self, run_mock):
"gh api graphql failed: permission denied",
)

@mock.patch.object(pipeline, "gh_json")
@mock.patch.object(pipeline, "run")
def test_large_pr_diff_uses_frozen_merge_base_and_head(self, run_mock, gh_mock):
base, head, merge_base = "a" * 40, "b" * 40, "c" * 40
gh_mock.return_value = {"merge_base_commit": {"sha": merge_base}}
run_mock.side_effect = [
pipeline.PipelineError(
"gh pr diff failed: HTTP 406: diff exceeded the maximum number of files (300)",
code="GITHUB_API_FAILED",
),
SimpleNamespace(returncode=1), # Missing merge base in a shallow checkout.
SimpleNamespace(returncode=1), # Missing PR head.
SimpleNamespace(stdout=""),
SimpleNamespace(stdout="complete diff including all files"),
]

result = pipeline.pull_request_diff("megaeth-labs/example", 7, base, head)

self.assertEqual(result, "complete diff including all files")
gh_mock.assert_called_once_with(
["api", f"repos/megaeth-labs/example/compare/{base}...{head}"]
)
fetch = run_mock.call_args_list[-2].args[0]
self.assertIn("fetch", fetch)
self.assertEqual(fetch[-2:], [merge_base, head])
self.assertEqual(
run_mock.call_args_list[-1].args[0],
["git", "diff", "--no-ext-diff", "--no-textconv", "--no-color", "--binary", merge_base, head, "--"],
)

@mock.patch.object(pipeline, "gh_json")
@mock.patch.object(pipeline, "run")
def test_normal_pr_diff_uses_github_without_fetching(self, run_mock, gh_mock):
run_mock.return_value = SimpleNamespace(stdout="github diff")
self.assertEqual(
pipeline.pull_request_diff("megaeth-labs/example", 7, "a" * 40, "b" * 40),
"github diff",
)
run_mock.assert_called_once()
gh_mock.assert_not_called()

@mock.patch.object(pipeline, "gh_json")
@mock.patch.object(pipeline, "run")
def test_diff_server_errors_are_not_hidden_by_git_fallback(self, run_mock, gh_mock):
failure = pipeline.PipelineError("gh pr diff failed: HTTP 500", code="GITHUB_API_FAILED")
run_mock.side_effect = failure
with self.assertRaises(pipeline.PipelineError) as context:
pipeline.pull_request_diff("megaeth-labs/example", 7, "a" * 40, "b" * 40)
self.assertIs(context.exception, failure)
run_mock.assert_called_once()
gh_mock.assert_not_called()

def test_large_pr_diff_is_complete_from_a_shallow_checkout(self):
real_run = subprocess.run
with tempfile.TemporaryDirectory() as directory:
source, checkout = Path(directory) / "source", Path(directory) / "checkout"
source.mkdir()

def git(*args):
return real_run(
["git", *args], cwd=source, text=True, capture_output=True, check=True
).stdout.strip()

git("init", "--initial-branch=base")
git("config", "user.name", "Test")
git("config", "user.email", "test@example.invalid")
names = [f"file-{index}.txt" for index in range(301)]
for name in names:
(source / name).write_text("before\n", encoding="utf-8")
(source / "binary.dat").write_bytes(b"\x00before")
git("add", "--", *names, "binary.dat")
git("commit", "-m", "merge base")
merge_base = git("rev-parse", "HEAD")
git("checkout", "-b", "head")
(source / names[0]).rename(source / "renamed.txt")
for name in names[1:]:
(source / name).write_text("after\n", encoding="utf-8")
(source / "binary.dat").write_bytes(b"\x00after")
git("add", "--", *names, "renamed.txt", "binary.dat")
git("commit", "-m", "large PR")
head = git("rev-parse", "HEAD")
git("checkout", "base")
(source / "base-only.txt").write_text("unrelated base change\n", encoding="utf-8")
git("add", "base-only.txt")
git("commit", "-m", "base advanced independently")
base = git("rev-parse", "HEAD")
git("clone", "--depth=1", "--branch=base", source.as_uri(), str(checkout))

def run_in_checkout(args, **kwargs):
if args[0] == "gh":
return subprocess.CompletedProcess(
args, 1, stdout="",
stderr="HTTP 406: diff exceeded the maximum number of files (300)",
)
return real_run(args, cwd=checkout, **kwargs)

with mock.patch.object(pipeline.subprocess, "run", side_effect=run_in_checkout), mock.patch.object(
pipeline, "gh_json", return_value={"merge_base_commit": {"sha": merge_base}}
):
result = pipeline.pull_request_diff("megaeth-labs/example", 7, base, head)

self.assertEqual(sum(line.startswith("diff --git ") for line in result.splitlines()), 302)
self.assertIn("rename from file-0.txt", result)
self.assertIn("GIT binary patch", result)
self.assertNotIn("base-only.txt", result)

@mock.patch("builtins.print")
def test_failure_report_is_human_readable(self, print_mock):
with tempfile.TemporaryDirectory() as directory:
Expand Down Expand Up @@ -2440,4 +2554,3 @@ def test_action_uploads_the_review_state_artifact(self):

if __name__ == "__main__":
unittest.main()

Loading