diff --git a/.github/workflows/pr-governance.yml b/.github/workflows/pr-governance.yml index e368adeb4..660b5b173 100644 --- a/.github/workflows/pr-governance.yml +++ b/.github/workflows/pr-governance.yml @@ -55,6 +55,37 @@ jobs: return; } + // Dependabot composes its body from a fixed template (release + // notes, changelog, commit list). It cannot emit the five required + // headings or a `Closes #` reference, so the contract below + // is not a bar it fails -- it is one it has no way to clear. + // + // The sibling `agent-completion/truth-gate` used to carry the same + // escape and defer the issue-binding requirement here, on the + // stated ground that this check "states a requirement an author can + // actually meet" -- which was false for Dependabot, and relocated + // the constraint rather than removing it. That gate was retired in + // #1431, so this is now the only place the exemption lives. + // + // `neutral`, not `success`: the contract is not applicable here, + // and reporting it as satisfied would be the same false signal this + // check exists to catch. Dependency PRs remain gated by + // `dependency-review`, `npm-audit`, `trivy`, `build` and `test`. + const AUTOMATED_DEPENDENCY_AUTHORS = new Set([ + "dependabot[bot]" + ]); + const prAuthor = (pr.user && pr.user.login) || ""; + if (AUTOMATED_DEPENDENCY_AUTHORS.has(prAuthor)) { + await publish( + "neutral", + "Governance not applicable to automated dependency PR", + `PR #${pr.number} is authored by ${prAuthor}, which cannot ` + + `author a canonical delivery contract. The Check is bound to ` + + `exact head ${pr.head.sha}.` + ); + return; + } + const body = pr.body || ""; function getSectionContent(text, heading) { diff --git a/tests/unit/test_pr_governance_workflow.py b/tests/unit/test_pr_governance_workflow.py index 216561e8c..a705ea37e 100644 --- a/tests/unit/test_pr_governance_workflow.py +++ b/tests/unit/test_pr_governance_workflow.py @@ -1,7 +1,12 @@ from __future__ import annotations +import json +import shutil +import subprocess +import textwrap from pathlib import Path +import pytest import yaml WORKFLOW_PATH = Path(__file__).resolve().parents[2] / ".github/workflows/pr-governance.yml" @@ -97,3 +102,192 @@ def test_governance_workflow_checks_issue_before_competitors() -> None: assert script.index("github.rest.issues.get") < script.index( "github.rest.pulls.list" ) + + +# -------------------------------------------------------------------------- +# Behavioural tests (#1419). +# +# The assertions above match strings in the script source, so they stay green +# even if the logic inverts. These run the real script under Node against +# synthetic payloads and assert on the conclusion it publishes. +# -------------------------------------------------------------------------- + +CONTRACT_BODY = """## Canonical issue + +Closes #1419 + +## Outcome + +Real outcome text. + +## Risk + +- Risk level: low +- Failure mode: none observed +- Rollback: git revert + +## Verification + +Ran the focused suite. + +## Production evidence + +Not applicable - workflow-only change. +""" + +# Shape of a real Dependabot body: release notes and a commit list, none of +# the five required headings, no closing reference. Dependabot composes this +# from a fixed template and cannot be made to emit the contract. +DEPENDABOT_BODY = """Bumps [github/gh-aw-actions/setup](https://github.com/github/gh-aw-actions) from 0.82.14 to 0.84.2. + +Release notes +

Sourced from setup's releases.

+Commits + +""" + +_DRIVER = """ +const fs = require('fs'); +const source = fs.readFileSync(process.argv[2], 'utf8'); +const pr = JSON.parse(process.argv[3]); + +(async () => { + const published = []; + const context = { + payload: { pull_request: pr }, + repo: { owner: 'groupthinking', repo: 'EventRelay' }, + runId: 1, + serverUrl: 'https://github.com', + }; + const core = { setFailed: () => {} }; + const github = { + rest: { + checks: { create: async (args) => { published.push(args); } }, + // Canonical issue resolves to a real, open, non-PR issue. + issues: { get: async () => ({ data: { number: 1419, state: 'open' } }) }, + pulls: { list: 'list' }, + }, + paginate: async () => [], // no competing PRs + }; + const fn = new Function( + 'context', 'core', 'github', + `return (async () => {${source}})();` + ); + await fn(context, core, github); + process.stdout.write(JSON.stringify({ + conclusion: published[0] && published[0].conclusion, + title: published[0] && published[0].output && published[0].output.title, + })); +})(); +""" + + +def _run_gate(tmp_path: Path, pull_request: dict) -> dict: + """Execute the real governance script against a synthetic PR payload.""" + script = _get_script(_load_workflow()) + source_path = tmp_path / "gov_source.js" + source_path.write_text(script) + driver_path = tmp_path / "driver.js" + driver_path.write_text(textwrap.dedent(_DRIVER)) + + result = subprocess.run( + ["node", str(driver_path), str(source_path), json.dumps(pull_request)], + capture_output=True, + text=True, + timeout=60, + check=False, + ) + assert result.returncode == 0, f"driver failed: {result.stderr}" + return json.loads(result.stdout) + + +requires_node = pytest.mark.skipif( + shutil.which("node") is None, reason="node is required to execute the gate" +) + + +@requires_node +def test_gate_is_not_applicable_to_dependabot(tmp_path: Path) -> None: + """Dependabot cannot author the contract, so the gate must not fail it. + + Regression guard for #1419: PR #1171 was permanently red on this check. + """ + verdict = _run_gate( + tmp_path, + { + "number": 1171, + "draft": False, + "user": {"login": "dependabot[bot]"}, + "body": DEPENDABOT_BODY, + "head": {"sha": "bfb1bb7"}, + }, + ) + assert verdict["conclusion"] == "neutral" + # "neutral", not "success": the contract is not applicable, not satisfied. + assert "not applicable" in verdict["title"].lower() + + +@requires_node +def test_gate_still_fails_a_human_with_the_same_body(tmp_path: Path) -> None: + """The exemption is keyed on author, not on body shape. + + Without this, a fix that simply stopped requiring the sections would also + pass test_gate_is_not_applicable_to_dependabot. + """ + verdict = _run_gate( + tmp_path, + { + "number": 9001, + "draft": False, + "user": {"login": "groupthinking"}, + "body": DEPENDABOT_BODY, + "head": {"sha": "deadbee"}, + }, + ) + assert verdict["conclusion"] == "failure" + + +@requires_node +def test_gate_still_fails_other_bots(tmp_path: Path) -> None: + """Only automated dependency authors are exempt, not every bot.""" + verdict = _run_gate( + tmp_path, + { + "number": 9003, + "draft": False, + "user": {"login": "google-labs-jules[bot]"}, + "body": DEPENDABOT_BODY, + "head": {"sha": "abc0001"}, + }, + ) + assert verdict["conclusion"] == "failure" + + +@requires_node +def test_gate_passes_a_complete_contract(tmp_path: Path) -> None: + verdict = _run_gate( + tmp_path, + { + "number": 9002, + "draft": False, + "user": {"login": "groupthinking"}, + "body": CONTRACT_BODY, + "head": {"sha": "cafe123"}, + }, + ) + assert verdict["conclusion"] == "success" + + +@requires_node +def test_gate_tolerates_a_missing_user_object(tmp_path: Path) -> None: + """The author lookup must not throw when `user` is absent.""" + verdict = _run_gate( + tmp_path, + { + "number": 9004, + "draft": False, + "body": CONTRACT_BODY, + "head": {"sha": "f00d111"}, + }, + ) + assert verdict["conclusion"] == "success"