[PR #342] [MERGED] fix(cursor-review): judge auto-approval staleness by the reviewed SHA #342

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/342
Author: @mattmillerai
Created: 10/6/2026
Status: ✅ Merged
Merged: 10/6/2026
Merged by: @mattmillerai

Base: main ← Head: matt/auto-approve-sha-marker

📝 Commits (3)

143a136 fix(cursor-review): judge auto-approval staleness by the reviewed SHA in the body
08f5e01 fix(cursor-review): warn when an approval is posted with no SHA marker
caab420 fix(cursor-review): dismiss-stale judges against the live head

📊 Changes

2 files changed (+135 additions, -14 deletions)

View changed files

📝 .github/cursor-review/auto-approve.py (+53 -8)
📝 .github/cursor-review/tests/test_auto_approve.py (+82 -6)

📄 Description

ELI-5

When someone pushed new commits, the bot was supposed to take its approval back, but it didn't. GitHub quietly relabels an old approval as if it were for the new commit, so the bot thought its approval was still current. The bot now writes the commit it reviewed into the approval text and checks that instead.

What happened (live test on a cloud PR)

The approver approved commit b01cf98.
After a push, GET /pulls/N/reviews reported that approval's commit_id as the new head, edaf99f. GitHub does this when dismiss_stale_reviews_on_push is off.
stale_reviews_to_dismiss compared commit_id to the head, saw a match, and withdrew nothing. The Dismiss stale auto-approval job went green while the PR stayed APPROVED.

Fix

render_body records .
stale_reviews_to_dismiss compares that recorded SHA with the live head and ignores commit_id.
A marked approval with no recorded SHA (from before this fix) counts as stale.

Testing

New regression tests:
- test_moved_commit_id_does_not_hide_a_stale_approval reproduces the live case.
- The legacy no-SHA case.
- The body records the reviewed SHA.
cursor-review (751 tests) and workflow-pins (502) pass.

🤖 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/342 **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/auto-approve-sha-marker` --- ### 📝 Commits (3) - [`143a136`](https://github.com/Comfy-Org/github-workflows/commit/143a136c72790a07d9957f857aa6947f23202ac1) fix(cursor-review): judge auto-approval staleness by the reviewed SHA in the body - [`08f5e01`](https://github.com/Comfy-Org/github-workflows/commit/08f5e0196735c693824c2bf92935635cd6030ab8) fix(cursor-review): warn when an approval is posted with no SHA marker - [`caab420`](https://github.com/Comfy-Org/github-workflows/commit/caab420cb5abeb880204a80ed338985d39042e69) fix(cursor-review): dismiss-stale judges against the live head ### 📊 Changes **2 files changed** (+135 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-review/auto-approve.py` (+53 -8) 📝 `.github/cursor-review/tests/test_auto_approve.py` (+82 -6) </details> ### 📄 Description ## ELI-5 When someone pushed new commits, the bot was supposed to take its approval back, but it didn't. GitHub quietly relabels an old approval as if it were for the new commit, so the bot thought its approval was still current. The bot now writes the commit it reviewed into the approval text and checks that instead. ## What happened (live test on a cloud PR) - The approver approved commit `b01cf98`. - After a push, `GET /pulls/N/reviews` reported that approval's `commit_id` as the new head, `edaf99f`. GitHub does this when `dismiss_stale_reviews_on_push` is off. - `stale_reviews_to_dismiss` compared `commit_id` to the head, saw a match, and withdrew nothing. The **Dismiss stale auto-approval** job went green while the PR stayed APPROVED. ## Fix - `render_body` records ``. - `stale_reviews_to_dismiss` compares that recorded SHA with the live head and ignores `commit_id`. - A marked approval with no recorded SHA (from before this fix) counts as stale. ## Testing - New regression tests: - `test_moved_commit_id_does_not_hide_a_stale_approval` reproduces the live case. - The legacy no-SHA case. - The body records the reviewed SHA. - cursor-review (751 tests) and workflow-pins (502) pass. 🤖 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