diff --git a/CHANGELOG.md b/CHANGELOG.md index bf192f6a9..8f459d5f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +### Required status context guard +- Extended that guard after sweeping branch protection across all 76 organization repositories: 13 use classic protection, and several pin the *job names* the central workflows declare -- `opencode-review` and `coverage-evidence` are each required by 7 repositories, `strix` by 5, `scan-pr-queue` by 4, `required-workflow-bootstrap` by 3, and `coverage-source-tree` by 2. `strix` and `coverage-source-tree` are required by siblings but not by `.github`, so a sweep of this repository alone missed them. The check now resolves each job's *effective* check-run name (its `name:` when present, its job id only otherwise) rather than accepting either spelling, which previously let a renamed `name:` pass whenever the job id still matched. A context whose check-run name is matrix-interpolated is now satisfied *only* by its template: branch protection requires the expanded contexts (`CodeQL compatibility analysis (actions)`, `... (python)`), which exist solely because the job name interpolates `${{ matrix.language }}`, so a rename to the bare `CodeQL compatibility analysis` still reports a check run while the expanded contexts are never produced. The earlier lookup accepted that bare spelling as a direct match and would have passed the rename (found in review by CodeRabbit on #1923). + +- Pinned each of this repository's classic branch-protection required status contexts to the job that reports it (`tests/test_branch_protection_required_context_jobs.py`). `.github` is excluded from the organization required-workflow ruleset, so its default branch is guarded by named contexts matched on the check-run name -- a job's `name:` when present and its job id otherwise. Renaming or folding away such a job leaves branch protection waiting forever for a context nothing reports, blocking every pull request with no failing check to point at, and nothing else in the suite caught it. The two identifiers already differ in practice (job id `opencode-review-target` reports context `opencode-review`), and `.github/workflows/` is under active consolidation, which is exactly the edit that renames jobs. + ### Failed-check finding names the Strix sandbox instead of the gateway - `opencode-review-dispatch.yml`'s `emit_strix_provider_failure_finding` rendered one fixed finding for every `STRIX_PROVIDER_UNAVAILABLE` line, whose Root cause read "The contextual-orchestrator gateway or its discovered provider pool was unavailable for this run". `#1953` had just given the Strix sandbox bootstrap failure its own second verdict token (`STRIX_SANDBOX_UNAVAILABLE`) precisely because that attribution is wrong for it -- the sandbox container never reaches its Caido proxy, so the run dies before the gateway serves anything -- and this consumer re-applied the wrong attribution one step downstream, into the review findings and the failure census. The emitter now branches on the second token: a sandbox verdict gets a finding that names Strix's sandbox, says the verdict does not name the gateway, and tells the reader not to change gateway or provider configuration on its strength. A `STRIX_PROVIDER_UNAVAILABLE` line without the token keeps its existing text verbatim, so the gateway class has no regression surface. No test covered this finding text at all before (`gateway or its discovered provider pool` matched nothing under `tests/`); `tests/test_opencode_dispatch_strix_sandbox_finding.py` now runs the production emitter from the published run block and pins both directions plus the no-signal case. Refs #1953, #1935. diff --git a/tests/test_branch_protection_required_context_jobs.py b/tests/test_branch_protection_required_context_jobs.py new file mode 100644 index 000000000..100eee596 --- /dev/null +++ b/tests/test_branch_protection_required_context_jobs.py @@ -0,0 +1,170 @@ +"""Pin every classic-branch-protection required context to the job that produces it. + +This repository is excluded from the organization required-workflow ruleset +(``repository_name.exclude`` lists ``.github``), so its default branch is guarded by +*classic* branch protection with a fixed list of named required status contexts. A +context is matched by the **check-run name**, which GitHub takes from a job's ``name:`` +when present and from the job id otherwise. + +That makes a job rename a repository-wide outage, not a local edit: branch protection +keeps waiting for a context that nothing will ever report, so every pull request stays +blocked with no failing check to point at. Nothing else in this suite catches it -- +these names are not pinned anywhere, and the two identifiers can drift apart. They +already have: the job id ``opencode-review-target`` reports the context +``opencode-review``, so renaming only the ``name:`` breaks protection while the job id +still looks correct. + +The hazard is live because ``.github/workflows/`` is under active consolidation (21 +consolidation/coalescing commits between 2026-09-01 and 2026-09-05), and folding jobs +together is exactly the edit that renames or removes them. + +The blast radius is not limited to this repository. A sweep of all 76 organization +repositories on 2026-09-05 found 13 with classic protection, and several pin the *job +names* these central workflows declare: ``opencode-review`` and ``coverage-evidence`` +are each required by 7 repositories, ``strix`` by 5, ``scan-pr-queue`` by 4, +``required-workflow-bootstrap`` by 3, and ``coverage-source-tree`` by 2. Renaming one +job here blocks every pull request in all of them at once, and those repositories +cannot see the change coming. ``admit-current-head`` is required by none, which is why +it is absent below. + +If a context here is deliberately retired, update branch protection **first** -- in +every repository that requires it, not just this one -- then this test. Changing this +test alone re-arms the outage. +""" + +from pathlib import Path + +import yaml + +WORKFLOW_DIR = Path(".github/workflows") + +# Live `required_status_checks.contexts` on this repository's default branch, +# read from the branch-protection API on 2026-09-05, paired with the workflow +# file whose job definition reports each one. +REQUIRED_CONTEXT_SOURCES = { + "CodeQL compatibility analysis": "codeql-pr.yml", + "Detect CodeQL languages": "codeql-pr.yml", + "coverage-evidence": "opencode-review.yml", + "dependency-review": "security-scan.yml", + "noema-review": "noema-review.yml", + "opencode-review": "opencode-review.yml", + "osv-scan": "security-scan.yml", + "required-workflow-bootstrap": "opencode-review.yml", + "scan-pr-queue": "pr-review-merge-scheduler.yml", + "scorecard": "security-scan.yml", + "trivy-fs": "security-scan.yml", + # Required by sibling repositories but NOT by `.github` itself, so a sweep of + # this repository's own protection would miss them. Measured 2026-09-05 across + # all 76 organization repositories: `strix` is required by pg-erd-cloud, + # bandscope, naruon, linux-cluster-ops and contextual-orchestrator; + # `coverage-source-tree` by naruon and linux-cluster-ops. Renaming either job + # in the central workflow blocks every pull request in those repositories. + "strix": "strix.yml", + "coverage-source-tree": "opencode-review.yml", +} + +# `CodeQL compatibility analysis` is reported once per matrix language, so branch +# protection names the expanded contexts (`... (actions)`, `... (python)`) while the +# workflow declares the template. +MATRIX_NAME_TEMPLATES = { + "CodeQL compatibility analysis": ( + "CodeQL compatibility analysis (${{ matrix.language }})" + ), +} + + +def _effective_check_names(workflow_name: str) -> set[str]: + """Return the check-run names one workflow can report. + + GitHub names a check run after the job's ``name:`` when it has one and after the + job id **only when it does not**. Accepting either spelling unconditionally would + pass a job whose id still matches while its ``name:`` was renamed away -- which is + precisely the break this test exists to catch, since the renamed name is what + branch protection would then wait for. + """ + document = yaml.safe_load((WORKFLOW_DIR / workflow_name).read_text(encoding="utf-8")) + names = set() + for job_id, job in (document.get("jobs") or {}).items(): + declared = job.get("name") if isinstance(job, dict) else None + names.add(str(declared) if declared else str(job_id)) + return names + + +def _matches_context(names: set[str], context: str) -> bool: + """Report whether declared check-run `names` can produce `context`. + + A context listed in ``MATRIX_NAME_TEMPLATES`` is satisfied **only** by the + template. Branch protection names the expanded contexts (``... (actions)``, + ``... (python)``), which exist solely because the job name interpolates the + matrix; a plain ``name:`` without the suffix reports one check run under the + bare name and never produces the expanded contexts protection waits for. + Accepting the bare spelling as well would therefore pass the exact rename that + blocks every pull request -- so the template is required where one is + registered, and the direct name is accepted only where none is. + """ + template = MATRIX_NAME_TEMPLATES.get(context) + if template is not None: + return template in names + return context in names + + +def _declares_context(workflow_name: str, context: str) -> bool: + """Report whether one workflow can report `context` as a check-run name.""" + return _matches_context(_effective_check_names(workflow_name), context) + + +def test_every_required_context_is_declared_by_a_job() -> None: + """Each required status context is still declared as a job name or job id.""" + missing = [] + for context, workflow_name in sorted(REQUIRED_CONTEXT_SOURCES.items()): + if not _declares_context(workflow_name, context): + missing.append(f"{context!r} not declared by any job in {workflow_name}") + assert not missing, ( + "Required status contexts lost their producing job. Branch protection will " + "wait forever for these and every pull request will stay blocked:\n " + + "\n ".join(missing) + ) + + +def test_required_context_workflow_files_exist() -> None: + """Every workflow named as a context source is present.""" + absent = sorted( + { + workflow_name + for workflow_name in REQUIRED_CONTEXT_SOURCES.values() + if not (WORKFLOW_DIR / workflow_name).is_file() + } + ) + assert not absent, f"Required-context workflow files are missing: {absent}" + + +def test_matrix_templates_reference_only_known_contexts() -> None: + """The matrix-template override table cannot name an unlisted context.""" + unknown = sorted(set(MATRIX_NAME_TEMPLATES) - set(REQUIRED_CONTEXT_SOURCES)) + assert not unknown, f"Matrix templates name unlisted contexts: {unknown}" + + +def test_matrix_context_rejects_the_unexpanded_job_name() -> None: + """A matrix context is not satisfied by a `name:` that drops the interpolation. + + Branch protection requires the *expanded* contexts (`CodeQL compatibility + analysis (actions)` and `... (python)`), which are produced only because the job + name interpolates `${{ matrix.language }}`. Renaming the job to the bare + `CodeQL compatibility analysis` still reports a check run, so the rename looks + harmless -- but it reports the bare name, the expanded contexts are never + produced, and every pull request blocks with no failing check. This asserts the + lookup rejects that spelling instead of accepting it as a direct match. + """ + for context, template in sorted(MATRIX_NAME_TEMPLATES.items()): + assert template != context, ( + f"{context!r} is registered as a matrix template but its template is " + "identical to the context, so this check proves nothing" + ) + assert _matches_context(names={template}, context=context), ( + f"{context!r} must be satisfied by its template {template!r}" + ) + assert not _matches_context(names={context}, context=context), ( + f"{context!r} was satisfied by the unexpanded job name {context!r}. " + "Branch protection waits for the expanded contexts, which that name " + "never reports, so this rename would block every pull request." + )