From 1f00944e1cd3b15c670c9e47f074146b8772fb3b Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 6 Oct 2026 00:55:52 -0400 Subject: [PATCH] ci(project-status): stop a superseded sync from leaving a red check (#1066) Drop cancel-in-progress and queue runs with `queue: max`, so no run in a per-item or reconcile group is cancelled. Freshness still holds: runs on an item never overlap and each re-reads the labels, so the last to execute sees the final state. TestConcurrency now pins that only `group` and `queue: max` are set. --- .github/workflows/project-status.yml | 39 +++++++++++++++++++--------- tests/project/test_project_status.py | 33 +++++++++++++---------- 2 files changed, 47 insertions(+), 25 deletions(-) diff --git a/.github/workflows/project-status.yml b/.github/workflows/project-status.yml index 6beabc4418..09e0fd5870 100644 --- a/.github/workflows/project-status.yml +++ b/.github/workflows/project-status.yml @@ -50,9 +50,9 @@ on: pull_request_target: types: [opened, reopened, labeled, unlabeled, closed, ready_for_review] schedule: - # Nightly backstop for events this workflow missed -- a run cancelled by the - # concurrency group below before it re-read the labels, a GitHub outage, or a - # label changed while the secret was absent. 00:37 UTC sits well clear of the + # Nightly backstop for events this workflow missed -- a run dropped because + # its item's concurrency queue below was full, a GitHub outage, or a label + # changed while the secret was absent. 00:37 UTC sits well clear of the # Saturday jobs (CodeQL and GitHub Pages at 02:00, Python Compatibility at # 04:00, Lint at 06:00, Vendor and Conda Update at 10:00), and off the hour # mark that GitHub's scheduler queues most heavily. @@ -64,18 +64,33 @@ on: permissions: {} # Event runs are keyed on the item number, so a label swap -- which fires an -# `unlabeled` and a `labeled` within the same second -- serializes on one item, -# and the later run supersedes the earlier. That is safe to cancel because each -# run re-reads the labels rather than trusting its payload: the last one to run -# sees the final state. Issues and pull requests share one number space, so the -# key cannot collide across the two event types. +# `unlabeled` and a `labeled` within the same second -- serializes on one item. +# Issues and pull requests share one number space, so the key cannot collide +# across the two event types. # -# Scheduled and manual reconciles share their own group and are never cancelled -# mid-run; a newer one only displaces one still pending, which would have done the -# same work. +# Nothing in a group is cancelled. `cancel-in-progress` used to cancel the older +# run of a label swap, and a cancelled run stays on the PR head as a failed Sync +# Status check although the newer run fixed the board (#1066). Dropping it is not +# enough on its own: by default a group holds one pending run and cancels it when +# a newer one arrives. `queue: max` lets up to 100 wait instead; only a run past +# that is cancelled. GitHub documents both, and that the two keys together are a +# validation error: +# https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency +# +# The board still ends right, whatever order the queue runs in -- GitHub states +# that order is not guaranteed. Runs on one item never overlap, and each re-reads +# the labels rather than trusting its payload. So the last run to execute starts +# after every other run on the item has finished, including the one started by +# the final label event. It therefore reads the final labels, and its write is +# the last. The cost is that a burst of N events makes N runs instead of +# collapsing into one, each a handful of API calls. +# +# Scheduled and manual reconciles share their own group and queue the same way. +# A second reconcile waiting behind the first repeats its work, which is cheap at +# nightly cadence. concurrency: group: project-status-${{ (github.event.issue.number || github.event.pull_request.number) && format('item-{0}', github.event.issue.number || github.event.pull_request.number) || 'reconcile' }} - cancel-in-progress: ${{ github.event_name == 'issues' || github.event_name == 'pull_request_target' }} + queue: max jobs: status: diff --git a/tests/project/test_project_status.py b/tests/project/test_project_status.py index af14e947ca..6f1e664805 100644 --- a/tests/project/test_project_status.py +++ b/tests/project/test_project_status.py @@ -17,7 +17,8 @@ announce the mistake. :class:`TestScannerFailsClosed` holds the line scan those assertions read to refusing any YAML it cannot read with certainty. :class:`TestConcurrency` evaluates the ``concurrency:`` - expressions per event, so the cancel policy is pinned rather than eyeballed. + expressions per event and pins that no run is cancelled, so the queueing policy + is checked rather than eyeballed. :class:`TestBoardSync` drives the script against a fake GraphQL endpoint, which is as close to the live board as a unit test may get: the tier has no network and no @@ -401,30 +402,36 @@ def test_least_privilege(self) -> None: class TestConcurrency(unittest.TestCase): - """Per-item runs supersede each other; reconciles never cancel mid-run.""" + """Runs serialize per item, or per reconcile, and none is cancelled (#1066).""" def setUp(self) -> None: - pairs = dict(scan(WORKFLOW.read_text(encoding='utf-8'))) - self.group = pairs[('concurrency', 'group')] - self.cancel = pairs[('concurrency', 'cancel-in-progress')] + self.concurrency = {path[1]: value + for path, value in scan(WORKFLOW.read_text(encoding='utf-8')) + if path[:1] == ('concurrency',)} - def resolve(self, context: 'dict') -> 'tuple[object, object]': - return evaluate_expression(self.group, context), evaluate_expression(self.cancel, context) + def group(self, context: 'dict') -> 'object': + return evaluate_expression(self.concurrency['group'], context) def test_item_events(self) -> None: for event, key in (('issues', 'github.event.issue.number'), ('pull_request_target', 'github.event.pull_request.number')): with self.subTest(event=event): - group, cancel = self.resolve({'github.event_name': event, key: 42}) - self.assertEqual(group, 'project-status-item-42') - self.assertIs(cancel, True) + self.assertEqual(self.group({'github.event_name': event, key: 42}), + 'project-status-item-42') def test_reconcile(self) -> None: for event in ('schedule', 'workflow_dispatch'): with self.subTest(event=event): - group, cancel = self.resolve({'github.event_name': event}) - self.assertEqual(group, 'project-status-reconcile') - self.assertIs(cancel, False) + self.assertEqual(self.group({'github.event_name': event}), + 'project-status-reconcile') + + def test_nothing_is_cancelled(self) -> None: + # A cancelled run stays on the PR head as a failed check. Without + # `queue: max` a group keeps one pending run and cancels it for the next; + # with `cancel-in-progress` the running one goes too, and GitHub rejects + # the two together. + self.assertEqual(sorted(self.concurrency), ['group', 'queue']) + self.assertEqual(self.concurrency['queue'], 'max') _TITLE = '${{ github.event.issue.title }}'