Skip to content

[DEVEX-1523] Make AI reviewer output deterministic and tighten safety gates#126

Merged
ronneseth merged 2 commits into
mainfrom
DEVEX-1523-ai-reviewer-deterministic-output
May 21, 2026
Merged

[DEVEX-1523] Make AI reviewer output deterministic and tighten safety gates#126
ronneseth merged 2 commits into
mainfrom
DEVEX-1523-ai-reviewer-deterministic-output

Conversation

@ronneseth
Copy link
Copy Markdown
Contributor

Summary

Reworks the shared Claude AI review workflow so the AI's output drives review actions and check status through a constrained, machine-readable contract rather than direct tool calls. Addresses the recent incident on encodium/manage#7507 where a stale parallel run approved a PR that another concurrent run had just deferred as too large.

Why

The previous design let the LLM judge size, decide concurrency, and call gh pr review --approve directly. Two runs on consecutive commits reached opposite conclusions on the same PR, and a stale --approve from the older run was attributed to the newer HEAD commit. Root causes: no concurrency control, LLM-judged size gate, no SHA pinning, and the LLM having direct approve authority.

What changed

  • Concurrency: cancel-in-progress per PR — new pushes cancel in-flight runs.
  • Size gate: deterministic shell step querying the Files API. Production lines + production files only (test files excluded by path/filename pattern). Default lowered from 1500 to 500 lines and 30 files. Lockfiles still count; happy to revisit.
  • No direct AI approvals: gh pr review and gh pr comment removed from Claude's allowed tools. Claude writes a constrained-schema verdict.json (approve | comment | defer) with required reasoning.
  • Apply-verdict step: validates the verdict, re-checks HEAD SHA hasn't moved, re-checks size, and downgrades approve → defer/comment if any safety guard trips. Posts the formal review with the AI's reasoning attached.
  • Slack notification (opt-in): callers can pass slack_channel; on auto-approve, the workflow posts a link to the PR plus reasoning via the Slack API (slack_bot_token secret).
  • Dedicated Check Run named AI Code Review published via the Check Runs API. Conclusion is deterministic from the verdict: approve → success, comment → neutral, defer/too-big/missing-verdict → skipped. Pinned to the head SHA at workflow start.
  • Prompt updates: tells the AI to skim test files and not hunt for bugs there; requires concrete reasoning naming specific files/behaviors/tests when approving; self-check rules downgrade vague approvals to defer.
  • Permissions: added checks: write for the Check Runs API.

Operator follow-ups (not in this PR)

  • Grant the rp-ai-reviewer token (PAT or App installation) Checks: read & write on consumer repos. Without it, the dedicated AI Code Review check won't publish (workflow will log a warning, other functionality unaffected).
  • Add AI_REVIEWER_SLACK_BOT_TOKEN org secret if any caller passes slack_channel. See the companion manage PR for the manage-side caller wiring and channel.
  • Optional: add AI Code Review to required status checks on protected branches for visibility.

Test plan

  • Open a small (<500 prod lines, <30 files) low-risk PR in a consumer repo and confirm Claude writes a verdict, the apply step posts an --approve review with reasoning, and the dedicated AI Code Review check shows success.
  • Open a PR that exceeds the size gate and confirm: workflow posts a deferral comment, dedicated check shows skipped, Claude action never runs (verify in logs).
  • Open a PR with intentional issues and confirm Claude posts inline comments, verdict file decision is comment, apply step posts a --comment review, dedicated check shows neutral.
  • Push a second commit while a review is in-flight and confirm the older run is cancelled (no late approval against the new HEAD).
  • (Manual) trigger a run with slack_channel set and slack_bot_token unset — confirm warning is logged and workflow completes without failure.

Made with Cursor

Reworks the shared Claude AI review workflow so the AI cannot
directly approve PRs and produces a deterministic verdict that the
workflow translates into review actions and a Check Run status.

Key changes:
- Concurrency: cancel-in-progress per PR, killing stale runs against
  older commits before they can post a late approval.
- Size gate moved out of the LLM into a deterministic shell step
  using the Files API. Default lowered to 500 lines / 30 files and
  test changes are excluded from the count.
- Claude no longer has access to gh pr review / gh pr comment.
  It writes a constrained-schema verdict.json (approve | comment |
  defer) plus reasoning. A follow-up step posts the formal review
  based on the verdict and re-checks HEAD SHA and current size
  before approving.
- Approval body always includes the AI's reasoning.
- Optional Slack notification on auto-approve (slack_channel input,
  slack_bot_token secret).
- Publishes a dedicated AI Code Review Check Run with conclusion
  derived deterministically from the verdict:
    approve -> success, comment -> neutral, defer/size -> skipped.
- Adds checks: write permission for the Check Runs API.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ronneseth ronneseth requested a review from a team as a code owner May 21, 2026 00:18
@ronneseth ronneseth requested a review from aspencer May 21, 2026 00:18
@cursor
Copy link
Copy Markdown

cursor Bot commented May 21, 2026

PR Summary

Medium Risk
Changes core CI/review automation and GitHub API interactions (reviews, check-runs, concurrency), so misconfiguration or logic bugs could incorrectly defer/approve or fail required checks. No production application code is touched.

Overview
Makes AI PR reviews deterministic and removes direct approval authority from the LLM. The workflow now enforces a pre-review size gate (production-only line/file counts; defaults lowered to 500 lines and 30 files, tests excluded via TEST_FILE_REGEX) and adds per-PR concurrency to cancel in-flight runs on new pushes.

The Claude step is constrained to writing a structured verdict.json and posting inline comments only; a new apply-verdict step validates the verdict, re-checks HEAD SHA/size before approving, and can downgrade approvals. Adds defense-in-depth to dismiss any unauthorized AI approvals, publishes a dedicated "AI Code Review" check run pinned to the starting SHA, and optionally posts a Slack notification on auto-approve.

Reviewed by Cursor Bugbot for commit 1242a30. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread .github/workflows/claude-review.yaml Outdated
Cursor bugbot flagged that retaining Bash(gh api:*) lets the AI POST
to /pulls/N/reviews with event=APPROVE, bypassing every safety gate
the previous commit added. Closes the gap with three layers:

1. Tool-runner enforcement (primary): narrows the allowedTools glob
   from Bash(gh api:*) to Bash(gh api:repos/*/pulls/*/comments*) so
   any path other than the comments endpoint is denied before the
   command runs. This blocks gh api -X POST .../reviews directly.

2. Server-side defense-in-depth: a new "Dismiss unauthorized AI
   approvals" step runs before Apply verdict and dismisses any
   APPROVED review by the reviewer bot on the current HEAD SHA.
   Filters by commit_id == HEAD_SHA_AT_START so stale approvals on
   older commits (already obsolete in GitHub's eyes) are left alone.
   Bot identity is derived via gh api user so the shared workflow
   isn't coupled to a specific bot username.

3. Prompt hard rule: explicit forbid on gh api -X POST/PUT/PATCH/
   DELETE/--method/-f/-F/--input, with note that any bypass will be
   dismissed by the workflow. Soft control, for documentation.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1242a30. Configure here.

Comment thread .github/workflows/claude-review.yaml
Comment thread .github/workflows/claude-review.yaml
@ronneseth ronneseth merged commit 8a2b003 into main May 21, 2026
2 checks passed
@ronneseth ronneseth deleted the DEVEX-1523-ai-reviewer-deterministic-output branch May 21, 2026 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant