From aad5ac9797817891734cb4aa7dd82e8f4de6312b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 01:55:53 +0900 Subject: [PATCH] test(concurrency): anchor every cancel-in-progress contract, strix included MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1975 moved the queue contract file's own assertions onto helpers that read the key's real value. This finishes the sweep across the rest of the suite: 19 sites in 13 files that still matched `cancel-in-progress` as a substring, which a comment quoting the setting satisfies while the key beside it says the opposite. The gap that mattered was strix.yml. host 2 measured complete coverage over the 20 workflows carrying a workflow-level `cancel-in-progress: true` and found 8 unprotected, strix among them: commenting its flag out and adding `false` left the whole suite green, so a required review workflow could stop cancelling superseded runs with every contract still passing. Its own test asserted the flag, but as a substring, and `test_required_pull_request_workflows_cancel_ superseded_runs` never listed the file at all. Not every site takes the workflow-level helper, and the difference is the point: - Workflow-level value contracts move to `workflow_level_cancels_in_progress`. - Job-level blocks (the agent-mention router, the sweep job) stay where they are and get a line anchor instead — those workflows declare no workflow-level block, so the helper would raise rather than read the block under test. - Presence-only contracts (`test_close_empty_pr_queue_pressure`) keep asserting presence, anchored. Their workflows legitimately differ in value. - `test_repository_metadata_workflow_pages` contracts an expression, not a constant, so it gets an anchored regex for that expression. - Two sites are left alone deliberately: the negative assertion that the group value excludes its sibling key, and the fixture that shows the substring present while the helper returns False. Mutation evidence, tree rebuilt with `git archive` and restored clean after each: value flip (comment out, add `false`) 9 workflow/test pairs CAUGHT strix.yml after this change CAUGHT by 2 tests comment-out on presence contracts 3 of 3 CAUGHT value flip on presence contracts NOT CAUGHT -- by design, their contract is presence, not value Still unprotected and left that way: audit-central-ruleset, sbom-generation, scheduled-security-scan, secret-scan. No test states a cancellation contract for them, and asserting one here would invent policy rather than pin an existing decision. Gates: 2964 passed, coverage 100%, interrogate 100%. Co-Authored-By: Claude Opus 5 --- .../test_agent_mention_downstream_idempotency.py | 5 ++++- tests/test_agent_mention_queue_isolation.py | 8 +++++++- ..._agent_review_runtime_quality_consolidation.py | 6 +++++- tests/test_bootstrap_codeql_pull_requests.py | 6 +++++- tests/test_close_empty_pr_queue_pressure.py | 3 ++- .../test_exact_artifact_quality_single_runner.py | 6 +++++- tests/test_noema_review_gate.py | 6 +++++- ...est_noema_token_lifetime_stale_run_contract.py | 5 ++++- tests/test_opencode_required_rerun_capacity.py | 5 ++++- .../test_opencode_required_verdict_regression.py | 10 +++++++--- tests/test_pr_review_fix_scheduler_source_pin.py | 6 +++++- tests/test_repository_metadata_workflow_pages.py | 8 +++++++- tests/test_required_workflow_queue_contract.py | 15 ++++++++++----- 13 files changed, 70 insertions(+), 19 deletions(-) diff --git a/tests/test_agent_mention_downstream_idempotency.py b/tests/test_agent_mention_downstream_idempotency.py index 85974a5719..cb3c31763e 100644 --- a/tests/test_agent_mention_downstream_idempotency.py +++ b/tests/test_agent_mention_downstream_idempotency.py @@ -1,5 +1,8 @@ """Static contracts for downstream review-agent invocation idempotency.""" +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) from pathlib import Path ROOT = Path(__file__).resolve().parents[1] @@ -54,7 +57,7 @@ def test_downstream_workflows_claim_artifacts_and_coalesce_by_pull_request() -> f"group: {workflow_name}-${{{{ github.event.client_payload.target_repository }}}}-${{{{ github.event.client_payload.pr_number || github.run_id }}}}" in concurrency ) - assert "cancel-in-progress: true" in concurrency + assert workflow_level_cancels_in_progress(text) assert "queue: max" not in text assert "^[0-9a-f]{64}$" in text assert "^[1-9][0-9]*$" in text diff --git a/tests/test_agent_mention_queue_isolation.py b/tests/test_agent_mention_queue_isolation.py index e93ae61aed..d751076661 100644 --- a/tests/test_agent_mention_queue_isolation.py +++ b/tests/test_agent_mention_queue_isolation.py @@ -2,6 +2,9 @@ from __future__ import annotations +import re + + from pathlib import Path ROOT = Path(__file__).resolve().parents[1] @@ -68,4 +71,7 @@ def test_interactive_queue_retires_older_requests_for_only_the_same_pr() -> None concurrency = _concurrency_block(local_job) assert "github.event.issue.number || github.run_id" in concurrency - assert "cancel-in-progress: true" in concurrency + # Anchored on the JOB block, not the workflow-level helper: this router + # declares no workflow-level concurrency, so the sibling helper would raise + # rather than read the block this test is about. + assert re.search(r"(?m)^[ \t]+cancel-in-progress:[ \t]+true[ \t]*$", concurrency) diff --git a/tests/test_agent_review_runtime_quality_consolidation.py b/tests/test_agent_review_runtime_quality_consolidation.py index b0c90eb707..4592cfd166 100644 --- a/tests/test_agent_review_runtime_quality_consolidation.py +++ b/tests/test_agent_review_runtime_quality_consolidation.py @@ -2,6 +2,10 @@ from __future__ import annotations +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) + import re import subprocess from pathlib import Path @@ -56,7 +60,7 @@ def test_pr_concurrency_cancels_only_the_same_workflow_repository_and_pr() -> No "${{ github.repository }}-${{ github.event.pull_request.number }}" in concurrency_contract ) - assert "cancel-in-progress: true" in concurrency_contract + assert workflow_level_cancels_in_progress(workflow) assert "github.sha" not in concurrency_contract assert "head.sha" not in concurrency_contract assert "github.ref" not in concurrency_contract diff --git a/tests/test_bootstrap_codeql_pull_requests.py b/tests/test_bootstrap_codeql_pull_requests.py index eb20c3d1e0..12fd1d8b52 100644 --- a/tests/test_bootstrap_codeql_pull_requests.py +++ b/tests/test_bootstrap_codeql_pull_requests.py @@ -2,6 +2,10 @@ from __future__ import annotations +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) + from io import StringIO import json import subprocess @@ -67,7 +71,7 @@ def test_rendered_workflow_redetects_stacks_and_pins_every_action() -> None: assert "pull_request:" not in workflow assert "github.event.pull_request" not in workflow assert "github.event_name == 'push' && github.ref || github.event_name" in workflow - assert "cancel-in-progress: true" in workflow + assert workflow_level_cancels_in_progress(workflow) assert workflow.count("@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9") == 2 assert "actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0" in workflow diff --git a/tests/test_close_empty_pr_queue_pressure.py b/tests/test_close_empty_pr_queue_pressure.py index 331a604631..6da88f63f1 100644 --- a/tests/test_close_empty_pr_queue_pressure.py +++ b/tests/test_close_empty_pr_queue_pressure.py @@ -1,5 +1,6 @@ """Regression contracts for close-event runner admission pressure.""" +import re from pathlib import Path import pytest @@ -29,7 +30,7 @@ def test_closed_pull_request_does_not_allocate_a_noop_runner( assert "closed" in workflow assert "github.event.pull_request.number" in concurrency assert "github.event.pull_request.head.sha" not in concurrency - assert "cancel-in-progress:" in concurrency + assert re.search(r"(?m)^[ \t]+cancel-in-progress:[ \t]+\S", concurrency) assert "cancel-closed-pr-runs:" not in workflow assert "github.event.action != 'closed'" in workflow assert evidence_job in workflow diff --git a/tests/test_exact_artifact_quality_single_runner.py b/tests/test_exact_artifact_quality_single_runner.py index b8711ab7a5..9378e2232c 100644 --- a/tests/test_exact_artifact_quality_single_runner.py +++ b/tests/test_exact_artifact_quality_single_runner.py @@ -2,6 +2,10 @@ from __future__ import annotations +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) + import re from pathlib import Path @@ -68,7 +72,7 @@ def test_pr_concurrency_uses_workflow_repository_and_pr_identity() -> None: "${{ github.event.pull_request.number }}" in concurrency ) - assert "cancel-in-progress: true" in concurrency + assert workflow_level_cancels_in_progress(workflow) assert "github.sha" not in concurrency assert "pull_request.head.sha" not in concurrency diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 5fa23dec53..e8a0dd6f59 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -1,3 +1,7 @@ + +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) import base64 import hashlib import http.client @@ -60,7 +64,7 @@ def test_noema_concurrency_and_live_head_cleanup_preserve_current_review(): workflow = Path(".github/workflows/noema-review.yml").read_text(encoding="utf-8") concurrency = workflow.split("concurrency:", 1)[1].split("permissions:", 1)[0] assert "github.event.workflow_run" not in concurrency - assert "cancel-in-progress: true" in concurrency + assert workflow_level_cancels_in_progress(workflow) admission = workflow.split("\n admit-current-head:\n", 1)[1].split( "\n cancel-closed-pr-runs:", 1 )[0] diff --git a/tests/test_noema_token_lifetime_stale_run_contract.py b/tests/test_noema_token_lifetime_stale_run_contract.py index 77a64cabdb..108647907a 100644 --- a/tests/test_noema_token_lifetime_stale_run_contract.py +++ b/tests/test_noema_token_lifetime_stale_run_contract.py @@ -1,5 +1,8 @@ """Regression contract for consolidated Noema quality-run retirement.""" +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) from pathlib import Path @@ -26,4 +29,4 @@ def test_noema_token_lifetime_quality_ci_retires_superseded_pr_runs() -> None: assert "github.event.pull_request.head.sha" not in concurrency_contract assert "github.sha" not in concurrency_contract assert "github.ref" not in concurrency_contract - assert "cancel-in-progress: true" in concurrency_contract + assert workflow_level_cancels_in_progress(workflow) diff --git a/tests/test_opencode_required_rerun_capacity.py b/tests/test_opencode_required_rerun_capacity.py index 431d3a8bc2..c85bc24e3c 100644 --- a/tests/test_opencode_required_rerun_capacity.py +++ b/tests/test_opencode_required_rerun_capacity.py @@ -1,5 +1,8 @@ """Capacity contract for Required OpenCode dispatch and exact-run wakeup.""" +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) import json import os from pathlib import Path @@ -48,7 +51,7 @@ def test_native_cancellation_runs_before_runner_admission() -> None: assert "required-opencode-review-${{" in concurrency assert "github.event.pull_request.number || github.run_id" in concurrency - assert "cancel-in-progress: true" in concurrency + assert workflow_level_cancels_in_progress(required) assert "live_head_matches()" in required diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index f29b97a663..5c5325d1aa 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -2,6 +2,10 @@ from __future__ import annotations +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) + import json import os import re @@ -86,7 +90,7 @@ def test_opencode_dispatch_uses_the_same_target_repo_pr_group() -> None: assert "opencode-review-${{" in dispatched assert "needs.validate-pr-metadata.outputs.target_repository" in dispatched assert "needs.validate-pr-metadata.outputs.pr_number || github.run_id" in dispatched - assert "cancel-in-progress: true" in dispatched + assert workflow_level_cancels_in_progress(dispatched) assert dispatched.index("validate-pr-metadata:") < dispatched.index(" concurrency:") @@ -621,7 +625,7 @@ def test_opencode_review_trigger_reacts_to_draft_conversion() -> None: "types: [opened, synchronize, reopened, ready_for_review, " "converted_to_draft, closed]" ) in trigger_block - assert "cancel-in-progress: true" in workflow.split("\npermissions:\n", 1)[0] + assert workflow_level_cancels_in_progress(workflow) def test_opencode_review_concurrency_group_is_workflow_level_repo_and_pr() -> None: @@ -637,7 +641,7 @@ def test_opencode_review_concurrency_group_is_workflow_level_repo_and_pr() -> No assert "required-opencode-review-${{" in concurrency_block assert "github.event.pull_request.head.sha || github.run_id" not in concurrency_block assert "github.event.pull_request.number || github.run_id" in concurrency_block - assert "cancel-in-progress: true" in concurrency_block + assert workflow_level_cancels_in_progress(workflow) assert " concurrency:" not in target_job.split(" permissions:", 1)[0] admission = workflow.split("\n admit-current-head:\n", 1)[1].split( "\n coverage-source-tree:", 1 diff --git a/tests/test_pr_review_fix_scheduler_source_pin.py b/tests/test_pr_review_fix_scheduler_source_pin.py index 7958ba5163..0f9e0adb1c 100644 --- a/tests/test_pr_review_fix_scheduler_source_pin.py +++ b/tests/test_pr_review_fix_scheduler_source_pin.py @@ -2,6 +2,10 @@ from __future__ import annotations +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) + from pathlib import Path @@ -88,7 +92,7 @@ def test_reusable_scheduler_retains_least_privilege_and_bounded_dispatch() -> No assert "pull-requests: write" not in workflow assert "MAX_DISPATCHES:" in workflow assert "RETRY_HOURS:" in workflow - assert "cancel-in-progress: true" in workflow + assert workflow_level_cancels_in_progress(workflow) def test_reusable_scheduler_bounds_both_oidc_exchange_requests() -> None: diff --git a/tests/test_repository_metadata_workflow_pages.py b/tests/test_repository_metadata_workflow_pages.py index 82aa4462f7..5c05dfbe3d 100644 --- a/tests/test_repository_metadata_workflow_pages.py +++ b/tests/test_repository_metadata_workflow_pages.py @@ -2,6 +2,8 @@ from __future__ import annotations +import re + import importlib.util import json from pathlib import Path @@ -40,7 +42,11 @@ def test_metadata_pr_validation_cancels_superseded_head_runs() -> None: concurrency = workflow.split("concurrency:", 1)[1].split("jobs:", 1)[0] assert "group: repository-metadata-reconcile-${{ github.ref }}" in concurrency - assert "cancel-in-progress: ${{ github.event_name == 'pull_request' }}" in concurrency + assert re.search( + r"(?m)^[ \t]+cancel-in-progress:[ \t]+\$\{\{ github\.event_name == 'pull_request' \}\}" + r"[ \t]*$", + concurrency, + ) assert "github.event.pull_request.head.sha" not in concurrency diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 512cb6bef5..8bc0791631 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -402,7 +402,12 @@ def test_agent_mention_router_keeps_its_two_distinct_job_groups() -> None: 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] + # Anchored on the sweep JOB block: this router declares no workflow-level + # concurrency, so the sibling helper would raise rather than read it. + assert re.search( + r"(?m)^[ \t]+cancel-in-progress:[ \t]+false[ \t]*$", + sweep.split("steps:", 1)[0], + ) def test_concurrency_group_slice_ignores_the_comment_that_documents_it() -> None: """A comment quoting the key must not satisfy an assertion about the key. @@ -721,7 +726,7 @@ def test_strix_serializes_provider_evidence_per_repository_and_pr() -> None: assert "github.run_id" in group_value assert "github.event.pull_request.head.sha" not in concurrency_contract assert "github.event.client_payload.pr_head_sha" not in concurrency_contract - assert "cancel-in-progress: true" in concurrency_contract + assert workflow_level_cancels_in_progress(workflow) assert " concurrency:" not in strix_job.split(" permissions:", 1)[0] assert "queue: max" not in workflow assert workflow.index("admit-current-head:") < workflow.index("\n strix:\n") @@ -981,7 +986,7 @@ def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() - # outside that queue so synchronize and close events can retire old work. assert "admit-current-head:" in strix_workflow assert "skipping stale evidence" in strix_workflow - assert "cancel-in-progress: true" in strix_workflow + assert workflow_level_cancels_in_progress(strix_workflow) def test_merge_scheduler_owns_empty_pr_cleanup_without_checkout() -> None: @@ -1043,7 +1048,7 @@ def test_noema_triggers_preserve_standalone_pull_request_review() -> None: assert "github.event_name" not in concurrency_contract.split( "cancel-in-progress:", 1 )[0] - assert "cancel-in-progress: true" in concurrency_contract + assert workflow_level_cancels_in_progress(workflow) assert re.search(r"(?m)^concurrency:", workflow) assert not re.search(r"(?m)^ concurrency:", workflow) assert "needs.admit-current-head.outputs.admitted == 'true'" in noema_job @@ -1360,7 +1365,7 @@ def test_fix_scheduler_cancels_superseded_cron_runs() -> None: workflow = workflow_text("pr-review-fix-scheduler.yml") assert "central-pr-review-fix-scheduler-" in workflow - assert "cancel-in-progress: true" in workflow + assert workflow_level_cancels_in_progress(workflow) def test_security_scan_fails_closed_when_dependency_review_is_unavailable() -> None: