Skip to content

Review bot: run only on /review - #961

Merged
dghosef merged 2 commits into
mainfrom
ci/review-bot-on-request-only
Oct 30, 2026
Merged

dghosef merged 2 commits into
mainfrom
ci/review-bot-on-request-only

Conversation

@dghosef

@dghosef dghosef commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator

The review bot no longer runs on its own. Opening, reopening or readying a PR, or pushing to it, never starts a review. It runs only when someone comments /review, /review thorough or /review continue, with the same permission checks as before.

Reviews are now on demand, so nothing is spent on PRs nobody asked to have reviewed.

Changes

  • claude-review.yml: the only trigger is issue_comment. The pull_request_target trigger is gone, and so is the ai-review-thorough label, which needed it. /review thorough does the same thing. The /review matching in the job's if, the preflight's write-access check, the author's self-serve /review, skip-ai-review, concurrency and permissions are unchanged.
  • Since every run is now a request, an out-of-credit refusal always gets the one-line reply. Before, an automatic run only logged a warning.
  • allow-unsafe-pr-checkout is removed from the PR checkout. That actions/checkout guard applies only to pull_request_target and workflow_run events, and the reviewer still only reads the PR's tree.
  • .github/review/README.md and CONTRIBUTING.md now say it runs only on /review, and the text about automatic runs and the label is gone.

A stacked PR whose base branch still has an old copy of the workflow can still get an automatic review from that copy until the base branch picks up this change, because pull_request_target runs the base branch's workflow file.

How it was checked

  • Triggers before: pull_request_target (opened, reopened, ready_for_review, labeled) and issue_comment (created). After: issue_comment (created) only.
  • yaml.safe_load parses the file, and actionlint reports nothing (run without shellcheck).
  • Not run on GitHub yet. The first /review after merge exercises it.

🤖 Generated with Claude Code

The review bot ran a light review automatically whenever a contributor
opened, reopened or readied a non-draft PR. Reviews now run only when
someone asks for one with `/review`, `/review thorough` or
`/review continue`, so the workflow's only trigger is issue_comment.

Dropping pull_request_target also drops the `ai-review-thorough` label
path; `/review thorough` does the same thing. With every run now a
request, an out-of-credit refusal always gets the one-line reply, and the
checkout no longer needs allow-unsafe-pr-checkout, whose guard applies
only to pull_request_target and workflow_run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mstar-review

mstar-review Bot commented Oct 29, 2026

Copy link
Copy Markdown

Automated review

🟡 1 P2 on aa611d3, light review

Outside the diff

  • [P2] .github/review/review-prompt.md:73: The light-mode text in the review prompt still says reviews start automatically when a PR is opened.

    After this PR the workflow's only trigger is issue_comment, so light mode no longer "runs once when a contributor opens or readies a non-draft PR". It runs only on each /review. This line now contradicts the workflow, the README and CONTRIBUTING.md, which the PR updated. The reviewer reads this file from main, so the stale sentence goes into every review prompt. Change it to something like "light runs on each /review."

Description

  • The Changes list says the docs were updated to describe a /review-only bot, but .github/review/review-prompt.md (light-mode paragraph) still describes the automatic review on open/ready and wasn't touched.
What the review checked
  • The workflow's on: now lists only issue_comment: created. No github.event.pull_request.*, labeled, draft or ai-review-thorough references are left in claude-review.yml, and the only remaining pull_request_target under .github is the unrelated pr-labels.yml.
  • The job-level if for /review, /review thorough and /review continue is unchanged, and so is the preflight's write-access check and self-serve check (commenter == issue author, contributor association, skip-ai-review).
  • concurrency.group, PR, AUTHOR and ASSOC now use the issue/comment fields alone, which is correct for an issue_comment-only trigger.
  • The THROUGH, the acknowledge step and the out-of-credit reply lost their event_name gating, and every run is now issue_comment, so behavior matches the description.
  • The handoff artifact filter now trusts only claude-review.yml:issue_comment runs, which matches the README and the header comment.
  • Removing allow-unsafe-pr-checkout is consistent with the description's claim that the guard covers only pull_request_target/workflow_run. The PR checkout still has persist-credentials: false and is only read.
  • README.md and CONTRIBUTING.md no longer mention the automatic review or the label. The cost-control and threat-model paragraphs match the new trigger.

Advisory, not a merge gate. Rules: AGENTS.md. Reply to discuss.

@mstar-review mstar-review Bot added the type/ci label Oct 29, 2026
@vasilevklart vasilevklart self-assigned this Oct 30, 2026

@vasilevklart vasilevklart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, i think it is nice change so we can just request on demand.. .github/review/review-prompt.md:73 still says “runs once when a contributor opens or readies a non-draft PR”, it should now just say it runs on each /review

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dghosef
dghosef merged commit 7757311 into main Oct 30, 2026
6 checks passed
@dghosef
dghosef deleted the ci/review-bot-on-request-only branch October 30, 2026 04:23
dghosef added a commit that referenced this pull request Oct 30, 2026
Brings in #810 (per-node dtype, YAML no longer overrides a model's autocast opt-out, implicit-downcast warning) and #961 (review bot runs only on /review).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dghosef added a commit that referenced this pull request Oct 30, 2026
…split-2-speech-language

Brings #858's deletion of NodeSubmodule.splits_heads in favour of
RaggedAttentionConfig.shard_over_sp, and origin/main's #810 (per-node dtype
and autocast), #961 and #931 (Rust loop-back fix). The compile-region
warm-up now reads the node's resolved dtype by name and names the node in
its autocast scope.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants