diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index ee79fa4f3..920b1ad25 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,17 +1,42 @@ -## Summary +## Canonical issue -Describe the outcome and the evidence that supports it. +Closes # -## Linked issue +## Outcome -Fixes # +Describe the user or operational result this PR produces. + +## Scope + +- Included: +- Explicitly excluded: + +## Risk + +- Risk level: low / medium / high +- Failure mode: +- Rollback: ## Verification +List exact automated and manual checks, tied to the current head SHA. + - [ ] Focused tests - [ ] Required CI - [ ] Review threads resolved +## Production evidence + +Provide the Vercel preview, production deployment, runtime evidence, or state why production evidence is not applicable. + +## Agent handoff + +- [ ] One canonical issue is linked +- [ ] No competing PR implements the same issue +- [ ] Acceptance criteria are satisfied +- [ ] Required checks pass on the current head +- [ ] Human decision is requested only for product, security, irreversible infrastructure, or production approval + ## Agent provenance Human-authored pull requests may delete this section. Agent-authored pull requests must replace agent-lock-example with agent-lock-manifest and fill the values. Scope and test paths remain authoritative in the linked issue. diff --git a/.github/workflows/AUDIT.md b/.github/workflows/AUDIT.md index 63fa27412..fa02fcc74 100644 --- a/.github/workflows/AUDIT.md +++ b/.github/workflows/AUDIT.md @@ -65,4 +65,9 @@ valid. Referenced paths were checked against the working tree: | `agent-completion-enforcement.yml` | **ADD** | Protected-default-branch verifier that creates the independent **Agent completion enforcement** Check directly against the PR head SHA. It accepts only an exact-head machine-readable report from the configured dedicated GitHub App; missing/stale/mutable evidence, untrusted label provenance, and custom roles all fail closed. The existing `agent-completion/truth-gate` status stays advisory and must not be made required. | -The protected policy at `.github/agent-lock/trusted-publishers.json` starts with empty allowlists and therefore blocks until a repository administrator provisions the dedicated App and trusted actor identities through protected review. The repository ruleset must then require **Agent completion enforcement**, one independent approval, and resolved conversations. \ No newline at end of file +The protected policy at `.github/agent-lock/trusted-publishers.json` starts with empty allowlists and therefore blocks until a repository administrator provisions the dedicated App and trusted actor identities through protected review. The repository ruleset must then require **Agent completion enforcement**, one independent approval, and resolved conversations. + +## 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". | \ No newline at end of file diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 0e5c52aca..f87eb7a92 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -121,6 +121,8 @@ A full audit of this directory was performed (see | Agent completion enforcement | `agent-completion-enforcement.yml` | `pull_request_target`; manual | Creates the independent, head-bound `Agent completion enforcement` Check from protected default-branch code. | +| 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 diff --git a/.github/workflows/pr-governance.yml b/.github/workflows/pr-governance.yml new file mode 100644 index 000000000..e368adeb4 --- /dev/null +++ b/.github/workflows/pr-governance.yml @@ -0,0 +1,173 @@ +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; + } + + 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/.github/workflows/repository-reconciliation.yml b/.github/workflows/repository-reconciliation.yml new file mode 100644 index 000000000..60fb04a93 --- /dev/null +++ b/.github/workflows/repository-reconciliation.yml @@ -0,0 +1,147 @@ +name: Repository Reconciliation + +on: + schedule: + - cron: "17 13 * * *" + workflow_dispatch: + +permissions: + contents: read + issues: write + pull-requests: read + +concurrency: + group: repository-reconciliation + cancel-in-progress: true + +jobs: + report: + runs-on: ubuntu-latest + steps: + - name: Reconcile canonical delivery state + uses: actions/github-script@v8 + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const repoFullName = `${owner}/${repo}`; + const now = Date.now(); + const staleAfterMs = 14 * 24 * 60 * 60 * 1000; + const closingPattern = /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)/gi; + + const pulls = await github.paginate(github.rest.pulls.list, { + owner, repo, state: "open", per_page: 100 + }); + // Fetch all branches (protected and unprotected) so the total metric is accurate. + const branches = await github.paginate(github.rest.repos.listBranches, { + owner, repo, per_page: 100 + }); + // Only track head refs from PRs targeting this repository (not forks) to prevent + // branch-name collisions between fork branches and local branches. + const activeHeads = new Set( + pulls + .filter(pr => pr.head.repo && pr.head.repo.full_name === repoFullName) + .map(pr => pr.head.ref) + ); + + // Collect all unique issue numbers referenced across open PRs and validate each one + // against the Issues API before using them for classification. This prevents textual + // references like "Closes #999999" from creating fictitious duplicate groups. + const allIssueNumbers = new Set(); + for (const pr of pulls) { + const nums = [...(pr.body || "").matchAll(closingPattern)].map(m => Number(m[1])); + nums.forEach(n => allIssueNumbers.add(n)); + } + const validIssues = new Set(); + for (const issueNum of allIssueNumbers) { + try { + const resp = await github.rest.issues.get({ owner, repo, issue_number: issueNum }); + if (!resp.data.pull_request && resp.data.state === "open") { + validIssues.add(issueNum); + } + } catch (err) { + if (err.status !== 404) throw err; + // 404 → non-existent; skip silently + } + } + + const untracked = []; + const issueToPulls = new Map(); + for (const pr of pulls) { + const issues = [...(pr.body || "").matchAll(closingPattern)] + .map(match => Number(match[1])); + // Restrict to validated issue references only. + const validUnique = [...new Set(issues)].filter(n => validIssues.has(n)); + // Drafts mirror the governance workflow's deferred-enforcement rule and are excluded. + if (validUnique.length !== 1 && !pr.draft) untracked.push(pr); + for (const issue of validUnique) { + const existing = issueToPulls.get(issue) || []; + existing.push(pr.number); + issueToPulls.set(issue, existing); + } + } + + const duplicates = [...issueToPulls.entries()] + .filter(([, numbers]) => numbers.length > 1); + + const staleBranches = []; + for (const branch of branches) { + // Exclude main, protected branches, and branches attached to open PRs. + if (branch.name === "main" || branch.protected || activeHeads.has(branch.name)) continue; + const commit = await github.rest.repos.getCommit({ + owner, repo, ref: branch.commit.sha + }); + const date = commit.data.commit.committer?.date || commit.data.commit.author?.date; + if (date && now - new Date(date).getTime() > staleAfterMs) { + staleBranches.push({ name: branch.name, date, sha: branch.commit.sha.slice(0, 8) }); + } + } + + const lines = [ + "## Canonical delivery-state reconciliation", + "", + `Generated: ${new Date().toISOString()}`, + "", + `- Open PRs: **${pulls.length}**`, + `- Total remote branches: **${branches.length}**`, + `- Ready PRs without exactly one canonical issue: **${untracked.length}**`, + `- Issues with competing implementation PRs: **${duplicates.length}**`, + `- Unattached branches older than 14 days: **${staleBranches.length}**`, + "", + "### PRs requiring canonical issue", + untracked.length + ? untracked.map(pr => `- #${pr.number} — ${pr.title}`).join("\n") + : "- None", + "", + "### Competing PRs", + duplicates.length + ? duplicates.map(([issue, numbers]) => `- Issue #${issue}: ${numbers.map(n => `#${n}`).join(", ")}`).join("\n") + : "- None", + "", + "### Stale unattached branches", + staleBranches.length + ? staleBranches.slice(0, 100).map(branch => + `- \`${branch.name}\` — ${branch.sha}, last commit ${branch.date}` + ).join("\n") + : "- None", + "", + "> This report is intentionally non-destructive. Branch deletion requires a merged PR or an explicit retention decision.", + "", + "Canonical governance: #898" + ]; + + const title = "[automation] Repository drift report"; + const query = `repo:${owner}/${repo} is:issue is:open in:title "${title}"`; + const existing = await github.rest.search.issuesAndPullRequests({ + q: query, per_page: 10 + }); + const report = existing.data.items.find(item => item.title === title); + const body = lines.join("\n"); + + if (report) { + await github.rest.issues.update({ + owner, repo, issue_number: report.number, body + }); + } else { + await github.rest.issues.create({ owner, repo, title, body }); + } diff --git a/tests/unit/test_pr_governance_workflow.py b/tests/unit/test_pr_governance_workflow.py new file mode 100644 index 000000000..fd342b808 --- /dev/null +++ b/tests/unit/test_pr_governance_workflow.py @@ -0,0 +1,100 @@ +from __future__ import annotations + +from pathlib import Path + +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" + ) diff --git a/tests/unit/test_repository_reconciliation_workflow.py b/tests/unit/test_repository_reconciliation_workflow.py new file mode 100644 index 000000000..6c47786e5 --- /dev/null +++ b/tests/unit/test_repository_reconciliation_workflow.py @@ -0,0 +1,104 @@ +from __future__ import annotations + +from pathlib import Path + +import yaml + + +WORKFLOW_PATH = ( + Path(__file__).resolve().parents[2] / ".github/workflows/repository-reconciliation.yml" +) + + +def _load_workflow() -> dict: + assert WORKFLOW_PATH.exists(), "Repository reconciliation workflow should exist" + return yaml.safe_load(WORKFLOW_PATH.read_text()) + + +def _get_script(workflow: dict) -> str: + steps = workflow["jobs"]["report"]["steps"] + script_step = next( + step for step in steps if "Reconcile" in step.get("name", "") + ) + return script_step["with"]["script"] + + +def test_reconciliation_workflow_file_is_valid_yaml() -> None: + workflow = _load_workflow() + assert workflow["name"] == "Repository Reconciliation" + + +def test_reconciliation_workflow_triggers_on_schedule_and_dispatch() -> None: + workflow = _load_workflow() + # PyYAML parses the YAML 'on' key as Python True. + triggers = workflow[True] + assert "schedule" in triggers + assert "workflow_dispatch" in triggers + crons = [entry["cron"] for entry in triggers["schedule"]] + assert len(crons) >= 1 + + +def test_reconciliation_workflow_minimum_permissions() -> None: + workflow = _load_workflow() + perms = workflow["permissions"] + assert perms.get("contents") == "read" + assert perms.get("pull-requests") == "read" + # Needs write to upsert the drift report issue. + assert perms.get("issues") == "write" + + +def test_reconciliation_workflow_excludes_draft_prs_from_untracked() -> None: + """Draft PRs must not be counted as governance drift in the untracked list.""" + script = _get_script(_load_workflow()) + assert "pr.draft" in script, ( + "Draft PRs must be excluded from the untracked list; governance defers enforcement for drafts." + ) + + +def test_reconciliation_workflow_validates_issue_numbers_via_api() -> None: + """Issue numbers referenced in PR bodies must be validated through the Issues API.""" + script = _get_script(_load_workflow()) + assert "github.rest.issues.get" in script, ( + "Issue numbers must be validated via the Issues API to prevent fictitious duplicate groups." + ) + # Must verify it's a real issue (not a PR number). + assert "pull_request" in script + # Must handle 404 (non-existent references). + assert "404" in script + + +def test_reconciliation_workflow_restricts_active_heads_to_same_repo() -> None: + """activeHeads must only include branches from the same repository, not forks.""" + script = _get_script(_load_workflow()) + assert "head.repo" in script and "full_name" in script, ( + "activeHeads must filter by pr.head.repo.full_name to exclude fork branch names." + ) + + +def test_reconciliation_workflow_stale_cutoff_is_positive() -> None: + """The stale-branch cutoff must be a positive number of milliseconds.""" + script = _get_script(_load_workflow()) + assert "staleAfterMs" in script + # The constant must appear as a numeric expression > 0. + assert "14 * 24 * 60 * 60 * 1000" in script or "staleAfterMs = " in script + + +def test_reconciliation_workflow_total_branches_metric_is_accurate() -> None: + """The branches metric must correctly reflect what was fetched (all branches).""" + script = _get_script(_load_workflow()) + # Should NOT fetch with protected: false, because that excludes protected branches. + assert "protected: false" not in script, ( + "Fetching with protected: false excludes protected branches and makes the total inaccurate." + ) + # The label in the report must say "Total remote branches" (includes all fetched). + assert "Total remote branches" in script + + +def test_reconciliation_workflow_report_is_idempotent() -> None: + """Running the reconciliation twice must upsert a single issue, not create duplicates.""" + script = _get_script(_load_workflow()) + # Should search for the existing report issue. + assert "search.issuesAndPullRequests" in script or "issuesAndPullRequests" in script + # Should update the existing issue if found, otherwise create a new one. + assert "issues.update" in script + assert "issues.create" in script