diff --git a/.github/workflows/ci-queue-janitor.yml b/.github/workflows/ci-queue-janitor.yml index f1c0b359c79a..d10c84777fe8 100644 --- a/.github/workflows/ci-queue-janitor.yml +++ b/.github/workflows/ci-queue-janitor.yml @@ -94,9 +94,13 @@ env: SOURCE_WORKFLOW_PATHS: .github/workflows/ci.yml # One sweep at a time; a second sweep waits rather than racing the first -# one's cancellations, and never cancels it. +# one's cancellations, and never cancels it. GitHub keeps one run waiting per +# group and cancels it for each newer arrival, so a run whose job's `if` starts +# no sweep (a CI run of a pull request into feat-cmux-next) gets a group of its +# own: in the shared group those runs cancelled the waiting sweep, often within +# seconds, and on 2026-10-30 left the pool snapshot 50 minutes old. concurrency: - group: ci-queue-janitor + group: ${{ (github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].base.ref == 'feat-cmux-next') && format('ci-queue-janitor-noop-{0}', github.run_id) || 'ci-queue-janitor' }} cancel-in-progress: false jobs: @@ -108,7 +112,9 @@ jobs: name: Sweep wasted macOS demand if: ${{ !(github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].base.ref == 'feat-cmux-next') }} runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || contains(fromJSON('["blacksmith-4vcpu-ubuntu-2404","blacksmith-16vcpu-ubuntu-2404","blacksmith-32vcpu-ubuntu-2404"]'), vars.LINUX_RUNNER) && vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} - timeout-minutes: 10 + # A sweep that lists the whole queue took up to 13 minutes on 2026-10-30; a timed-out + # job skips the upload below, so this leaves room for the longest one seen. + timeout-minutes: 15 steps: # Before the freshness gate, so its one read is on the App's budget too. Minting # spends the App's own requests, never GITHUB_TOKEN's. diff --git a/tests/test_ci_queue_janitor.py b/tests/test_ci_queue_janitor.py index 7ea765dad665..658df224f846 100644 --- a/tests/test_ci_queue_janitor.py +++ b/tests/test_ci_queue_janitor.py @@ -738,6 +738,30 @@ def test_a_feat_cmux_next_ci_run_starts_no_sweep(self): "${{ !(github.event_name == 'workflow_run' && " "github.event.workflow_run.pull_requests[0].base.ref == 'feat-cmux-next') }}") + def test_a_run_that_starts_no_sweep_never_evicts_a_waiting_sweep(self): + # The concurrency group keeps one run going and one waiting, and each new arrival cancels the + # waiting one. The CI runs of pull requests into feat-cmux-next start no sweep (the job's `if`), + # but with the whole workflow in the group they still arrived there and cancelled the real sweep + # that was waiting: on 2026-10-30 they were 238 of the janitor's last 400 runs, crons were + # cancelled within seconds of being queued, and after 17:57Z no snapshot was written until the + # 18:14 cron (the pool snapshot was 50 minutes old). Such a run gets a group of its own now. + workflow = yaml.safe_load(self.text) + no_sweep = ("github.event_name == 'workflow_run' && " + "github.event.workflow_run.pull_requests[0].base.ref == 'feat-cmux-next'") + self.assertEqual(workflow["jobs"]["sweep"]["if"], "${{ !(" + no_sweep + ") }}") + self.assertEqual(workflow["concurrency"], { + "group": "${{ (" + no_sweep + ") && format('ci-queue-janitor-noop-{0}', github.run_id) " + "|| 'ci-queue-janitor' }}", + "cancel-in-progress": False, + }) + + def test_a_long_sweep_still_publishes_its_snapshot(self): + # The snapshot is uploaded by the last step, which a job timeout skips. Sweeps listing the whole + # queue took up to 13 minutes on 2026-10-30, and two in a row hit the 10-minute timeout at + # 17:46Z and 17:58Z, so neither published. The job gets room for the longest sweep seen. + workflow = yaml.safe_load(self.text) + self.assertGreaterEqual(workflow["jobs"]["sweep"]["timeout-minutes"], 15) + def test_requested_runs_only_stand_in_for_a_late_cron(self): # 2026-10-06: CI was requested hundreds of times an hour, a 240 s skip # let those requests sweep about every 5 minutes on top of the cron, and @@ -759,13 +783,15 @@ def test_triggers_permissions_and_runner(self): self.assertIn('- cron: "*/10 * * * *"', text) self.assertIn("workflow_dispatch:", text) self.assertIn("dry_run:", text) - self.assertNotIn("pull_request", text.split("jobs:")[0].replace("pull-requests: read", "")) + # No pull_request trigger (the concurrency group reads workflow_run.pull_requests, which is no trigger). + self.assertNotIn("pull_request", text.split("jobs:")[0].replace("pull-requests: read", "") + .replace("workflow_run.pull_requests[0]", "")) self.assertIn("\npermissions: {}\n", text) self.assertIn(" sweep:\n permissions:\n actions: write\n pull-requests: read\n contents: read\n", text) # It mints GLAEDA_ROUTE_APP_KEY and CMUX_BOT_APP_KEY: LINUX_RUNNER moves it only among the # Blacksmith Linux sizes the expression names (tests/test_ci_fork_runner_routing.py). self.assertIn("runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || contains(fromJSON('[\"blacksmith-4vcpu-ubuntu-2404\",\"blacksmith-16vcpu-ubuntu-2404\",\"blacksmith-32vcpu-ubuntu-2404\"]'), vars.LINUX_RUNNER) && vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }}", text) - self.assertIn("concurrency:\n group: ci-queue-janitor\n cancel-in-progress: false\n", text) + self.assertIn("concurrency:\n group: ${{ (github.event_name == 'workflow_run'", text) self.assertIn("vars.CI_JANITOR_QUEUE_THRESHOLD", text) self.assertIn("ORPHAN_MINUTES: ${{ vars.CI_JANITOR_ORPHAN_MINUTES }}", text) self.assertIn("run: python3 scripts/ci/queue_janitor.py", text) diff --git a/tests/test_ci_starved_labels.py b/tests/test_ci_starved_labels.py index 4719073179b1..9177d2b774b0 100644 --- a/tests/test_ci_starved_labels.py +++ b/tests/test_ci_starved_labels.py @@ -294,7 +294,10 @@ def test_a_snapshot_without_the_starved_key_closes_nothing(self) -> None: class WorkflowTests(unittest.TestCase): def test_sweeps_are_serialized_and_never_cancelled_mid_reconcile(self) -> None: workflow = yaml.safe_load(WORKFLOW.read_text(encoding="utf-8")) - self.assertEqual(workflow["concurrency"], {"group": "ci-queue-janitor", "cancel-in-progress": False}) + concurrency = workflow["concurrency"] + self.assertIs(concurrency["cancel-in-progress"], False) + # every run that sweeps shares one group; only a run whose job starts no sweep gets its own + self.assertTrue(concurrency["group"].endswith("|| 'ci-queue-janitor' }}"), concurrency["group"]) def test_the_alert_step_uses_the_bot_token_and_adds_no_permission(self) -> None: workflow = yaml.safe_load(WORKFLOW.read_text(encoding="utf-8"))