diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index e734e6a13..f6f9caaa1 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -109,7 +109,8 @@ matrix. Converting an active pull request to draft does not cancel its existing run. Prefer Draft for workload-affecting changes until local deterministic gates pass. -- Pull-request concurrency cancels superseded runs. Before debugging a cancelled +- Pull-request concurrency cancels superseded runs, while `dev` pushes queue in + order and never cancel one another. Before debugging a cancelled pull-request run, compare its head SHA with the current pull-request head and classify an expected supersession separately from a failure. Use `nvx-ci-investigation` for failed or unexpectedly cancelled runs. diff --git a/.github/skills/nvx-ci-investigation/SKILL.md b/.github/skills/nvx-ci-investigation/SKILL.md index 51cdcbb01..a515a0443 100644 --- a/.github/skills/nvx-ci-investigation/SKILL.md +++ b/.github/skills/nvx-ci-investigation/SKILL.md @@ -36,6 +36,10 @@ cancels an older run when a newer head is synchronized. - If a cancelled run's head differs from the current pull-request head and a newer run exists, classify it as expected supersession. Report the replacement run and stop unless cancellation cleanup itself failed. +- When the workflow at its head sets `queue: max` for pushes, a `dev` push run waits + behind earlier pushes instead of being replaced, so its cancellation is never a + supersession. Under earlier workflows, a newer `dev` push replaced the one waiting + run. - A run started while a pull request was Ready remains active after conversion to Draft because `converted_to_draft` is not a workflow trigger. Do not diagnose this as the draft guard being ignored. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7db9b76c9..2686a5911 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,6 +19,10 @@ on: concurrency: group: ci-${{ github.workflow }}-${{ (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') && format('full-{0}', github.run_id) || github.event.pull_request.number || github.ref }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} + # dev pushes wait in order instead of cancelling the run that waits. GitHub + # rejects queueing together with cancel-in-progress, so pull requests keep + # the single waiting run that a newer push replaces. + queue: ${{ github.event_name == 'pull_request' && 'single' || 'max' }} permissions: contents: read diff --git a/doc/ci.md b/doc/ci.md index 45f0102af..8e9bc513f 100644 --- a/doc/ci.md +++ b/doc/ci.md @@ -532,6 +532,29 @@ still 13% with a warm sccache, which would take over 2 GiB of the repository's repositories pay for, or a self-hosted Windows runner without a hypervisor would cost about as much as another general WHP runner. +## Run concurrency + +Each pull request runs in a concurrency group of its own, and a push to it +cancels the run of the previous head, whether that run waits or runs. Pushes +to `dev` share one group and run one at a time in the order in which they +arrive. The group keeps up to 100 waiting runs (`queue: max`), so a merge does +not cancel the waiting run of an earlier merge. Until October 2026 the group +kept a single waiting run, which each newer push replaced: 12 of the 199 `dev` +pushes from October 4 to 21 lost their run that way, five of them merged +within four minutes on October 20. Those commits got no development release +and no performance results, and because the detector compares a push with its +previous commit, the run that followed classified only its own changes. +GitHub rejects `queue: max` together with `cancel-in-progress: true`, so the +workflow derives both settings from the event. Each scheduled or manually +dispatched full run has a group of its own (see +[Change classification](#change-classification)). + +A queued run waits for the runs ahead of it. Replaying the `dev` pushes of +October 4 to 21 one at a time, at their measured durations and the median one +for those that were cancelled, half would have started at once, one in ten +would have waited at least 50 minutes, and the longest wait, on October 20, +would have been about three hours. + ## Change classification `openvmm-changes` lists the files that changed between the pull request's base diff --git a/scripts/test_nvx_tools.py b/scripts/test_nvx_tools.py index 7a553f8c4..394e91fca 100644 --- a/scripts/test_nvx_tools.py +++ b/scripts/test_nvx_tools.py @@ -4030,6 +4030,26 @@ def test_ci_full_run_triggers_and_concurrency_are_separate(self): header, ) + def test_ci_dev_pushes_queue_while_pull_requests_cancel(self): + workflow = ( + BuildConstants.REPO_ROOT / ".github" / "workflows" / "ci.yml" + ).read_text(encoding="utf-8") + header = workflow.split("\njobs:\n", 1)[0] + concurrency = header.split("\nconcurrency:\n", 1)[1].split("\n\n", 1)[0] + + # GitHub rejects `queue: max` with `cancel-in-progress: true`, so a + # static queue mode would stop every pull-request run from starting. + self.assertIn( + "\n cancel-in-progress: ${{ github.event_name == 'pull_request' }}\n", + concurrency, + ) + self.assertIn( + "\n queue: ${{ github.event_name == 'pull_request' && " + "'single' || 'max' }}", + concurrency, + ) + self.assertNotRegex(concurrency, r"(?m)^\s*queue:\s*['\"]?max") + def test_ci_full_runs_short_circuit_change_detection(self): workflow = ( BuildConstants.REPO_ROOT / ".github" / "workflows" / "ci.yml"