[PR #340] [MERGED] feat(cursor-review): opt-in auto-approve by severity threshold #341

New issue

Closed

opened 2026-10-26 22:38:44 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:38:44 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/340
Author: @mattmillerai
Created: 10/6/2026
Status: ✅ Merged
Merged: 10/6/2026
Merged by: @mattmillerai

Base: main ← Head: matt/cursor-review-auto-approve

📝 Commits (3)

5395890 feat(cursor-review): opt-in auto-approve by severity threshold
87c4541 fix(cursor-review): resolve coderabbit review comments
6753b2e fix(cursor-review): address the auto-approve panel findings and CI

📊 Changes

9 files changed (+961 additions, -4 deletions)

View changed files

📝 .github/cursor-review/README.md (+2 -0)
➕ .github/cursor-review/auto-approve.py (+427 -0)
📝 .github/cursor-review/gate-unresolved.py (+3 -0)
➕ .github/cursor-review/tests/test_auto_approve.py (+236 -0)
📝 .github/cursor-review/tests/test_workflow_job_isolation.py (+1 -1)
📝 .github/cursor-review/tests/test_workflow_ref_guard_parity.py (+1 -1)
📝 .github/workflow-pins/tests/test_check_workflow_pins.py (+5 -2)
📝 .github/workflows/cursor-review.yml (+204 -0)
📝 docs/callers/cursor-review.md (+82 -0)

📄 Description

ELI-5

The Cursor review bot can now approve a PR by itself, if a repo turns that on. Each repo picks how bad a finding can be and still get approved (medium, low or nit). Anything worse gets "request changes" instead. If anything about the review looks unreliable, the bot does nothing and takes back any approval it gave earlier. When new commits are pushed, it also takes its earlier approval back.

What changed

New input approve_max_severity (default '' = off) and optional secret APPROVER_TOKEN.
.github/cursor-review/auto-approve.py
- decide runs as a new last step of post-review, the job that already holds the write token and checks out no PR code.
- It approves, pinned to the reviewed commit, when every finding is at or below the threshold.
- It requests changes when any finding is above the threshold or has an unrecognised severity.
- It does nothing if the judge didn't adjudicate, any panel reviewer didn't complete, the review didn't land as threads (or a finding landed in the body only), the head moved, the PR state couldn't be read, or an earlier thread above the threshold is still open. A do-nothing round also withdraws the bot's own earlier approvals.
- The step is continue-on-error, so a refused approval can't make the blocking gate read post-review as "review did not land".
- Model-supplied severity/path/line in its review body are sanitised with post-review.py's render_code_ref.
New job dismiss-stale-approval: on synchronize, it dismisses this identity's own auto-approve approvals that aren't on the new head. Rulesets with dismiss_stale_reviews_on_push: false would otherwise keep a stale approval valid. A request-changes is left alone: a push starts no new round under a label-triggered caller, so only the next round replaces it.
gate-unresolved.py's thread query now also fetches the first comment body, which is where the severity badge lives.
Docs: a new Auto-approve section in docs/callers/cursor-review.md, plus an input row and a script-table row in the README.

Trust model: the decision rests on model output over the PR's own diff, and for pull_request events the input is read from the caller workflow at the PR head. So enabling it makes the approval no stronger than push access. The guide's new Trust model section says this explicitly. Whether to ship with that posture is the owner's call; the two review threads that raised it are still open.

Who approves: only APPROVER_TOKEN's user. There is no fallback to the bot_app_id App or github-actions[bot], and nothing is submitted if the token is unset. Use a dedicated machine user in a code-owner team (on cloud, comfy-standard-approver) with a token scoped to that one repo. The dismiss job holds only that token, and its job token is read-only.

Security (documented): a prompt-injected PR can get itself approved, and anyone with push access can enable the input from their own PR or read the token. Both are accepted risks for this feature.

Testing

tests/test_auto_approve.py covers every rule: threshold validation, approve, request changes, unknown severity, degraded judge, incomplete panel, undelivered or body-only review, moved head, unreadable state, threshold-derived prior-thread gate, withdrawal on an untrusted round, the approvals-only dismissal filter, and markdown sanitising.
Existing tests updated for the new job: the cursor-review guard-parity count goes from 8 to 9, the bot-key-holders set gains the checkout-free job, and the workflow-pins guarded-checkout count goes from 18 to 19.
Not yet tested live. Callers are unaffected until they set the input.

Provenance

Authored by: agent-work loop
Verified:

python3 -m unittest discover for .github/cursor-review/tests (748 OK) and .github/workflow-pins/tests (502 OK, 29 skipped) at the current head
check_workflow_pins.py OK. check-org-repo-literals.sh OK. check_agents_md.py passes with 2 warnings.
CI green on the current head (all 8 checks pass; no required checks configured). Merged with current main locally: the same suites and lints still pass.

Deviations:

The prompt-injection and PR-editable-input findings are documented, not fixed in code. No out-of-model signal exists, so the owner has to decide.
The dismiss-stale-approval robustness gaps (concurrency cancellation, disabled-input cleanup, required-gate wiring) are deferred to a follow-up.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/340 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 10/6/2026 **Status:** ✅ Merged **Merged:** 10/6/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/cursor-review-auto-approve` --- ### 📝 Commits (3) - [`5395890`](https://github.com/Comfy-Org/github-workflows/commit/5395890b307e0f9909b87b459ad51b746b43b81d) feat(cursor-review): opt-in auto-approve by severity threshold - [`87c4541`](https://github.com/Comfy-Org/github-workflows/commit/87c4541a43262367a8b458fdbbe1ab67f956547e) fix(cursor-review): resolve coderabbit review comments - [`6753b2e`](https://github.com/Comfy-Org/github-workflows/commit/6753b2e02a37e69b75585877d49429d955c893c9) fix(cursor-review): address the auto-approve panel findings and CI ### 📊 Changes **9 files changed** (+961 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-review/README.md` (+2 -0) ➕ `.github/cursor-review/auto-approve.py` (+427 -0) 📝 `.github/cursor-review/gate-unresolved.py` (+3 -0) ➕ `.github/cursor-review/tests/test_auto_approve.py` (+236 -0) 📝 `.github/cursor-review/tests/test_workflow_job_isolation.py` (+1 -1) 📝 `.github/cursor-review/tests/test_workflow_ref_guard_parity.py` (+1 -1) 📝 `.github/workflow-pins/tests/test_check_workflow_pins.py` (+5 -2) 📝 `.github/workflows/cursor-review.yml` (+204 -0) 📝 `docs/callers/cursor-review.md` (+82 -0) </details> ### 📄 Description ## ELI-5 The Cursor review bot can now approve a PR by itself, if a repo turns that on. Each repo picks how bad a finding can be and still get approved (`medium`, `low` or `nit`). Anything worse gets "request changes" instead. If anything about the review looks unreliable, the bot does nothing and takes back any approval it gave earlier. When new commits are pushed, it also takes its earlier approval back. ## What changed - **New input `approve_max_severity`** (default `''` = off) and optional secret **`APPROVER_TOKEN`**. - **`.github/cursor-review/auto-approve.py`** - `decide` runs as a new last step of `post-review`, the job that already holds the write token and checks out no PR code. - It **approves**, pinned to the reviewed commit, when every finding is at or below the threshold. - It **requests changes** when any finding is above the threshold or has an unrecognised severity. - It **does nothing** if the judge didn't adjudicate, any panel reviewer didn't complete, the review didn't land as threads (or a finding landed in the body only), the head moved, the PR state couldn't be read, or an earlier thread above the threshold is still open. A do-nothing round also **withdraws** the bot's own earlier approvals. - The step is `continue-on-error`, so a refused approval can't make the blocking gate read `post-review` as "review did not land". - Model-supplied severity/path/line in its review body are sanitised with `post-review.py`'s `render_code_ref`. - **New job `dismiss-stale-approval`**: on `synchronize`, it dismisses this identity's own auto-approve **approvals** that aren't on the new head. Rulesets with `dismiss_stale_reviews_on_push: false` would otherwise keep a stale approval valid. A request-changes is left alone: a push starts no new round under a label-triggered caller, so only the next round replaces it. - `gate-unresolved.py`'s thread query now also fetches the first comment `body`, which is where the severity badge lives. - Docs: a new **Auto-approve** section in `docs/callers/cursor-review.md`, plus an input row and a script-table row in the README. **Trust model:** the decision rests on model output over the PR's own diff, and for `pull_request` events the input is read from the caller workflow at the PR head. So enabling it makes the approval no stronger than push access. The guide's new **Trust model** section says this explicitly. Whether to ship with that posture is the owner's call; the two review threads that raised it are still open. **Who approves:** only `APPROVER_TOKEN`'s user. There is no fallback to the `bot_app_id` App or `github-actions[bot]`, and nothing is submitted if the token is unset. Use a dedicated machine user in a code-owner team (on cloud, `comfy-standard-approver`) with a token scoped to that one repo. The dismiss job holds only that token, and its job token is read-only. **Security (documented):** a prompt-injected PR can get itself approved, and anyone with push access can enable the input from their own PR or read the token. Both are accepted risks for this feature. ## Testing - `tests/test_auto_approve.py` covers every rule: threshold validation, approve, request changes, unknown severity, degraded judge, incomplete panel, undelivered or body-only review, moved head, unreadable state, threshold-derived prior-thread gate, withdrawal on an untrusted round, the approvals-only dismissal filter, and markdown sanitising. - Existing tests updated for the new job: the cursor-review guard-parity count goes from 8 to 9, the bot-key-holders set gains the checkout-free job, and the workflow-pins guarded-checkout count goes from 18 to 19. - **Not yet tested live.** Callers are unaffected until they set the input. ## Provenance **Authored by:** agent-work loop **Verified:** - `python3 -m unittest discover` for `.github/cursor-review/tests` (748 OK) and `.github/workflow-pins/tests` (502 OK, 29 skipped) at the current head - `check_workflow_pins.py` OK. `check-org-repo-literals.sh` OK. `check_agents_md.py` passes with 2 warnings. - CI green on the current head (all 8 checks pass; no required checks configured). Merged with current `main` locally: the same suites and lints still pass. **Deviations:** - The prompt-injection and PR-editable-input findings are documented, not fixed in code. No out-of-model signal exists, so the owner has to decide. - The `dismiss-stale-approval` robustness gaps (concurrency cancellation, disabled-input cleanup, required-gate wiring) are deferred to a follow-up. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:38:44 +00:00