diff --git a/.github/workflows/AUDIT.md b/.github/workflows/AUDIT.md index 9040cf04f..8bfd14a5b 100644 --- a/.github/workflows/AUDIT.md +++ b/.github/workflows/AUDIT.md @@ -62,15 +62,14 @@ valid. Referenced paths were checked against the working tree: `gh issue edit` used a GraphQL mutation unsupported by GitHub App installation tokens; the workflow now calls the REST assignees endpoint. - +## Agent completion enforcement ## Agent completion enforcement -The agent-lock trust policy and both agent-completion Checks were removed as unsatisfiable; `pr-governance.yml` is now the sole binding gate. See `MERGE_POLICY.md`. - +The agent-lock trust policy and both agent-completion Checks were removed as unsatisfiable; the governance workflow was later removed, and binding is no longer enforced by an automated check. See `MERGE_POLICY.md`. +## Repository governance workflows ## Repository governance workflows -| `pr-governance.yml` | **ADD** | Validates that every non-draft ready PR links exactly one real open issue (not a PR number) with non-empty delivery evidence sections (Outcome, Risk, Verification, Production evidence). Fails closed on competing implementation PRs. Triggers on `pull_request_target`. | | `repository-reconciliation.yml` | **ADD** | Scheduled (13:17 UTC daily) non-destructive reconciliation report: identifies ready PRs missing a canonical issue, issues with competing implementation PRs (references validated via Issues API), and stale unattached branches. Excludes draft PRs and fork-branch name collisions. Upserts a single issue titled "[automation] Repository drift report". | ## Multi-agent pipeline alignment (Phase 1) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 5d089066b..cf3157870 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -171,18 +171,16 @@ A full audit of this directory was performed (see - [pytest-cov Documentation](https://pytest-cov.readthedocs.io/) -| PR Governance | `pr-governance.yml` | `pull_request_target` (opened/edited/reopened/synchronize/ready_for_review) | Validates that every ready PR links exactly one real open canonical issue and contains non-empty delivery evidence sections; fails on competing PRs. | | Repository Reconciliation | `repository-reconciliation.yml` | daily (13:17 UTC); manual | Non-destructive daily report of ready PRs missing a canonical issue, issues with competing implementation PRs, and stale unattached branches. | ## Agent-completion enforcement Removed. The `agent-completion/truth-gate` status and the `Agent completion enforcement` Check were retired because they were unsatisfiable: the gate scored a -pull request against an intent snapshot written only on `issues` events, so any -pull request that satisfied `PR Governance` (which requires `Closes #`) -necessarily armed the gate and then failed it. It was red on ~100% of pull -requests, including merged ones such as #1368. - -Binding a pull request to one focused issue is now owned solely by -`pr-governance.yml`, which produces the `PR Governance` and `Canonical issue and -evidence` Checks. See `MERGE_POLICY.md` at the repository root. +pull request against an intent snapshot written only on `issues` events, so a pull +request with a `Closes #` binding necessarily armed the gate and then +failed it. It was red on ~100% of pull requests, including merged ones such as +#1368. + +Binding a pull request to one focused issue is now a policy-level requirement +with no automated gate. See `MERGE_POLICY.md` at the repository root. diff --git a/.github/workflows/dependabot-auto-merge.yml b/.github/workflows/dependabot-auto-merge.yml index dc674bc21..76acb48a0 100644 --- a/.github/workflows/dependabot-auto-merge.yml +++ b/.github/workflows/dependabot-auto-merge.yml @@ -255,8 +255,6 @@ jobs: 'CodeQL', 'gitleaks (working tree)', 'dependency-review', - 'PR Governance', - 'Canonical issue and evidence', 'Security Scan - python', 'Security Scan - javascript', 'bandit', diff --git a/.github/workflows/pr-governance.yml b/.github/workflows/pr-governance.yml deleted file mode 100644 index 660b5b173..000000000 --- a/.github/workflows/pr-governance.yml +++ /dev/null @@ -1,204 +0,0 @@ -name: PR Governance - -on: - pull_request_target: - types: [opened, edited, reopened, synchronize, ready_for_review] - -permissions: - checks: write - contents: read - issues: read - pull-requests: read - -concurrency: - group: pr-governance-${{ github.event.pull_request.number }} - cancel-in-progress: true - -jobs: - policy: - name: Canonical issue and evidence - runs-on: ubuntu-latest - steps: - - name: Validate delivery contract and publish exact-head Check - uses: actions/github-script@v8 - with: - script: | - const pr = context.payload.pull_request; - const runUrl = - `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; - - async function publish(conclusion, title, summary) { - await github.rest.checks.create({ - owner: context.repo.owner, - repo: context.repo.repo, - name: "PR Governance", - head_sha: pr.head.sha, - status: "completed", - conclusion, - details_url: runUrl, - output: { - title, - summary: summary.slice(0, 60000) - } - }); - if (conclusion === "failure") { - core.setFailed(summary); - } - } - - if (pr.draft) { - await publish( - "neutral", - "Governance deferred for draft PR", - `Draft PR #${pr.number} is not enforced. The Check is bound to exact head ${pr.head.sha}.` - ); - 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) { - const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - const pattern = new RegExp( - escapedHeading + "\\s*\\n([\\s\\S]*?)(?=\\n## |$)", - "i" - ); - const match = text.match(pattern); - if (!match) return null; - return match[1].replace(//g, "").trim(); - } - - const placeholderPatterns = [ - /^Describe the user or operational result this PR produces\.?$/i, - /^List exact automated and manual checks, tied to the current head SHA\.?$/i, - /^Provide the Vercel preview, production deployment, runtime evidence, or state why production evidence is not applicable\.?$/i, - /^-\s*Risk level:\s*low\s*\/\s*medium\s*\/\s*high\s*$/i, - /^-\s*Failure mode:\s*$/i, - /^-\s*Rollback:\s*$/i, - /^-\s*\[\s\]\s*(Focused tests|Required CI|Review threads resolved)\s*$/i, - /^(Closes?|Fix(?:es|ed)?|Resolves?)\s+#\s*$/i - ]; - - function hasMeaningfulContent(content) { - if (content === null) return false; - const meaningfulLines = content - .split(/\r?\n/) - .map(line => line.trim()) - .filter(Boolean) - .filter(line => !placeholderPatterns.some(pattern => pattern.test(line))); - return meaningfulLines.length > 0; - } - - const requiredSections = [ - "## Canonical issue", - "## Outcome", - "## Risk", - "## Verification", - "## Production evidence" - ]; - const findings = requiredSections - .filter(section => !hasMeaningfulContent(getSectionContent(body, section))) - .map(section => `${section} is missing or still contains only template placeholders`); - - const closingPattern = - /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)/gi; - const canonicalIssues = [ - ...new Set( - [...body.matchAll(closingPattern)].map(match => Number(match[1])) - ) - ]; - - if (canonicalIssues.length !== 1) { - findings.push("exactly one closing reference is required: Closes #"); - } - - if (canonicalIssues.length === 1) { - const canonical = canonicalIssues[0]; - try { - const issueResp = await github.rest.issues.get({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: canonical - }); - const issue = issueResp.data; - if (issue.pull_request) { - findings.push(`#${canonical} is a pull request, not an issue`); - } else if (issue.state !== "open") { - findings.push(`#${canonical} is not open (state: ${issue.state})`); - } - } catch (error) { - if (error.status === 404) { - findings.push(`#${canonical} does not exist in this repository`); - } else { - throw error; - } - } - - if (findings.length === 0) { - const pulls = await github.paginate(github.rest.pulls.list, { - owner: context.repo.owner, - repo: context.repo.repo, - state: "open", - per_page: 100 - }); - const competing = pulls.filter(candidate => { - if (candidate.number === pr.number) return false; - const matches = [ - ...(candidate.body || "").matchAll(closingPattern) - ].map(match => Number(match[1])); - return matches.includes(canonical); - }); - if (competing.length) { - findings.push( - `Issue #${canonical} already has another open implementation PR: ` + - competing.map(candidate => `#${candidate.number}`).join(", ") - ); - } - } - } - - if (findings.length) { - await publish( - "failure", - "Canonical delivery contract blocked", - findings.join("; ") - ); - return; - } - - await publish( - "success", - "Canonical delivery contract verified", - `PR #${pr.number} has one real open canonical issue and meaningful evidence. Verified exact head ${pr.head.sha}.` - ); diff --git a/.jules/agent_orchestration_sop.md b/.jules/agent_orchestration_sop.md index e87e66f0e..43da00f5a 100644 --- a/.jules/agent_orchestration_sop.md +++ b/.jules/agent_orchestration_sop.md @@ -62,9 +62,6 @@ Before a PR advances: - all current review findings are fixed and resolved with evidence; - a current-head independent review exists; - deployment evidence is bound to the same head, or deployment is explicitly non-applicable; -- `PR Governance` / `Canonical issue and evidence` report the real remaining blockers - (this bullet named `agent-completion/truth-gate` until #1434 retired it — the gate - was red on ~100% of pull requests, so it never reported real blockers); - the focused issue and #898 are updated with exact evidence. Vercel proves the Next.js application build and runtime only. It does not prove Python, Cloud Run, Cloud SQL, worker, webhook, or credential behavior unless those paths are explicitly exercised. diff --git a/MERGE_POLICY.md b/MERGE_POLICY.md index 9997092ca..f4843dae4 100644 --- a/MERGE_POLICY.md +++ b/MERGE_POLICY.md @@ -58,8 +58,8 @@ A pull request may merge when all of the following hold. Exactly one `Closes #` reference, and the pull request description follows `.github/pull_request_template.md`. -*Already enforced by the `PR Governance` and `Canonical issue and evidence` -checks. These work — keep them.* +Binding is a policy-level, author-declared requirement. There is no automated +gate for it. ### 2. Required checks green on the head commit @@ -78,14 +78,11 @@ The real contexts, as observed on live pull requests: | Security | `Security Scan - python`, `Security Scan - javascript`, `bandit`, `python-safety`, `npm-audit`, `trivy` | | Secrets | `gitleaks (working tree)` | | Dependencies | `dependency-review` | -| Binding | `PR Governance`, `Canonical issue and evidence` | Required for every pull request: `validate`, `guards`, `lint-python`, `lint-frontend`, `build`, `test`, `test-frontend`, `CodeQL`, -`gitleaks (working tree)`, `dependency-review`, `PR Governance`, -`Canonical issue and evidence`, `Security Scan - python`, -`Security Scan - javascript`, `bandit`, `python-safety`, `npm-audit`, -`trivy`. +`gitleaks (working tree)`, `dependency-review`, `Security Scan - python`, +`Security Scan - javascript`, `bandit`, `python-safety`, `npm-audit`, `trivy`. > **`test` vs `test-frontend`.** The CI job id/name `test` runs **Python** > pytest only. Frontend unit tests (apps/web vitest, including CWE-209 / diff --git a/tests/unit/test_dependabot_automation_workflow.py b/tests/unit/test_dependabot_automation_workflow.py index d1179d6ff..0afdbe0ef 100644 --- a/tests/unit/test_dependabot_automation_workflow.py +++ b/tests/unit/test_dependabot_automation_workflow.py @@ -195,8 +195,6 @@ def test_dependabot_ignores_eslint_v10() -> None: "CodeQL", "gitleaks (working tree)", "dependency-review", - "PR Governance", - "Canonical issue and evidence", "Security Scan - python", "Security Scan - javascript", "bandit", @@ -298,17 +296,14 @@ def test_merge_gate_blocks_on_a_failing_check_run(tmp_path: Path) -> None: def test_merge_gate_treats_skipped_and_neutral_as_satisfied(tmp_path: Path) -> None: """`MERGE_POLICY.md` gate 2 lists conditional checks; `E2E Pipeline Tests` - is routinely `skipped` and `PR Governance` `neutral`. Neither should - deadlock a merge.""" + is routinely `skipped` and should not deadlock a merge.""" outcome = _run_merge_gate( tmp_path, { "commitMessage": DIRECT_PATCH_COMMIT, "combinedState": "success", "checkRuns": [ - *_all_required_green( - **{"PR Governance": {"conclusion": "neutral"}} - ), + *_all_required_green(), { "name": "E2E Pipeline Tests", "status": "completed", diff --git a/tests/unit/test_pr_governance_workflow.py b/tests/unit/test_pr_governance_workflow.py deleted file mode 100644 index a705ea37e..000000000 --- a/tests/unit/test_pr_governance_workflow.py +++ /dev/null @@ -1,293 +0,0 @@ -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" - - -def _load_workflow() -> dict: - assert WORKFLOW_PATH.exists(), "PR governance workflow should exist" - return yaml.safe_load(WORKFLOW_PATH.read_text()) - - -def _get_script(workflow: dict) -> str: - steps = workflow["jobs"]["policy"]["steps"] - script_step = next( - step - for step in steps - if "Validate delivery contract" in step.get("name", "") - ) - return script_step["with"]["script"] - - -def test_governance_workflow_file_is_valid_yaml() -> None: - workflow = _load_workflow() - assert workflow["name"] == "PR Governance" - - -def test_governance_workflow_triggers_on_pull_request_target() -> None: - workflow = _load_workflow() - # PyYAML parses the YAML 'on' key as Python True. - triggers = workflow[True] - assert "pull_request_target" in triggers - types = triggers["pull_request_target"]["types"] - assert "opened" in types - assert "synchronize" in types - assert "ready_for_review" in types - - -def test_governance_workflow_uses_minimum_permissions() -> None: - workflow = _load_workflow() - perms = workflow["permissions"] - assert perms.get("checks") == "write" - assert perms.get("contents") == "read" - assert perms.get("pull-requests") == "read" - assert perms.get("issues") == "read" - assert set(perms) == {"checks", "contents", "issues", "pull-requests"} - - -def test_governance_workflow_publishes_exact_head_check() -> None: - script = _get_script(_load_workflow()) - assert 'name: "PR Governance"' in script - assert "github.rest.checks.create" in script - assert "head_sha: pr.head.sha" in script - assert 'status: "completed"' in script - - -def test_governance_workflow_draft_bypass_is_head_bound() -> None: - script = _get_script(_load_workflow()) - assert "pr.draft" in script - assert '"neutral"' in script - assert "Governance deferred for draft PR" in script - assert "pr.head.sha" in script - - -def test_governance_workflow_rejects_default_placeholders() -> None: - script = _get_script(_load_workflow()) - assert "placeholderPatterns" in script - assert "hasMeaningfulContent" in script - assert "Describe the user or operational result" in script - assert "Risk level:" in script - assert "Focused tests" in script - assert "meaningfulLines.length > 0" in script - assert r'replace(//g, "").trim()' in script - assert r'replace(//g, "").trim()' not in script - - -def test_governance_workflow_validates_issue_via_api() -> None: - script = _get_script(_load_workflow()) - assert "github.rest.issues.get" in script - assert "pull_request" in script - assert "issue.state" in script - assert "404" in script - - -def test_governance_workflow_detects_competing_prs() -> None: - script = _get_script(_load_workflow()) - assert "github.paginate" in script - assert "github.rest.pulls.list" in script - assert "competing" in script - assert "another open implementation PR" in script - - -def test_governance_workflow_checks_issue_before_competitors() -> None: - script = _get_script(_load_workflow()) - 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 -
  • fd783ac chore: sync actions from gh-aw@v0.84.2
-""" - -_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"