From d49d5d9c0ffffc492a65d4e7af676d6ba77dca1d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 22:25:03 +0900 Subject: [PATCH] fix(actions): coalesce superseded agent mentions while they are queued Both agent-mention dispatchers carried their concurrency group on the single `validate-and-forward` job. A job-level group is not evaluated while the whole run waits behind the organization job ceiling, so a superseded mention keeps its queue slot until a runner frees up and only cancels once it is already running -- which is after the slot has been held, not instead of holding it. Moving the group to workflow level coalesces the older run while both are still queued. Measured on `opencode-review-dispatch.yml`, which carried the identical defect until #1958, over the 39.7 hours ending 2026-09-06T12:41Z: 24 pairs of runs for one pull request overlapped in time, and none was coalesced. The five whose older run ended `cancelled` were cancelled 0.7 to 2.9 hours after the newer run was created, which is a sweep rather than concurrency; the other 18 ran to `failure` while already superseded, the longest living 9.4 hours past its successor's arrival (ContextualWisdomLab/.github#1529). `agent-mention-router.yml` is deliberately left alone: it has two jobs with different groups, and its organization sweep sets `cancel-in-progress: false` on purpose. A workflow carries at most one workflow-level group, so hoisting either one would give the sweep the route's cancelling policy and let a later comment kill a sweep part way through the organization. A test now pins that. The group text is unchanged and stays on one line, so the string `test_agent_mention_downstream_idempotency.py` pins still matches verbatim. That test's `assert "concurrency:" not in header` is retargeted rather than deleted: `git log -S` shows it arrived with 109d79b7 ("replace unsupported queue concurrency"), which removed a workflow-level block using `queue: max` -- a key GitHub Actions does not support -- and parked the group on the job on the way past. What that commit pins is the absence of `queue:`, not the level, so the assertion now reads `assert "queue:" not in header` plus a job-level absence check. Controls both ways: reverting to job level fails the retargeted contract and the new test; reintroducing `queue: max` still fails the retargeted contract. Gate: 2959 passed, 1 skipped, coverage 100%, interrogate 100%. Co-Authored-By: Claude Opus 5 --- .../agent-mention-noema-dispatch.yml | 17 ++++- .../agent-mention-opencode-dispatch.yml | 17 ++++- ...st_agent_mention_downstream_idempotency.py | 17 +++-- .../test_required_workflow_queue_contract.py | 67 +++++++++++++++++++ 4 files changed, 108 insertions(+), 10 deletions(-) diff --git a/.github/workflows/agent-mention-noema-dispatch.yml b/.github/workflows/agent-mention-noema-dispatch.yml index 5bed3e8963..ad8abc7b25 100644 --- a/.github/workflows/agent-mention-noema-dispatch.yml +++ b/.github/workflows/agent-mention-noema-dispatch.yml @@ -8,15 +8,26 @@ on: repository_dispatch: types: [agent-mention-noema] +concurrency: + # Workflow-level admission, for the same reason strix.yml, noema-review.yml, + # opencode-review.yml and opencode-review-dispatch.yml carry theirs at this level: + # a job-level group is never evaluated while the whole run waits behind the + # organization job ceiling, so a superseded mention keeps its queue slot until a + # runner frees up and only then cancels. At workflow level the older run is + # coalesced while both are still queued, which is where the slot is actually held. + # This workflow has a single job, so the group lives here and nowhere else -- + # every workflow in this repository that carries a group at both levels + # (strix.yml, opencode-review-dispatch.yml) gives the two levels DIFFERENT names, + # because a job requesting the group its own run already holds would wait on itself. + group: agent-mention-noema-${{ github.event.client_payload.target_repository }}-${{ github.event.client_payload.pr_number || github.run_id }} + cancel-in-progress: true + permissions: contents: read jobs: validate-and-forward: if: github.repository == 'ContextualWisdomLab/.github' - concurrency: - group: agent-mention-noema-${{ github.event.client_payload.target_repository }}-${{ github.event.client_payload.pr_number || github.run_id }} - cancel-in-progress: true runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: diff --git a/.github/workflows/agent-mention-opencode-dispatch.yml b/.github/workflows/agent-mention-opencode-dispatch.yml index b27062ae37..05461c9551 100644 --- a/.github/workflows/agent-mention-opencode-dispatch.yml +++ b/.github/workflows/agent-mention-opencode-dispatch.yml @@ -8,15 +8,26 @@ on: repository_dispatch: types: [agent-mention-opencode] +concurrency: + # Workflow-level admission, for the same reason strix.yml, noema-review.yml, + # opencode-review.yml and opencode-review-dispatch.yml carry theirs at this level: + # a job-level group is never evaluated while the whole run waits behind the + # organization job ceiling, so a superseded mention keeps its queue slot until a + # runner frees up and only then cancels. At workflow level the older run is + # coalesced while both are still queued, which is where the slot is actually held. + # This workflow has a single job, so the group lives here and nowhere else -- + # every workflow in this repository that carries a group at both levels + # (strix.yml, opencode-review-dispatch.yml) gives the two levels DIFFERENT names, + # because a job requesting the group its own run already holds would wait on itself. + group: agent-mention-opencode-${{ github.event.client_payload.target_repository }}-${{ github.event.client_payload.pr_number || github.run_id }} + cancel-in-progress: true + permissions: contents: read jobs: validate-and-forward: if: github.repository == 'ContextualWisdomLab/.github' - concurrency: - group: agent-mention-opencode-${{ github.event.client_payload.target_repository }}-${{ github.event.client_payload.pr_number || github.run_id }} - cancel-in-progress: true runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: diff --git a/tests/test_agent_mention_downstream_idempotency.py b/tests/test_agent_mention_downstream_idempotency.py index c9b6ab86ea..85974a5719 100644 --- a/tests/test_agent_mention_downstream_idempotency.py +++ b/tests/test_agent_mention_downstream_idempotency.py @@ -33,10 +33,19 @@ def test_downstream_workflows_claim_artifacts_and_coalesce_by_pull_request() -> ): header = text.split("\npermissions:\n", 1)[0] job = text.split(" validate-and-forward:\n", 1)[1] - concurrency = job.split(" concurrency:\n", 1)[1].split( - "\n runs-on:", 1 - )[0] - assert "concurrency:" not in header + concurrency = header.split("\nconcurrency:\n", 1)[1] + # 109d79b7 ("replace unsupported queue concurrency") deleted a + # workflow-level block that used ``queue: max``, a key GitHub Actions + # does not support, and parked the group on the job while it was at it. + # What that commit pins is the absence of ``queue:``, not the level: the + # group is back at workflow level because a job-level group is never + # evaluated while the run waits behind the organization job ceiling, so a + # superseded mention held its queue slot until a runner freed up. Every + # other queue-bearing workflow here (strix.yml, noema-review.yml, + # opencode-review.yml, codeql-scan-dispatch.yml, + # opencode-review-dispatch.yml) keys its group at workflow level too. + assert "queue:" not in header + assert " concurrency:" not in job assert "github.event.client_payload.agent_invocation_key" in text assert "cwl-agent-invocation:" in text assert "source_comment_id" in text diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 48035c2c40..5dc70e27bf 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -250,6 +250,73 @@ def test_privileged_review_dispatch_coalesces_superseded_runs_before_admission() assert re.search(r"(?m)^ concurrency:", workflow) +@pytest.mark.parametrize( + ("workflow_name", "group_prefix"), + ( + ("agent-mention-opencode-dispatch.yml", "agent-mention-opencode-"), + ("agent-mention-noema-dispatch.yml", "agent-mention-noema-"), + ), +) +def test_agent_mention_dispatch_coalesces_while_queued( + workflow_name: str, group_prefix: str +) -> None: + """A superseded agent mention must be discarded before it holds a queue slot. + + Both mention dispatchers carried the same defect + ``opencode-review-dispatch.yml`` carried before #1958: the group sat on the + single ``validate-and-forward`` job, and a job-level group is not evaluated + while the run waits behind the organization job ceiling. Measured on the + review dispatcher over the 39.7 hours ending 2026-09-06T12:41Z, 23 pairs of + runs for one pull request overlapped -- the older run was still open when its + successor arrived -- and none was coalesced; the five that ended + ``cancelled`` were cancelled between 0.7 and 2.9 hours after the newer run + was created, which is a sweep, not concurrency. + + The group moves to workflow level and is not duplicated on the job. Every + workflow here that keys a group at both levels (``strix.yml``, + ``opencode-review-dispatch.yml``) gives the two levels different names, + because a job that requests the group its own run already holds waits on + itself. + """ + workflow = workflow_text(workflow_name) + header = workflow.split("permissions:", 1)[0] + concurrency_contract = header.split("concurrency:", 1)[1] + + assert re.search(r"(?m)^concurrency:", header) + assert group_prefix in concurrency_contract + assert "github.event.client_payload.target_repository" in concurrency_contract + assert ( + "github.event.client_payload.pr_number || github.run_id" + in concurrency_contract + ) + assert "cancel-in-progress: true" in concurrency_contract + # ``\s`` also matches the newline before a column-0 key, so anchor the + # job-level search on horizontal whitespace only. + assert not re.search(r"(?m)^[ \t]+concurrency:", workflow) + + +def test_agent_mention_router_keeps_its_two_distinct_job_groups() -> None: + """The router must not be hoisted: its two jobs need different groups. + + ``agent-mention-router.yml`` runs a per-issue local route that supersedes + itself and an organization-wide sweep that must never be cancelled midway. + A workflow carries at most one workflow-level group, so hoisting either one + would silently give the sweep the route's ``cancel-in-progress: true`` and + let a later comment kill a sweep that is part way through the organization. + """ + workflow = workflow_text("agent-mention-router.yml") + + assert not re.search(r"(?m)^concurrency:", workflow) + assert ( + "group: review-agent-mention-router-local-${{ github.repository }}" + in workflow + ) + assert "group: review-agent-mention-router-sweep-${{ github.repository }}" in workflow + + sweep = workflow.split("sweep-organization-agent-mentions:", 1)[1] + assert "cancel-in-progress: false" in sweep.split("steps:", 1)[0] + + def test_required_opencode_dispatch_does_not_wait_on_merge_scheduler() -> None: """Dispatch review execution directly so polling cannot starve its producer.""" workflow = workflow_text("opencode-review.yml")