diff --git a/tests/test_codeql_scan_dispatch_workflow_contract.py b/tests/test_codeql_scan_dispatch_workflow_contract.py index bad19b54aa..2439d2936e 100644 --- a/tests/test_codeql_scan_dispatch_workflow_contract.py +++ b/tests/test_codeql_scan_dispatch_workflow_contract.py @@ -19,6 +19,9 @@ from scripts.ci import audit_central_required_workflows as ruleset_audit from tests.test_opencode_workflow_shell_syntax import _extract_run_block +from tests.test_required_workflow_queue_contract import ( + workflow_level_concurrency_group, +) REPO_ROOT = Path(__file__).resolve().parents[1] WORKFLOW_PATH = REPO_ROOT / ".github/workflows/codeql-scan-dispatch.yml" @@ -94,10 +97,14 @@ def test_codeql_scan_dispatch_keeps_current_head_language_shards_independent(): """A current-head language scan cannot cancel its sibling language scans.""" workflow = WORKFLOW_PATH.read_text(encoding="utf-8") concurrency = workflow.split("concurrency:\n", 1)[1].split("\n\npermissions:", 1)[0] - - assert "github.event.client_payload.target_repository" in concurrency - assert "github.event.client_payload.pr_number" in concurrency - assert "github.event.client_payload.required_language" in concurrency + group_value = workflow_level_concurrency_group(workflow) + + # The language segment is what keeps sibling language shards in separate groups, so it is + # asserted on the group's own value: a comment naming it would otherwise satisfy the check + # while the key had lost it, silently letting one language's scan cancel another's. + assert "github.event.client_payload.target_repository" in group_value + assert "github.event.client_payload.pr_number" in group_value + assert "github.event.client_payload.required_language" in group_value assert "cancel-in-progress: true" in concurrency diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 48035c2c40..9724d67ea7 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -21,6 +21,38 @@ def workflow_text(name: str) -> str: return (REPO_ROOT / ".github" / "workflows" / name).read_text(encoding="utf-8") +def workflow_level_concurrency_group(workflow: str) -> str: + """Return only the workflow-level ``concurrency.group`` value, comments removed. + + Asserting that an expression "appears in the concurrency block" is satisfied by + a comment that merely documents the key while the key itself says something + else, because the block's raw text carries its comments. That is not + hypothetical: the block above this workflow's group explains the key in prose, + so a maintainer quoting the expressions there while another change collapsed + the group to the repository alone would leave every pull request in one group, + cancelling each other, with the contract still green. Slice to the group's own + value so the assertion tests the key rather than the documentation beside it. + """ + header = workflow.split("permissions:", 1)[0] + block = header.split("concurrency:", 1)[1] + value: list[str] = [] + collecting = False + for line in block.splitlines(): + if line.strip().startswith("#"): + continue + if not collecting: + if re.match(r"^\s*group:", line): + collecting = True + value.append(line.split("group:", 1)[1]) + continue + if re.match(r"^\s*[A-Za-z][\w-]*:", line): + break + value.append(line) + if not collecting: + raise AssertionError("workflow-level concurrency block declares no group") + return "\n".join(value) + + def workflow_step(workflow: str, name: str) -> str: """Extract one named workflow step without parsing YAML dynamically.""" step = f" - name: {name}\n" @@ -234,22 +266,77 @@ def test_privileged_review_dispatch_coalesces_superseded_runs_before_admission() workflow = workflow_text("opencode-review-dispatch.yml") header = workflow.split("permissions:", 1)[0] concurrency_contract = header.split("concurrency:", 1)[1] + group_value = workflow_level_concurrency_group(workflow) assert re.search(r"(?m)^concurrency:", header) - assert "opencode-review-dispatch-" in concurrency_contract + assert "opencode-review-dispatch-" in group_value assert ( "github.event.client_payload.target_repository || github.repository" - in concurrency_contract - ) - assert ( - "github.event.client_payload.pr_number || github.run_id" - in concurrency_contract + in group_value ) + assert "github.event.client_payload.pr_number || github.run_id" in group_value assert "cancel-in-progress: true" in concurrency_contract assert "github.event.client_payload.pr_head_sha" not in concurrency_contract assert re.search(r"(?m)^ concurrency:", workflow) +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. + + This is the negative control for ``workflow_level_concurrency_group``. The + synthetic workflow below is exactly the shape that defeated the previous + contract: the real group is collapsed to the repository alone, so every pull + request in that repository shares one group and they cancel each other, while + a comment directly above still quotes both expressions the contract looks for. + Reading the raw block finds them; reading the group's value does not. + """ + defeated = textwrap.dedent( + """\ + name: Example + on: + repository_dispatch: + concurrency: + # Key: github.event.client_payload.target_repository || github.repository + # with github.event.client_payload.pr_number || github.run_id + group: opencode-review-dispatch-${{ github.repository }} + cancel-in-progress: true + permissions: + contents: read + """ + ) + raw_block = defeated.split("permissions:", 1)[0].split("concurrency:", 1)[1] + group_value = workflow_level_concurrency_group(defeated) + + assert "github.event.client_payload.pr_number || github.run_id" in raw_block + assert "github.event.client_payload.pr_number || github.run_id" not in group_value + assert "github.event.client_payload.target_repository" not in group_value + assert "opencode-review-dispatch-${{ github.repository }}" in group_value + + +def test_concurrency_group_slice_reads_a_folded_multi_line_key() -> None: + """The real key is a folded block, so the slice must join its continuation lines.""" + folded = textwrap.dedent( + """\ + concurrency: + group: >- + opencode-review-dispatch-${{ + github.event.client_payload.target_repository || github.repository }}-${{ + github.event.client_payload.pr_number || github.run_id }} + cancel-in-progress: true + permissions: + contents: read + """ + ) + group_value = workflow_level_concurrency_group(folded) + + assert ( + "github.event.client_payload.target_repository || github.repository" + in group_value + ) + assert "github.event.client_payload.pr_number || github.run_id" in group_value + assert "cancel-in-progress" not in group_value + + 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") @@ -292,33 +379,32 @@ def test_required_pull_request_workflows_cancel_superseded_runs() -> None: concurrency_contract = workflow.split("concurrency:", 1)[1].split( "permissions:", 1 )[0] + group_value = workflow_level_concurrency_group(workflow) assert "concurrency:" in workflow - assert "github.event.pull_request.base.repo.full_name" in concurrency_contract - assert "github.repository" in concurrency_contract + assert "github.event.pull_request.base.repo.full_name" in group_value + assert "github.repository" in group_value assert "github.event.pull_request.number" in workflow assert re.search(r"(?m)^concurrency:", workflow) assert "cancel-in-progress: true" in concurrency_contract if filename == "security-scan.yml": assert ( - "github.event_name == 'pull_request_target'" in concurrency_contract - or ("github.event_name == 'pull_request'" in concurrency_contract) + "github.event_name == 'pull_request_target'" in group_value + or ("github.event_name == 'pull_request'" in group_value) ) elif filename == "opencode-review.yml": - assert "required-opencode-review-${{" in concurrency_contract + assert "required-opencode-review-${{" in group_value assert "outputs.admitted == 'true'" in workflow elif filename == "noema-review.yml": assert not re.search(r"(?m)^ concurrency:", workflow) assert "github.event.workflow_run" not in concurrency_contract - assert "required-noema-review-${{" in concurrency_contract + assert "required-noema-review-${{" in group_value assert "outputs.admitted == 'true'" in workflow else: if filename == "codeql-pr.yml": - assert "github.event_name == 'pull_request'" in concurrency_contract + assert "github.event_name == 'pull_request'" in group_value else: - assert ( - "github.event_name == 'pull_request_target'" in concurrency_contract - ) + assert "github.event_name == 'pull_request_target'" in group_value assert "github.event.pull_request.head.sha" not in concurrency_contract assert "format('pr-{0}-{1}'" not in concurrency_contract @@ -425,15 +511,17 @@ def test_strix_serializes_provider_evidence_per_repository_and_pr() -> None: )[0] strix_job = workflow.split("\n strix:\n", 1)[1] + group_value = workflow_level_concurrency_group(workflow) + assert re.search(r"(?m)^concurrency:", workflow) assert "needs: [changed-scope, admit-current-head]" in strix_job assert "needs.admit-current-head.outputs.admitted == 'true'" in strix_job - assert "strix-security-scan-${{" in concurrency_contract - assert "github.event.pull_request.base.repo.full_name" in concurrency_contract - assert "github.event.client_payload.target_repository" in concurrency_contract - assert "github.event.pull_request.number" in concurrency_contract - assert "github.event.client_payload.pr_number" in concurrency_contract - assert "github.run_id" in concurrency_contract + assert "strix-security-scan-${{" in group_value + assert "github.event.pull_request.base.repo.full_name" in group_value + assert "github.event.client_payload.target_repository" in group_value + assert "github.event.pull_request.number" in group_value + assert "github.event.client_payload.pr_number" in group_value + 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