Comfy-Org/github-workflows

Fork 0

mirror of https://github.com/Comfy-Org/github-workflows.git synced 2026-11-05 00:17:37 +00:00

[PR #440] [MERGED] fix(cursor-review): pass Panel integrity on a cancelled run only when a push superseded it #440

New issue

Closed

opened 2026-10-26 22:39:50 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:39:50 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: cursor/panel-integrity-skip-cancelled-run-c60d

📝 Commits (2)

061a2ca fix(cursor-review): skip Panel integrity when the run is cancelled
4c51389 fix(cursor-review): keep Panel integrity on always(), pass a cancel only after the head moved

📊 Changes

5 files changed (+285 additions, -41 deletions)

View changed files

📝 .github/cursor-review/README.md (+12 -2)
📝 .github/cursor-review/tests/test_workflow_panel_integrity.py (+187 -15)
📝 .github/workflows/cursor-review.yml (+64 -15)
📝 README.md (+1 -1)
📝 docs/callers/cursor-review.md (+21 -8)

📄 Description

ELI5

When a code-review run gets cancelled because someone pushed newer code, the review check no longer shows a confusing red "re-run the failed job". A run cancelled for any other reason, such as a manual cancel, still fails. A required check can't quietly go green when nobody finished the review.

Before

Panel integrity used if: always() && (…). always() still starts the job after cancel-in-progress cancels the run. The decision jobs (gate, round-cap, diff-size, preflight, ledger) then report cancelled, not failure, and the guard failed the check:

Panel integrity cannot be judged: the jobs that decide whether a panel should run did not succeed (gate: cancelled, …) … re-run the failed job.

That is a false failure when the run was superseded by a newer push. The same red happened when a cancel landed mid-panel, because consolidate / post-review come back cancelled.

After

The job stays on always(). Skipping it with !cancelled() (this PR's first revision) would be a fail-open: cancelled() is true for every workflow cancel. A manual cancel has no replacement run, and a same-SHA replacement can decline to review. A skipped required check passes, so that skip would hand the head a green completeness gate over an unfinished panel.

Instead, a new first step, Judge a cancelled run, runs only when the run was cancelled and re-reads the PR's live head:

Head moved past this run's SHA (superseded by a push): the step passes and sets superseded. The guard and report steps stand down, so the check is green on a head nobody can merge any more, and required checks are read on the new head.
Head unchanged, no PR head in the event, or the read fails: exit 1. The check stays red, as before.

The job's permissions go from {} to pull-requests: read for that one API read.

Fail-closed is unchanged for a run that was not cancelled

A decision job whose result is failure still satisfies != 'success', and the guard still exits 1.
A job that hits its own timeout-minutes fails closed whichever way GitHub reports cancelled() for it. Either the guard reads its cancelled result, or the head check finds the head unmoved.
Deliberate no-panel branches (no label, already reviewed, max_rounds, over the diff-size cap) still skip.

A same-SHA re-trigger under cancel-in-progress (re-applying the label while a panel runs) still paints the cancelled run red on that SHA, which is the fail-closed side. The replacement run's later result is the one to read. The Blocking gate is untouched and still uses always().

Not in this PR

Two runs a few seconds apart for one event is the caller (or the app delivery) starting the workflow twice. This reusable's concurrency group already cancels the earlier one, which is what stops both panels from posting. Collapsing the duplicate trigger is not something the reusable can do safely; letting both runs finish would post two reviews.

Provenance

Verified:

python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py': 1740 tests OK, including the new CancelledRunTest, which runs the step against a stub gh (head moved / unmoved / unreadable / no PR head)
cursor-approve suite (339 OK), workflow-pins suite (551 OK), check_workflow_pins.py OK
org-repo literal lint OK, check_agents_md.py passed, shellcheck on the new step's script OK, workflow YAML parses

Deviations:

Replaced the first revision's !cancelled() job skip with an in-job head-moved check, per the review panel's High finding (raised by all six reviewers) that the skip was a fail-open on a manual or same-SHA cancel
Panel integrity now holds pull-requests: read instead of no permissions

_{🔄 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/440 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `cursor/panel-integrity-skip-cancelled-run-c60d` --- ### 📝 Commits (2) - [`061a2ca`](https://github.com/Comfy-Org/github-workflows/commit/061a2ca2484d06ff1dc642d09187d75d1b03361e) fix(cursor-review): skip Panel integrity when the run is cancelled - [`4c51389`](https://github.com/Comfy-Org/github-workflows/commit/4c51389a17cc3f5e645d1cef269813b131444c3c) fix(cursor-review): keep Panel integrity on always(), pass a cancel only after the head moved ### 📊 Changes **5 files changed** (+285 additions, -41 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-review/README.md` (+12 -2) 📝 `.github/cursor-review/tests/test_workflow_panel_integrity.py` (+187 -15) 📝 `.github/workflows/cursor-review.yml` (+64 -15) 📝 `README.md` (+1 -1) 📝 `docs/callers/cursor-review.md` (+21 -8) </details> ### 📄 Description ## ELI5 When a code-review run gets cancelled because someone pushed newer code, the review check no longer shows a confusing red "re-run the failed job". A run cancelled for any other reason, such as a manual cancel, still fails. A required check can't quietly go green when nobody finished the review.  ## Before `Panel integrity` used `if: always() && (…)`. `always()` still starts the job after `cancel-in-progress` cancels the run. The decision jobs (`gate`, `round-cap`, `diff-size`, `preflight`, `ledger`) then report `cancelled`, not `failure`, and the guard failed the check: > Panel integrity cannot be judged: the jobs that decide whether a panel should run did not succeed (gate: cancelled, …) … re-run the failed job. That is a false failure when the run was superseded by a newer push. The same red happened when a cancel landed mid-panel, because `consolidate` / `post-review` come back `cancelled`. ## After The job stays on `always()`. Skipping it with `!cancelled()` (this PR's first revision) would be a fail-open: `cancelled()` is true for every workflow cancel. A manual cancel has no replacement run, and a same-SHA replacement can decline to review. A skipped required check passes, so that skip would hand the head a green completeness gate over an unfinished panel. Instead, a new first step, `Judge a cancelled run`, runs only when the run was cancelled and re-reads the PR's live head: - **Head moved past this run's SHA** (superseded by a push): the step passes and sets `superseded`. The guard and report steps stand down, so the check is green on a head nobody can merge any more, and required checks are read on the new head. - **Head unchanged, no PR head in the event, or the read fails**: exit 1. The check stays red, as before. The job's permissions go from `{}` to `pull-requests: read` for that one API read. ## Fail-closed is unchanged for a run that was not cancelled - A decision job whose result is `failure` still satisfies `!= 'success'`, and the guard still exits 1. - A job that hits its own `timeout-minutes` fails closed whichever way GitHub reports `cancelled()` for it. Either the guard reads its `cancelled` result, or the head check finds the head unmoved. - Deliberate no-panel branches (no label, already reviewed, `max_rounds`, over the diff-size cap) still skip. A same-SHA re-trigger under `cancel-in-progress` (re-applying the label while a panel runs) still paints the cancelled run red on that SHA, which is the fail-closed side. The replacement run's later result is the one to read. The Blocking gate is untouched and still uses `always()`. ## Not in this PR Two runs a few seconds apart for one event is the caller (or the app delivery) starting the workflow twice. This reusable's concurrency group already cancels the earlier one, which is what stops both panels from posting. Collapsing the duplicate trigger is not something the reusable can do safely; letting both runs finish would post two reviews.  ## Provenance **Verified:** - `python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py'`: 1740 tests OK, including the new `CancelledRunTest`, which runs the step against a stub `gh` (head moved / unmoved / unreadable / no PR head) - cursor-approve suite (339 OK), workflow-pins suite (551 OK), `check_workflow_pins.py` OK - org-repo literal lint OK, `check_agents_md.py` passed, shellcheck on the new step's script OK, workflow YAML parses **Deviations:** - Replaced the first revision's `!cancelled()` job skip with an in-job head-moved check, per the review panel's High finding (raised by all six reviewers) that the skip was a fail-open on a manual or same-SHA cancel - Panel integrity now holds `pull-requests: read` instead of no permissions <div><a href="https://cursor.com/agents/bc-0a907785-b5bc-51c8-8aa6-9df7a4cac60d?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-0a907785-b5bc-51c8-8aa6-9df7a4cac60d&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:39:50 +00:00

closed this issue
added the
pull-request
label

No labels

cursor-review-catalog-drift

groom

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/github-workflows#440

No description provided.

Rows
Columns