diff --git a/.github/review/README.md b/.github/review/README.md index 127da82fe6..26893721ba 100644 --- a/.github/review/README.md +++ b/.github/review/README.md @@ -1,34 +1,33 @@ # Automated PR review -An advisory LLM reviewer runs on contributors' non-draft PRs, including PRs from forks. It posts each finding as an inline comment, tagged P0-P3, plus a summary comment that covers the PR description and any open questions. The summary opens with a status line: 🔴 for any open P0 or P1, 🟡 for anything else to report, 🟢 for nothing, then the head commit and the mode. An inline finding stays open until its thread is resolved or a commit changes its lines. Each run, including each `/review`, posts a new summary comment at the bottom of the conversation and hides the earlier ones as outdated. It is not a merge gate and does not submit GitHub reviews. Each review also sets the PR's `type/*` labels, one to three of `new-model`, `bug`, `perf`, `feature`, `refactor`, `test`, `docs` and `ci`, replacing any earlier ones; the `area/*`, `model/*` and `size/*` labels come from [`../workflows/pr-labels.yml`](../workflows/pr-labels.yml). +An advisory LLM reviewer runs on a PR when someone asks for it with `/review`, including on PRs from forks. Opening a PR or pushing to it never starts a review. It posts each finding as an inline comment, tagged P0-P3, plus a summary comment that covers the PR description and any open questions. The summary opens with a status line: 🔴 for any open P0 or P1, 🟡 for anything else to report, 🟢 for nothing, then the head commit and the mode. An inline finding stays open until its thread is resolved or a commit changes its lines. Each run, including each `/review`, posts a new summary comment at the bottom of the conversation and hides the earlier ones as outdated. It is not a merge gate and does not submit GitHub reviews. Each review also sets the PR's `type/*` labels, one to three of `new-model`, `bug`, `perf`, `feature`, `refactor`, `test`, `docs` and `ci`, replacing any earlier ones; the `area/*`, `model/*` and `size/*` labels come from [`../workflows/pr-labels.yml`](../workflows/pr-labels.yml). -"Contributor" means the PR author's GitHub author association is `OWNER`, `MEMBER`, `COLLABORATOR` or `CONTRIBUTOR`: the team, plus anyone who has had a PR merged here. Their PRs are reviewed automatically, and they can comment `/review` on their own PR. Any other request (`/review` on someone else's PR, `/review thorough`, or the label) takes write access to this repo, which the workflow checks against the user's actual permission. Someone with write access can `/review` any PR, including a first-timer's. +"Contributor" means the PR author's GitHub author association is `OWNER`, `MEMBER`, `COLLABORATOR` or `CONTRIBUTOR`: the team, plus anyone who has had a PR merged here. They can comment `/review` on their own PR. Any other request (`/review` on someone else's PR, or `/review thorough`) takes write access to this repo, which the workflow checks against the user's actual permission. Someone with write access can `/review` any PR, including a first-timer's. | Trigger | Mode | | --- | --- | -| A contributor's non-draft PR is opened, reopened or marked ready | `light` (P0-P2, effort `high`), once | -| Someone with write access comments exactly `/review` on a PR | `light`, e.g. to re-review after pushes | +| Someone with write access comments exactly `/review` on a PR | `light` (P0-P2, effort `high`) | | A contributor comments exactly `/review` on their own PR, without the `skip-ai-review` label | `light` | -| Someone with write access comments exactly `/review thorough`, or applies the `ai-review-thorough` label | `thorough` (also P3 improvements and design questions; effort `xhigh`) | +| Someone with write access comments exactly `/review thorough` | `thorough` (also P3 improvements and design questions; effort `xhigh`) | | Anyone who may `/review` comments exactly `/review continue` after a review stopped part-way | continues that review in its mode, in a fresh session; a thorough one needs write access | -Pushes don't trigger a review on their own. The `skip-ai-review` label turns off the automatic run and the author's own `/review`; a `/review` from someone with write access still runs. +To re-review after pushes, comment `/review` again. The `skip-ai-review` label turns off the author's own `/review`; a `/review` or `/review thorough` from someone with write access still runs. The action refuses actors without write access, which is most contributors on a fork. For those runs alone the workflow passes the PR author's login, and nobody else's, as the action's `allowed_non_write_users`. -A stacked PR (one whose base is another feature branch) gets the automatic review only if its base branch has a current copy of the workflow, because `pull_request_target` runs the base branch's workflow file. Comment `/review` on stacked PRs; that always runs `main`'s. Either way the rules are read from `main`. +A comment always runs `main`'s copy of the workflow and reads the rules from `main`, whatever the PR's base, so stacked PRs are reviewed the same way. -The bot reacts 👀 to a `/review` comment when it picks it up. A review that doesn't complete (an API error, running out of turns, a timeout, a fork that no longer exists) leaves a PR comment linking the log, never a red check; a later failure updates that comment rather than adding another, and a later run that finishes rewrites it to point at its summary. A request the workflow declines (no write access and not the author, or no API key) exits green without commenting, and so does an automatic review the API refuses because the account is out of credit (with a warning in the log); a `/review` refused that way gets a one-line reply saying so. +The bot reacts 👀 to a `/review` comment when it picks it up. A review that doesn't complete (an API error, running out of turns, a timeout, a fork that no longer exists) leaves a PR comment linking the log, never a red check; a later failure updates that comment rather than adding another, and a later run that finishes rewrites it to point at its summary. A request the workflow declines (no write access and not the author, or no API key) exits green without commenting. A review the API refuses because the account is out of credit gets a one-line reply saying so (and a warning in the log) instead of a "didn't complete" comment. Running out of turns is handled in two ways. A review that finished but went past `--max-turns` (the action then reports a failure) still has its summary in the action's execution file, and the workflow posts it with a note. A review that stopped part-way gets a "didn't complete" comment listing the inline comments it made (the action posts them even when the run fails), and a handoff is saved as a workflow artifact for 7 days. The handoff ([`session.py`](session.py) `handoff`) is built from the action's execution file, or after a timeout from the session transcript. Its reliable part is what the workflow recovers itself: the inline comments the run made and whether each posted, the files it read, the changed files it never opened, and its latest messages, which usually say what it was doing. The prompt also asks the model to keep a progress note (candidate findings with their anchors, what it covered, what is left); that is best-effort, and adds the candidate findings when the model kept one. The notes list unverified leads, so they stay out of the PR conversation, though the artifact can be downloaded. -`/review continue` starts a fresh session with a fresh turn budget and gives it the handoff as `handoff.md`, rather than resuming the old session, which sends the whole old transcript again, probably after its prompt cache has expired: in a measured run, resuming after the cache expired cost about a quarter more than a fresh session, and while it is warm the two cost about the same. The fresh session knows only what the handoff says, so it rereads `AGENTS.md`, any skill its findings cite, and the code they anchor on; inline comments already on the PR are in `pr-comments.json` and in the handoff, and it posts only findings that have none. A continued run that stops too merges its handoff onto the one it started from. `/review continue` refuses, in a comment of its own that leaves the stopped review's comment as it was, if there is no saved handoff, if a later run already finished (any run that finishes marks the stopped review's comment resolved), if the PR has new commits since (comment `/review` then), or if a thorough review is continued without write access. Only artifacts made by this workflow on `issue_comment` or `pull_request_target` are used: a fork's own `pull_request` workflow can upload an artifact with any name. +`/review continue` starts a fresh session with a fresh turn budget and gives it the handoff as `handoff.md`, rather than resuming the old session, which sends the whole old transcript again, probably after its prompt cache has expired: in a measured run, resuming after the cache expired cost about a quarter more than a fresh session, and while it is warm the two cost about the same. The fresh session knows only what the handoff says, so it rereads `AGENTS.md`, any skill its findings cite, and the code they anchor on; inline comments already on the PR are in `pr-comments.json` and in the handoff, and it posts only findings that have none. A continued run that stops too merges its handoff onto the one it started from. `/review continue` refuses, in a comment of its own that leaves the stopped review's comment as it was, if there is no saved handoff, if a later run already finished (any run that finishes marks the stopped review's comment resolved), if the PR has new commits since (comment `/review` then), or if a thorough review is continued without write access. Only artifacts made by this workflow on `issue_comment` are used: a fork's own `pull_request` workflow can upload an artifact with any name. | File | Role | | --- | --- | | [`../../AGENTS.md`](../../AGENTS.md) | **The substance.** Numbered invariants the reviewer checks and cites. Also read by contributors' own coding agents. | | [`review-prompt.md`](review-prompt.md) | How the reviewer behaves: trust boundary, path routing, reporting bar, output format. | -| [`../workflows/claude-review.yml`](../workflows/claude-review.yml) | The workflow, and the safety rules that come with `pull_request_target`. | +| [`../workflows/claude-review.yml`](../workflows/claude-review.yml) | The workflow, and the safety rules that come with running on `issue_comment`. | | [`render_summary.py`](render_summary.py) | The summary's fields (its `--schema`), and the comment they render to. | | [`session.py`](session.py) | Reads a run's execution file and transcripts: the usage record, and the handoff for `/review continue`. | | [`replay.sh`](replay.sh) | Run the reviewer locally against a past PR, to tune before trusting it. | @@ -43,7 +42,7 @@ Procedural knowledge — how to run a benchmark, how to debug a hang — belongs ## One-time setup 1. Add `ANTHROPIC_API_KEY` as an **organization** secret on `mstar-project` (Settings → Secrets and variables → Actions), scoped to this repository. Bill it to a project account rather than a personal one so usage is attributable. -2. Create a `skip-ai-review` label (applying it to a PR skips the automatic review) and an `ai-review-thorough` label (applying it runs one thorough review). +2. Create a `skip-ai-review` label (applying it to a PR stops its author's own `/review`). 3. Give the bot its own name and logo with the `mstar-review` GitHub App, owned by `mstar-project` and installed on this repository only, with contents read and issues and pull requests write. Add its client ID as the repository variable `REVIEW_APP_CLIENT_ID` and a private key as the secret `REVIEW_APP_PRIVATE_KEY`. This workflow and [`../workflows/pr-labels.yml`](../workflows/pr-labels.yml) post with the app's token, and with the job's own token as `github-actions[bot]` until both are set. 4. Nothing else. The workflow is advisory, so it needs no branch-protection change. @@ -78,10 +77,10 @@ Even then, prefer keeping it advisory: a non-deterministic check shouldn't have ## Cost control -Current levers, roughly in order of effect: only contributors' PRs are reviewed automatically, once per PR rather than per push; re-reviews are on request with `/review`, which takes write access or authorship of the PR (light mode only for authors, and `skip-ai-review` stops them); draft PRs are skipped; `concurrency` cancels a run superseded by a newer one on the same PR; in light mode, diffs over about 4,000 lines are triaged rather than covered in full; `--max-turns 40` caps a runaway session (`/review continue` grants another 40; the `CLAUDE_REVIEW_MAX_TURNS` repository variable overrides it). The model is pinned to Opus in the workflow, at effort `high` for light reviews and `xhigh` for thorough ones (Claude Code's default without `--effort` is medium); lowering light to `medium` is the next cost lever. Drop to Sonnet if cost matters more than recall and replay shows it catching the same defects. +Current levers, roughly in order of effect: nothing runs automatically, only on request with `/review`, which takes write access or authorship of the PR (light mode only for authors, and `skip-ai-review` stops them); `concurrency` cancels a run superseded by a newer one on the same PR; in light mode, diffs over about 4,000 lines are triaged rather than covered in full; `--max-turns 40` caps a runaway session (`/review continue` grants another 40; the `CLAUDE_REVIEW_MAX_TURNS` repository variable overrides it). The model is pinned to Opus in the workflow, at effort `high` for light reviews and `xhigh` for thorough ones (Claude Code's default without `--effort` is medium); lowering light to `medium` is the next cost lever. Drop to Sonnet if cost matters more than recall and replay shows it catching the same defects. Every run that reaches Claude records what it used: API calls (one per API response in the transcript, however many lines it spans), uncached input, cache-write, cache-read and output tokens (per model from the run's result, which also counts calls the transcript doesn't hold, or summed from the transcript when a timeout leaves no result), turns and cost, with the mode, whether it was a `/review continue`, the PR and the head commit. It goes in the run's job summary as a table and in a `review-usage---` artifact kept for 30 days, never on the PR. Compare runs with `gh run download -n review-usage---`. ## Threat model -`pull_request_target` and `issue_comment` run with secrets on PRs from forks, so the PR's contents and comments are untrusted input. A comment only selects the mode, by exact match in the job's `if`; its text never reaches a shell or the prompt. The workflow never executes PR code, checks the PR out only into `pr-head/`, reads its rules from the base tree, gives the model no shell and no mutation or network tools, redacts credential-shaped strings from everything it posts, and holds write permission on pull requests alone. The residual risk is prompt injection producing a misleading comment: annoying, not dangerous. A contributor's run on their own PR reads exactly the content a maintainer's `/review` of that PR would, with the same tools and token, so it adds cost exposure (bounded by the rules above and one run at a time per PR), not new capability. A saved handoff (for `/review continue`) is a public artifact holding the model's notes on the PR's content, redacted like everything else the bot posts; a continued run trusts only artifacts from this workflow's own `issue_comment` and `pull_request_target` runs, checks the handoff's PR, head commit and mode, and gives the model its notes as untrusted input. Preserve the rules listed in the workflow header when editing it. +`issue_comment` runs with secrets on PRs from forks, so the PR's contents and comments are untrusted input. A comment only selects the mode, by exact match in the job's `if`; its text never reaches a shell or the prompt. The workflow never executes PR code, checks the PR out only into `pr-head/`, reads its rules from the base tree, gives the model no shell and no mutation or network tools, redacts credential-shaped strings from everything it posts, and holds write permission on pull requests alone. The residual risk is prompt injection producing a misleading comment: annoying, not dangerous. A contributor's run on their own PR reads exactly the content a maintainer's `/review` of that PR would, with the same tools and token, so it adds cost exposure (bounded by the rules above and one run at a time per PR), not new capability. A saved handoff (for `/review continue`) is a public artifact holding the model's notes on the PR's content, redacted like everything else the bot posts; a continued run trusts only artifacts from this workflow's own `issue_comment` runs, checks the handoff's PR, head commit and mode, and gives the model its notes as untrusted input. Preserve the rules listed in the workflow header when editing it. diff --git a/.github/review/review-prompt.md b/.github/review/review-prompt.md index 331e5a6e8f..c28965c341 100644 --- a/.github/review/review-prompt.md +++ b/.github/review/review-prompt.md @@ -70,7 +70,7 @@ Ask a question when its answer decides whether something is a defect and the cod ## Modes -**light** runs once when a contributor opens or readies a non-draft PR, and again on each `/review`. Report P0 to P2, description problems, and only the questions that could turn into a P0 or P1. On a diff over about 4,000 lines, review the files most likely to hold a defect and list the ones you skipped in `skipped`. +**light** runs on each `/review`. Report P0 to P2, description problems, and only the questions that could turn into a P0 or P1. On a diff over about 4,000 lines, review the files most likely to hold a defect and list the ones you skipped in `skipped`. **thorough** runs on request. Report everything, including P3 and design questions. Follow the change across layers, and cover the whole diff. diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 29cdb274e4..ddf3189f3c 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -3,12 +3,11 @@ name: Claude Review # Advisory LLM review of PRs. Not a merge gate. Rules live in AGENTS.md; # the reviewer's operating instructions are in .github/review/review-prompt.md. # -# Why pull_request_target: a plain `pull_request` trigger withholds secrets from -# fork PRs, so the reviewer could never review a fork's code. pull_request_target -# runs with secrets, and uses -# the workflow file from the PR's *base branch*, never the PR's version. For a PR -# into main that is main's; for a stacked PR it is the base branch's copy, which -# may be old or missing (then use /review, which always runs main's). +# Why issue_comment: a plain `pull_request` trigger withholds secrets from fork +# PRs, so the reviewer could never review a fork's code. issue_comment runs with +# secrets, and always uses the default branch's copy of this workflow file, never +# the PR's version, whatever the PR's base. The comment body is only matched by +# the `if` below, never passed to a shell. # # The safety rules that come with that, and that must hold for any edit here: # @@ -29,33 +28,25 @@ name: Claude Review # 5. Third-party actions are pinned to a commit SHA, since they run with the # API key. Bump them deliberately. -# When it runs, and who can cause it: -# - light, once, when a contributor's non-draft PR is opened, reopened or -# readied. "Contributor" is the PR author's author_association: OWNER, -# MEMBER, COLLABORATOR or CONTRIBUTOR, i.e. the team plus anyone who has had -# a PR merged here, from a fork or not. First-time contributors and -# strangers get no automatic review (cost, spam). -# - light, when someone with write access comments `/review` on a PR -# (re-review after pushes), or when a contributor comments `/review` on -# their own PR, unless it has the `skip-ai-review` label; -# - thorough, when someone with write access comments `/review thorough` or -# applies the `ai-review-thorough` label. +# When it runs, and who can cause it. Only on request; opening or pushing to +# a PR never starts a review. +# - light, when someone with write access comments `/review` on a PR, or +# when a contributor comments `/review` on their own PR, unless it has the +# `skip-ai-review` label. "Contributor" is the commenter's +# author_association: OWNER, MEMBER, COLLABORATOR or CONTRIBUTOR, i.e. the +# team plus anyone who has had a PR merged here, from a fork or not; +# - thorough, when someone with write access comments `/review thorough`. # Write access (write, maintain or admin) is checked against the user's actual # permission in the preflight; author_association can't tell write from read. # The PR can be from anyone, so a maintainer can /review a first-timer's PR. -# Pushes don't trigger a review on their own; comment /review for that. # # The action refuses to run for an actor without write access unless that # actor is listed in allowed_non_write_users. Most contributors' PRs come from # forks without write access, so the preflight lists exactly one login there, # and only when the actor is the PR's author, is a contributor, and the run is -# the automatic review or their own light `/review`. Never '*'. Such a run -# reads the same untrusted PR content as a maintainer's /review of that PR, -# with the same tools and token; only who can start it changes. -# -# issue_comment also runs with secrets and the default branch's workflow file, -# so the same rules hold. The comment body is only matched by the `if` below, -# never passed to a shell. +# their own light `/review`. Never '*'. Such a run reads the same untrusted PR +# content as a maintainer's /review of that PR, with the same tools and token; +# only who can start it changes. # # Failures never turn the PR's check red: every step after the preflight is # continue-on-error and gated on the one before it, and if the review didn't @@ -73,16 +64,13 @@ name: Claude Review # `/review continue` starts a fresh session from that handoff with a fresh turn # budget, rather than resuming the old session, which would replay the whole # transcript, usually after its prompt cache has expired. Only artifacts made -# by this workflow on issue_comment or pull_request_target are trusted, since a -# fork's own `pull_request` workflow can upload an artifact with any name. +# by this workflow on issue_comment are trusted, since a fork's own +# `pull_request` workflow can upload an artifact with any name. # # Every run that reaches Claude records its API calls, tokens, turns and cost # in the job summary and a review-usage-* artifact, never on the PR. on: - pull_request_target: - # `labeled` is for on-demand thorough reviews; other labels are filtered out below. - types: [opened, reopened, ready_for_review, labeled] issue_comment: types: [created] @@ -96,16 +84,8 @@ jobs: # rather than a silently skipped job. It also means a fork that *does* set # the key gets reviews, which is how you test changes to this workflow. # - # For /review and the label this only filters cheaply; the preflight's - # permission check decides. + # This only filters cheaply; the preflight's permission check decides. if: >- - (github.event_name == 'pull_request_target' && - !github.event.pull_request.draft && - !contains(github.event.pull_request.labels.*.name, 'skip-ai-review') && - ((github.event.action == 'labeled' && github.event.label.name == 'ai-review-thorough') || - (github.event.action != 'labeled' && - contains(fromJSON('["OWNER","MEMBER","COLLABORATOR","CONTRIBUTOR"]'), github.event.pull_request.author_association)))) - || (github.event_name == 'issue_comment' && github.event.issue.pull_request && (((github.event.comment.body == '/review' || github.event.comment.body == '/review thorough' || @@ -118,7 +98,7 @@ jobs: # Job-level, so an unrelated PR comment (which still starts a workflow run # that this `if` skips) can never cancel a review in progress. concurrency: - group: claude-review-${{ github.event.pull_request.number || github.event.issue.number }} + group: claude-review-${{ github.event.issue.number }} cancel-in-progress: true runs-on: ubuntu-latest timeout-minutes: 50 @@ -130,7 +110,7 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} REPO: ${{ github.repository }} - PR: ${{ github.event.pull_request.number || github.event.issue.number }} + PR: ${{ github.event.issue.number }} # Credential-shaped strings, removed (sed -E "$REDACT") from everything # the workflow posts or saves. REDACT: | @@ -142,8 +122,8 @@ jobs: # Secrets can't be tested in a job-level `if`, so short-circuit here # instead. Lets this land before ANTHROPIC_API_KEY exists without # failing a check on every PR. Then the actor needs write access, or must - # be the PR's author, a contributor, on the automatic review or a light - # `/review` (see the header). Anyone else exits green with a notice. + # be the PR's author, a contributor, on a light `/review` (see the + # header). Anyone else exits green with a notice. # non_write_user is set only for that author, and only when they lack # write access, so a maintainer's run is unchanged. - name: Preflight @@ -151,15 +131,13 @@ jobs: env: KEY_SET: ${{ secrets.ANTHROPIC_API_KEY != '' }} ACTOR: ${{ github.actor }} - AUTHOR: ${{ github.event.pull_request.user.login || github.event.issue.user.login }} - # The PR author's association on the automatic run; the commenter's on - # /review, where SELF_SERVE also requires commenter == author. - ASSOC: ${{ github.event.comment.author_association || github.event.pull_request.author_association }} + AUTHOR: ${{ github.event.issue.user.login }} + # The commenter's association; SELF_SERVE also requires commenter == author. + ASSOC: ${{ github.event.comment.author_association }} SELF_SERVE: >- - ${{ (github.event_name == 'pull_request_target' && github.event.action != 'labeled') || - (github.event_name == 'issue_comment' && - (github.event.comment.body == '/review' || github.event.comment.body == '/review continue') && - !contains(github.event.issue.labels.*.name, 'skip-ai-review')) }} + ${{ github.event_name == 'issue_comment' && + (github.event.comment.body == '/review' || github.event.comment.body == '/review continue') && + !contains(github.event.issue.labels.*.name, 'skip-ai-review') }} run: | if [ "$KEY_SET" != "true" ]; then echo "::warning::ANTHROPIC_API_KEY is not set; skipping the review." @@ -206,22 +184,21 @@ jobs: # Tell the commenter their /review was picked up; a comment-triggered run # is otherwise invisible from the PR. - name: Acknowledge the /review comment - if: steps.pre.outputs.run == 'true' && github.event_name == 'issue_comment' + if: steps.pre.outputs.run == 'true' continue-on-error: true env: GH_TOKEN: ${{ steps.app.outputs.token || secrets.GITHUB_TOKEN }} COMMENT_ID: ${{ github.event.comment.id }} run: gh api -X POST "repos/$REPO/issues/comments/$COMMENT_ID/reactions" -f content=eyes > /dev/null - # The PR's refs and title, the same way for both triggers (an - # issue_comment event doesn't carry the PR's head or base). The title is - # untrusted; it only ever reaches the prompt. + # The PR's refs and title (an issue_comment event doesn't carry the PR's + # head or base). The title is untrusted; it only ever reaches the prompt. - name: Resolve the PR if: steps.pre.outputs.run == 'true' id: pr continue-on-error: true env: - THOROUGH: ${{ (github.event.action == 'labeled' || github.event.comment.body == '/review thorough') && 'true' || 'false' }} + THOROUGH: ${{ github.event.comment.body == '/review thorough' && 'true' || 'false' }} CONTINUE: ${{ github.event.comment.body == '/review continue' && 'true' || 'false' }} run: | gh api "repos/$REPO/pulls/$PR" > pr.json @@ -275,7 +252,7 @@ jobs: [.artifacts[] | select(.expired | not)] | sort_by(.created_at) | reverse | .[].workflow_run.id'); do read -r path event < <(gh api "repos/$REPO/actions/runs/$run_id" --jq '"\(.path) \(.event)"') || continue case "$path:$event" in - .github/workflows/claude-review.yml*:issue_comment|.github/workflows/claude-review.yml*:pull_request_target) + .github/workflows/claude-review.yml*:issue_comment) found=$run_id; break ;; esac done @@ -301,10 +278,8 @@ jobs: echo "mode=$mode"; echo "effort=$effort"; echo "timeout=$timeout" } >> "$GITHUB_OUTPUT" - # Trusted: main's tip, pinned by name. Without a `ref`, pull_request_target - # would check out the PR's base branch, which for a stacked PR is another - # feature branch, and the PR's base.sha is where main was when the PR was - # opened. Either way the PR would be judged by stale rules. + # Trusted: main's tip, pinned by name, never the PR's base branch or + # base.sha, which for a stacked or old PR would be stale rules. # AGENTS.md and the review prompt come from here. - name: Check out main if: steps.pr.outcome == 'success' && steps.prev.outcome != 'failure' @@ -327,11 +302,6 @@ jobs: ref: ${{ steps.pr.outputs.head_sha }} path: pr-head persist-credentials: false - # actions/checkout refuses a fork's code under pull_request_target - # unless told otherwise (a July 2026 change, backported to v4). Safe - # here because of rule 1 above: pr-head/ is only read, never built or - # executed, and holds no credentials. - allow-unsafe-pr-checkout: true # The body and all comments are untrusted; the prompt says so. - name: Collect the diff, description and existing comments @@ -434,25 +404,22 @@ jobs: continue-on-error: true env: GH_TOKEN: ${{ steps.app.outputs.token || secrets.GITHUB_TOKEN }} - EVENT: ${{ github.event_name }} run: | f="$RUNNER_TEMP/claude-execution-output.json" if [ ! -s "$f" ]; then echo "why=it stopped before Claude Code returned (a timeout or a setup error)" >> "$GITHUB_OUTPUT" exit 0 fi - # Out of API credit: an automatic review skips with a warning in the - # log, rather than leave a "didn't complete" comment on every PR until - # it's topped up. A /review someone asked for gets a one-line answer. + # Out of API credit: a one-line answer instead of a "didn't complete" + # comment, since there's nothing to retry or continue until it's + # topped up. if jq -e 'any(.[]; .type == "assistant" and (.error == "billing_error" or ([.message.content[]? | select(.type == "text") | .text] | join("") | startswith("Credit balance is too low"))))' \ "$f" > /dev/null; then echo "::warning::The Anthropic API account is out of credit; skipping the review." echo "billing=true" >> "$GITHUB_OUTPUT" - if [ "$EVENT" = "issue_comment" ]; then - gh pr comment "$PR" --repo "$REPO" \ - --body "The review didn't run: the API account for the review bot is out of credit." > /dev/null - fi + gh pr comment "$PR" --repo "$REPO" \ + --body "The review didn't run: the API account for the review bot is out of credit." > /dev/null exit 0 fi res=$(jq -c '[.[] | select(.type == "result")] | last // {}' "$f") diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3424402f21..8e886f1e05 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,7 +20,7 @@ General questions are welcome too — a plain issue or an email to