[PR #16] [MERGED] feat(cursor-review): optional blocking gate for unresolved findings (BE-1891) #29

New issue

Closed

opened 2026-10-26 22:35:33 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:35:33 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-1891-cursor-review-blocking

📝 Commits (1)

da45649 feat(cursor-review): optional blocking gate for unresolved findings (BE-1891)

📊 Changes

4 files changed (+258 additions, -1 deletions)

View changed files

📝 .github/cursor-review/README.md (+43 -0)
➕ .github/cursor-review/gate-unresolved.py (+156 -0)
📝 .github/workflows/cursor-review.yml (+58 -0)
📝 README.md (+1 -1)

📄 Description

ELI-5

The cursor-review bot posts its findings on your PR as review threads, but today
they're just advice — a red review doesn't stop a merge. This adds an opt-in
switch (blocking: true) that makes those findings count: if any cursor-review
thread is still unresolved, a "Blocking gate" check turns red. Resolve the
threads (or push a fix and re-review) and it goes green. Nothing changes for
anyone until they flip the switch on.

What

New optional input blocking (boolean, default false) on the reusable
cursor-review.yml.
- false (default) → unchanged: review is advisory, the new gate job is
  skipped entirely, no caller behavior changes.
- true → a final Blocking gate job fails the check while the PR has
  unresolved cursor-review finding threads.
New script .github/cursor-review/gate-unresolved.py: queries the PR's
reviewThreads (GraphQL, paginated) and exits non-zero when any cursor-review
finding thread is unresolved.
README + workflow-header docs for the input and the required-status-check
step the caller must do to actually block merges.

How "unresolved" is defined (and why)

A review thread counts as a cursor-review finding thread when its originating
review's body starts with the consolidated marker (## 🔍 Cursor Review — Consolidated panel) — the same discriminator the workflow's existing dup-check
already uses. I chose the body-marker over matching the posting identity
(cloud-code-bot / github-actions[bot], per BE-1800/BE-1812) on purpose: it's
identity-independent, so the gate works whether the review posts as
github-actions[bot] (default) or under a dedicated bot_app_id App, with no
need to resolve a per-caller bot login. Only the inline-findings review creates
threads (the "no findings"/error/fallback reviews are body-only), so a matching
thread always means a real anchored finding.

Outdated threads (their hunk changed since the finding posted) are excluded —
a re-review re-posts anything still wrong as a fresh thread, so blocking on
outdated ones would force resolving superseded findings.

Idempotency

The gate queries live thread state on every run and its if: doesn't depend
on already_reviewed, so: resolve the threads → re-run the check (re-add the
label, or "Re-run jobs") → green, with no fresh panel needed.

Verification

Static: actionlint clean on the workflow; py_compile clean on the script;
YAML parses.

Behavioral — ran the gate script against real PRs (proves the marker matches
live GitHub data and the resolved/outdated filtering works):

PR	cursor threads	live unresolved	gate exit
cloud#4504	—	5	1 (RED)
cloud#4502	—	2	1 (RED)
cloud#4511	9 (all resolved/outdated)	0	0 (GREEN)
github-workflows#13	0	0	0 (GREEN)

Plus a unit-style logic test covering pagination + resolved/outdated/non-cursor/
empty-comment exclusion.

Acceptance criteria

blocking: true + unresolved bot thread → check RED; resolving → GREEN.
blocking: false / unset → check passes regardless of findings (gate job
skipped; rest of pipeline untouched).
Caller opts in with one input; README documents the required-status-check
step.

Judgment calls / notes

Body-marker over identity match for "is this our thread" — see above.
Outdated threads don't block — deliberate, documented.
The gate blocks on unresolved findings, not on review-infra failures. If
the panel/judge itself fails (no review posted, or an error review with no
threads), the gate is green — notify-complete already surfaces that failure
separately, and blocking merges on a transient cursor-agent/judge outage would
be too aggressive. Flagging in case fail-closed is preferred.
This workflow cannot set branch protection; merge-blocking still requires
marking the Blocking gate check as a required status check in the caller
repo (documented). Could not exercise the full GitHub Actions run here (no
live blocking: true caller yet) — the gate script is proven end-to-end
against live PRs; the job wiring is actionlint-clean and reuses the exact
asset-checkout pattern of the existing notify-* jobs.

_{🔄 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/16 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 6/27/2026 **Status:** ✅ Merged **Merged:** 6/30/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-1891-cursor-review-blocking` --- ### 📝 Commits (1) - [`da45649`](https://github.com/Comfy-Org/github-workflows/commit/da45649bed33a6fdbaa567ebd19508710914a5f0) feat(cursor-review): optional blocking gate for unresolved findings (BE-1891) ### 📊 Changes **4 files changed** (+258 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-review/README.md` (+43 -0) ➕ `.github/cursor-review/gate-unresolved.py` (+156 -0) 📝 `.github/workflows/cursor-review.yml` (+58 -0) 📝 `README.md` (+1 -1) </details> ### 📄 Description ## ELI-5 The cursor-review bot posts its findings on your PR as review threads, but today they're just advice — a red review doesn't stop a merge. This adds an **opt-in switch** (`blocking: true`) that makes those findings count: if any cursor-review thread is still unresolved, a "Blocking gate" check turns **red**. Resolve the threads (or push a fix and re-review) and it goes **green**. Nothing changes for anyone until they flip the switch on. ## What - New optional input **`blocking`** (boolean, default `false`) on the reusable `cursor-review.yml`. - `false` (default) → **unchanged**: review is advisory, the new gate job is skipped entirely, no caller behavior changes. - `true` → a final **`Blocking gate`** job fails the check while the PR has unresolved cursor-review finding threads. - New script `.github/cursor-review/gate-unresolved.py`: queries the PR's `reviewThreads` (GraphQL, paginated) and exits non-zero when any cursor-review finding thread is unresolved. - README + workflow-header docs for the input **and the required-status-check step** the caller must do to actually block merges. ## How "unresolved" is defined (and why) A review thread counts as a cursor-review finding thread when its **originating review's body starts with the consolidated marker** (`## 🔍 Cursor Review — Consolidated panel`) — the same discriminator the workflow's existing dup-check already uses. I chose the body-marker over matching the posting **identity** (`cloud-code-bot` / `github-actions[bot]`, per BE-1800/BE-1812) on purpose: it's identity-independent, so the gate works whether the review posts as `github-actions[bot]` (default) or under a dedicated `bot_app_id` App, with no need to resolve a per-caller bot login. Only the inline-findings review creates threads (the "no findings"/error/fallback reviews are body-only), so a matching thread always means a real anchored finding. **Outdated** threads (their hunk changed since the finding posted) are excluded — a re-review re-posts anything still wrong as a fresh thread, so blocking on outdated ones would force resolving superseded findings. ## Idempotency The gate queries **live** thread state on every run and its `if:` doesn't depend on `already_reviewed`, so: resolve the threads → re-run the check (re-add the label, or "Re-run jobs") → green, with no fresh panel needed. ## Verification Static: `actionlint` clean on the workflow; `py_compile` clean on the script; YAML parses. Behavioral — ran the gate script against **real PRs** (proves the marker matches live GitHub data and the resolved/outdated filtering works): | PR | cursor threads | live unresolved | gate exit | |---|---|---|---| | cloud#4504 | — | 5 | **1 (RED)** | | cloud#4502 | — | 2 | **1 (RED)** | | cloud#4511 | 9 (all resolved/outdated) | 0 | **0 (GREEN)** | | github-workflows#13 | 0 | 0 | **0 (GREEN)** | Plus a unit-style logic test covering pagination + resolved/outdated/non-cursor/ empty-comment exclusion. ## Acceptance criteria - [x] `blocking: true` + unresolved bot thread → check RED; resolving → GREEN. - [x] `blocking: false` / unset → check passes regardless of findings (gate job skipped; rest of pipeline untouched). - [x] Caller opts in with one input; README documents the required-status-check step. ## Judgment calls / notes - **Body-marker over identity match** for "is this our thread" — see above. - **Outdated threads don't block** — deliberate, documented. - **The gate blocks on unresolved findings, not on review-infra failures.** If the panel/judge itself fails (no review posted, or an error review with no threads), the gate is green — `notify-complete` already surfaces that failure separately, and blocking merges on a transient cursor-agent/judge outage would be too aggressive. Flagging in case fail-closed is preferred. - This workflow **cannot** set branch protection; merge-blocking still requires marking the `Blocking gate` check as a required status check in the caller repo (documented). Could not exercise the full GitHub Actions run here (no live `blocking: true` caller yet) — the gate *script* is proven end-to-end against live PRs; the job wiring is `actionlint`-clean and reuses the exact asset-checkout pattern of the existing `notify-*` jobs. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:35:33 +00:00