diff --git a/.github/workflows/pr-body-autoinject.yml b/.github/workflows/pr-body-autoinject.yml index e6b30e9..262dd43 100644 --- a/.github/workflows/pr-body-autoinject.yml +++ b/.github/workflows/pr-body-autoinject.yml @@ -3,7 +3,8 @@ name: PR Body Auto-Inject (reusable) # Bot-authored PRs (Copilot SWE agent, GitHub Apps) freehand their bodies and # do not read .github/PULL_REQUEST_TEMPLATE.md, so they fail the pr-policy # Verification/Closes-N requirements. This workflow runs ONCE when a bot -# opens a non-doc PR and prepends a minimal stub that satisfies pr-policy. +# opens a non-doc PR and prepends a minimal scaffold. The scaffold is +# intentionally invalid until the agent records real verification. # Human PRs are untouched. on: @@ -77,19 +78,25 @@ jobs: const stub = [ '', '', - '## Linked Issue', + '## Summary', '', - `Closes ${issueRef}`, + 'TODO: Fill in summary.', '', '## Verification', '', - '- [x] Automated CI checks green on this PR', - '- [ ] Manual verification — N/A for bot-authored fix', + '- [ ] TODO: Run required verification and replace this line.', '', '### Verification Notes', '', - '_Auto-injected for bot-authored PR. CI-green is the verification surface._', - '_Human reviewer: if this PR touches user-visible behavior, replace this block with manual smoke results before merge._', + 'TODO: Replace this scaffold with concrete command output, CI check names, or manual smoke-test notes.', + '', + '## Docs / Changelog', + '', + 'TODO: Record docs/changelog handling.', + '', + '## Linked Issue', + '', + `TODO: Closes ${issueRef}`, '', '---', '', diff --git a/.github/workflows/pr-policy.yml b/.github/workflows/pr-policy.yml index 7eb56f6..988b8db 100644 --- a/.github/workflows/pr-policy.yml +++ b/.github/workflows/pr-policy.yml @@ -24,6 +24,10 @@ on: description: "Require at least one `- [x]` checked box in the body." type: boolean default: true + strict-body-contract: + description: "Require the canonical Summary / Verification / Verification Notes / Docs-Changelog structure." + type: boolean + default: true verification-section-heading: description: "H2 heading exact text (without the `## `)." type: string @@ -155,60 +159,78 @@ jobs: REQUIRE_LINKED_ISSUE: ${{ inputs.require-linked-issue }} REQUIRE_VERIFICATION_SECTION: ${{ inputs.require-verification-section }} REQUIRE_CHECKED_BOX: ${{ inputs.require-checked-box }} + STRICT_BODY_CONTRACT: ${{ inputs.strict-body-contract }} VERIFICATION_H2: ${{ inputs.verification-section-heading }} VERIFICATION_H3: ${{ inputs.verification-notes-heading }} DOC_EXT_LIST: ${{ inputs.doc-only-extensions }} DOC_PREFIXES: ${{ inputs.doc-only-path-prefixes }} + WORKFLOW_LIBRARY_REF: ${{ inputs.workflow-library-ref }} with: script: | + const path = require('path'); + const url = require('url'); + const pr = context.payload.pull_request; if (pr.draft) { core.info('Draft PR; ready-for-review policy is advisory until draft is cleared.'); return; } + const validatorPath = path.resolve( + process.env.GITHUB_WORKSPACE, + '__github-workflows__/scripts/pr-contract.mjs', + ); + const workflowRef = process.env.WORKFLOW_LIBRARY_REF || '(unset)'; + let validatePrContract; + let formatPrContractResult; + try { + ({ validatePrContract, formatPrContractResult } = await import(url.pathToFileURL(validatorPath).href)); + } catch (err) { + const msg = `Could not load PR contract validator at ${validatorPath} (workflow-library-ref=${workflowRef}): ${err && err.message ? err.message : err}.`; + core.setFailed(msg); + return; + } + const files = await github.paginate(github.rest.pulls.listFiles, { owner: context.repo.owner, repo: context.repo.repo, pull_number: pr.number, per_page: 100, }); - const docExtRe = new RegExp(`\\.(${process.env.DOC_EXT_LIST})$`, 'i'); - const prefixes = process.env.DOC_PREFIXES - .split('\n') - .map((s) => s.trim()) - .filter(Boolean); - const isDocFile = (p) => docExtRe.test(p) || prefixes.some((pre) => p.startsWith(pre)); - if (files.length > 0 && files.every((f) => isDocFile(f.filename))) { - core.info(`Doc-only PR (${files.length} file${files.length === 1 ? '' : 's'}); verification ceremony skipped.`); - return; - } - - const body = pr.body || ''; - const failures = []; - if (process.env.REQUIRE_LINKED_ISSUE === 'true') { - if (!/(Closes|Fixes|Refs)\s+#\d+/i.test(body)) { - failures.push('PR body must link an issue with `Closes #N`, `Fixes #N`, or `Refs #N`.'); - } - } - - if (process.env.REQUIRE_VERIFICATION_SECTION === 'true') { - const h2 = new RegExp(`##\\s+${process.env.VERIFICATION_H2}`, 'i'); - const h3 = new RegExp(`###\\s+${process.env.VERIFICATION_H3}`, 'i'); - if (!h2.test(body) || !h3.test(body)) { - failures.push(`PR body must include \`## ${process.env.VERIFICATION_H2}\` and \`### ${process.env.VERIFICATION_H3}\` sections.`); - } - } - - if (process.env.REQUIRE_CHECKED_BOX === 'true') { - if (!/-\s+\[[xX]\]/.test(body)) { - failures.push('PR body must record completed verification with at least one checked checkbox.'); - } - } + const strict = process.env.STRICT_BODY_CONTRACT === 'true'; + const requireVerification = process.env.REQUIRE_VERIFICATION_SECTION === 'true'; + const result = validatePrContract({ + title: pr.title || '', + body: pr.body || '', + branch: pr.head.ref || '', + files: files.map((file) => file.filename), + }, { + requireTitle: false, + requireBranch: false, + docOnlyExtensions: process.env.DOC_EXT_LIST, + docOnlyPathPrefixes: process.env.DOC_PREFIXES, + requireIssueLink: process.env.REQUIRE_LINKED_ISSUE === 'true', + requiredHeadings: strict + ? undefined + : requireVerification + ? [`## ${process.env.VERIFICATION_H2}`, `### ${process.env.VERIFICATION_H3}`] + : [], + requireSummary: strict && requireVerification, + requireDocsChangelog: strict && requireVerification, + requireSubstantiveVerificationNotes: strict && requireVerification, + rejectGenericVerificationNotes: strict && requireVerification, + requireCheckedVerification: process.env.REQUIRE_CHECKED_BOX === 'true', + requireEvidenceBlocks: false, + }); - if (failures.length > 0) { - core.setFailed(failures.join('\n')); + const report = formatPrContractResult(result); + await core.summary + .addHeading(result.ok ? 'PR policy passed' : 'PR policy failed', 3) + .addCodeBlock(report, 'text') + .write(); + if (!result.ok) { + core.setFailed(report); } # ---------------------------------------------------------------------- diff --git a/.github/workflows/repo-required-gate.yml b/.github/workflows/repo-required-gate.yml index ebec05a..b78ca4e 100644 --- a/.github/workflows/repo-required-gate.yml +++ b/.github/workflows/repo-required-gate.yml @@ -54,6 +54,15 @@ on: require-checked-box: type: boolean default: true + doc-only-extensions: + description: "Pipe-separated extensions (no dots) considered doc-only for the PR contract." + type: string + default: "md|txt|png|jpg|jpeg|gif|svg|webp|bmp|ico|avif" + doc-only-path-prefixes: + description: "Newline-separated path prefixes also treated as doc-only for the PR contract." + type: string + default: | + .changelog/ run-pr-contract: type: boolean default: true @@ -295,55 +304,121 @@ jobs: if: inputs.run-pr-contract && github.event_name == 'pull_request' && needs.detect.outputs.ok == 'true' runs-on: ubuntu-latest permissions: - pull-requests: read + contents: read + pull-requests: write steps: + - name: Check out github-workflows for PR contract validator + uses: actions/checkout@v4 + with: + repository: ArchonVII/github-workflows + ref: ${{ inputs.workflow-library-ref }} + path: __github-workflows__ + - name: Enforce PR policy uses: actions/github-script@v7 env: - DOCS_ONLY: ${{ needs.detect.outputs.docs-only }} BRANCH_PATTERN: ${{ inputs.branch-pattern }} REQUIRE_LINKED_ISSUE: ${{ inputs.require-linked-issue }} REQUIRE_VERIFICATION_SECTION: ${{ inputs.require-verification-section }} REQUIRE_CHECKED_BOX: ${{ inputs.require-checked-box }} + DOC_EXT_LIST: ${{ inputs.doc-only-extensions }} + DOC_PREFIXES: ${{ inputs.doc-only-path-prefixes }} + WORKFLOW_LIBRARY_REF: ${{ inputs.workflow-library-ref }} with: script: | + const path = require('path'); + const url = require('url'); + const pr = context.payload.pull_request; if (pr.draft) { core.info('Draft PR; pr contract is advisory until ready_for_review.'); return; } - const failures = []; - const branch = pr.head.ref || ''; - const branchRe = new RegExp(process.env.BRANCH_PATTERN); - if (!branchRe.test(branch)) { - failures.push(`Head branch \`${branch}\` does not match required pattern.`); - } + const validatorPath = path.resolve( + process.env.GITHUB_WORKSPACE, + '__github-workflows__/scripts/pr-contract.mjs', + ); + const ref = process.env.WORKFLOW_LIBRARY_REF || '(unset)'; - if (!/^(feat|fix|refactor|test|docs|style|chore|perf|ci|build|revert)(\([^)]+\))?: .+/.test(pr.title || '')) { - failures.push('PR title must use Conventional Commits format.'); + let validatePrContract; + let formatPrContractResult; + try { + ({ validatePrContract, formatPrContractResult } = await import(url.pathToFileURL(validatorPath).href)); + } catch (err) { + const msg = `Could not load PR contract validator at ${validatorPath} (workflow-library-ref=${ref}): ${err && err.message ? err.message : err}.`; + core.setFailed(msg); + return; } - const docsOnly = process.env.DOCS_ONLY === 'true'; - if (!docsOnly) { - const body = pr.body || ''; - if (process.env.REQUIRE_LINKED_ISSUE === 'true' && !/(Closes|Fixes|Refs)\s+#\d+/i.test(body)) { - failures.push('PR body must link an issue with `Closes #N`, `Fixes #N`, or `Refs #N`.'); - } - if (process.env.REQUIRE_VERIFICATION_SECTION === 'true' && !(/##\s+Verification/i.test(body) && /###\s+Verification Notes/i.test(body))) { - failures.push('PR body must include `## Verification` and `### Verification Notes` sections.'); - } - if (process.env.REQUIRE_CHECKED_BOX === 'true' && !/-\s+\[[xX]\]/.test(body)) { - failures.push('PR body must record completed verification with at least one checked checkbox.'); + const filesPaged = await github.paginate(github.rest.pulls.listFiles, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pr.number, + per_page: 100, + }); + + const requireVerification = process.env.REQUIRE_VERIFICATION_SECTION === 'true'; + const result = validatePrContract({ + title: pr.title || '', + body: pr.body || '', + branch: pr.head.ref || '', + files: filesPaged.map((file) => file.filename), + }, { + branchPattern: process.env.BRANCH_PATTERN, + docOnlyExtensions: process.env.DOC_EXT_LIST, + docOnlyPathPrefixes: process.env.DOC_PREFIXES, + requireIssueLink: process.env.REQUIRE_LINKED_ISSUE === 'true', + requiredHeadings: requireVerification ? undefined : [], + requireSummary: requireVerification, + requireDocsChangelog: requireVerification, + requireSubstantiveVerificationNotes: requireVerification, + rejectGenericVerificationNotes: requireVerification, + requireCheckedVerification: process.env.REQUIRE_CHECKED_BOX === 'true', + requireEvidenceBlocks: process.env.REQUIRE_CHECKED_BOX === 'true', + }); + + const report = formatPrContractResult(result); + await core.summary + .addHeading(result.ok ? 'PR contract passed' : 'PR contract failed', 3) + .addCodeBlock(report, 'text') + .write(); + + if (!result.ok) { + const marker = ''; + const repairBody = [ + marker, + '', + report, + '', + 'Run locally:', + '', + '```bash', + `npm run pr:contract -- --repo ${context.repo.owner}/${context.repo.repo} --pr ${pr.number}`, + '```', + ].join('\n'); + + try { + await github.rest.pulls.createReview({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pr.number, + event: 'COMMENT', + body: repairBody, + }); + } catch (err) { + core.warning(`Could not post PR contract repair comment: ${err && err.message ? err.message : err}`); } - } - if (failures.length) core.setFailed(failures.join('\n')); + core.setFailed(report); + } workflow-validation: name: workflow and hook validation - needs: detect - if: inputs.run-workflow-validation && needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-workflow-validation == 'true' + needs: + - detect + - pr-contract + if: always() && inputs.run-workflow-validation && needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-workflow-validation == 'true' && (github.event_name != 'pull_request' || inputs.run-pr-contract == false || needs.pr-contract.result == 'success') runs-on: ubuntu-latest permissions: contents: read @@ -372,8 +447,10 @@ jobs: policy-validation: name: policy validation - needs: detect - if: inputs.run-policy-validation && needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-policy-validation == 'true' + needs: + - detect + - pr-contract + if: always() && inputs.run-policy-validation && needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-policy-validation == 'true' && (github.event_name != 'pull_request' || inputs.run-pr-contract == false || needs.pr-contract.result == 'success') runs-on: ubuntu-latest permissions: contents: read @@ -390,8 +467,10 @@ jobs: dependency-review: name: dependency review - needs: detect - if: inputs.run-dependency-review && github.event_name == 'pull_request' && needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-dependency-review == 'true' + needs: + - detect + - pr-contract + if: always() && inputs.run-dependency-review && github.event_name == 'pull_request' && needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-dependency-review == 'true' && (github.event_name != 'pull_request' || inputs.run-pr-contract == false || needs.pr-contract.result == 'success') runs-on: ubuntu-latest permissions: contents: read @@ -407,8 +486,10 @@ jobs: node-ci: name: node ci - needs: detect - if: needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-node-ci == 'true' + needs: + - detect + - pr-contract + if: always() && needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-node-ci == 'true' && (github.event_name != 'pull_request' || inputs.run-pr-contract == false || needs.pr-contract.result == 'success') uses: ArchonVII/github-workflows/.github/workflows/node-ci.yml@v1 with: node-versions: ${{ inputs.node-versions }} @@ -422,8 +503,10 @@ jobs: python-ci: name: python ci - needs: detect - if: needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-python-ci == 'true' + needs: + - detect + - pr-contract + if: always() && needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-python-ci == 'true' && (github.event_name != 'pull_request' || inputs.run-pr-contract == false || needs.pr-contract.result == 'success') uses: ArchonVII/github-workflows/.github/workflows/python-ci.yml@v1 with: python-versions: ${{ inputs.python-versions }} @@ -437,8 +520,10 @@ jobs: snapshot-validation: name: snapshot validation - needs: detect - if: needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-snapshot-validation == 'true' + needs: + - detect + - pr-contract + if: always() && needs.detect.outputs.ok == 'true' && needs.detect.outputs.run-snapshot-validation == 'true' && (github.event_name != 'pull_request' || inputs.run-pr-contract == false || needs.pr-contract.result == 'success') runs-on: ubuntu-latest permissions: contents: read diff --git a/README.md b/README.md index dd80bab..e204534 100644 --- a/README.md +++ b/README.md @@ -17,8 +17,8 @@ Companion repos: | Workflow | Purpose | Example | | -------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------- | -| [`pr-policy.yml`](.github/workflows/pr-policy.yml) | Enforce PR body has `## Verification` / `### Verification Notes` / a checked box / a linked issue. Doc-only PRs skip that ceremony. Also warns on role-separation concerns and can hard-gate protected agent PR paths. Also runs `actionlint`. | [`examples/pr-policy.yml`](examples/pr-policy.yml) | -| [`pr-body-autoinject.yml`](.github/workflows/pr-body-autoinject.yml) | When a bot opens a non-doc PR with a freehand body, prepend a stub that satisfies `pr-policy`. Human PRs untouched. | [`examples/pr-body-autoinject.yml`](examples/pr-body-autoinject.yml) | +| [`pr-policy.yml`](.github/workflows/pr-policy.yml) | Enforce the shared PR contract validator: canonical title/body structure, linked issue, verification notes, and checked verification. Doc-only PRs skip body ceremony. Also warns on role-separation concerns and can hard-gate protected agent PR paths. Also runs `actionlint`. | [`examples/pr-policy.yml`](examples/pr-policy.yml) | +| [`pr-body-autoinject.yml`](.github/workflows/pr-body-autoinject.yml) | When a bot opens a non-doc PR with a freehand body, prepend an intentionally incomplete scaffold that agents must fill before ready-for-review. Human PRs untouched. | [`examples/pr-body-autoinject.yml`](examples/pr-body-autoinject.yml) | | [`semantic-pr-title.yml`](.github/workflows/semantic-pr-title.yml) | Enforce Conventional Commits format on PR titles. Wraps `amannn/action-semantic-pull-request@v5`. | [`examples/semantic-pr-title.yml`](examples/semantic-pr-title.yml) | | [`branch-naming.yml`](.github/workflows/branch-naming.yml) | Enforce the `open`-skill branch convention (`agent//-` or `/`). Configurable regex. | [`examples/branch-naming.yml`](examples/branch-naming.yml) | | [`changelog-fragment.yml`](.github/workflows/changelog-fragment.yml) | Require a new file under `.changelog/unreleased/` whenever a PR touches `src/`. Skippable via `no-changelog` label. | [`examples/changelog-fragment.yml`](examples/changelog-fragment.yml) | @@ -58,7 +58,7 @@ Companion repos: ## How to consume -In any repo where you want one of these, copy the example caller into `.github/workflows/` and commit it. Inputs are all optional — defaults are conservative. +In any repo where you want one of these, copy the example caller into `.github/workflows/` and commit it. Inputs are all optional; the PR contract defaults are intentionally strict for ready-for-review. For branch protection, prefer the single-gate contract: @@ -69,6 +69,17 @@ repo-required-gate / decision Keep targeted checks inside the gate or leave them non-required. Do not make branch protection depend on workflows that can be skipped by path filters; GitHub can leave those required checks pending. +For agent closeout, use the shared local preflight instead of direct promotion: + +```bash +npm run agent:close-preflight -- --repo OWNER/REPO --pr 123 +npm run agent:pr-ready -- --repo OWNER/REPO --pr 123 +``` + +Agents must not run `gh pr ready` directly. `agent:pr-ready` fetches the PR title, body, head branch, and changed files; runs `scripts/pr-contract.mjs`; prints exact repair failures; and only then calls `gh pr ready`. + +The canonical scaffold lives at [`contracts/pr-template.md`](contracts/pr-template.md). It is intentionally invalid until the TODOs are replaced with real summary, verification, docs/changelog, and issue-link content. + ```yaml # .github/workflows/pr-policy.yml in a consumer repo name: PR Policy diff --git a/contracts/pr-template.md b/contracts/pr-template.md new file mode 100644 index 0000000..c878b33 --- /dev/null +++ b/contracts/pr-template.md @@ -0,0 +1,19 @@ +## Summary + +TODO: Replace with a concise description of the change. + +## Verification + +- [ ] TODO: Run required verification and replace this line. + +### Verification Notes + +TODO: Record concrete command output, CI check names, or manual smoke-test notes. + +## Docs / Changelog + +TODO: Record docs/changelog handling. + +## Linked Issue + +TODO: Closes #___ diff --git a/examples/pr-policy.yml b/examples/pr-policy.yml index 7707158..e8c804d 100644 --- a/examples/pr-policy.yml +++ b/examples/pr-policy.yml @@ -33,6 +33,7 @@ jobs: # require-linked-issue: true # require-verification-section: true # require-checked-box: true + # strict-body-contract: true # run-actionlint: true # # Warning-only by default. Set true to hard-fail agent-managed PRs # # touching protected paths unless an independent approval or diff --git a/package.json b/package.json index 052d103..61deb12 100644 --- a/package.json +++ b/package.json @@ -3,6 +3,9 @@ "private": true, "type": "module", "scripts": { + "agent:close-preflight": "node scripts/agent-close-preflight.mjs", + "agent:pr-ready": "node scripts/agent-pr-ready.mjs", + "pr:contract": "node scripts/pr-contract.mjs", "test": "vitest run" }, "devDependencies": { diff --git a/scripts/agent-close-preflight.mjs b/scripts/agent-close-preflight.mjs new file mode 100644 index 0000000..9d5db32 --- /dev/null +++ b/scripts/agent-close-preflight.mjs @@ -0,0 +1,116 @@ +#!/usr/bin/env node +import { execFileSync } from 'node:child_process'; +import { + formatPrContractResult, + loadPrFromGh, + validatePrContract, +} from './pr-contract.mjs'; + +function parseArgs(argv) { + const args = {}; + for (let i = 0; i < argv.length; i++) { + const item = argv[i]; + if (!item.startsWith('--')) continue; + + const key = item.slice(2); + if (key === 'json' || key === 'allow-ready' || key === 'skip-git') { + args[key] = true; + continue; + } + args[key] = argv[i + 1]; + i += 1; + } + return args; +} + +function git(args, options = {}) { + return execFileSync('git', args, { + encoding: 'utf8', + stdio: ['ignore', 'pipe', options.ignoreErrors ? 'ignore' : 'pipe'], + }).trim(); +} + +function collectGitFailures({ expectedBranch }) { + const failures = []; + + try { + const status = git(['status', '--porcelain']); + if (status) { + failures.push('Working tree must be clean before close preflight.'); + } + + const branch = git(['branch', '--show-current']); + if (expectedBranch && branch && branch !== expectedBranch) { + failures.push(`Current branch \`${branch}\` does not match PR head branch \`${expectedBranch}\`.`); + } + + let upstream = ''; + try { + upstream = git(['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{u}'], { ignoreErrors: true }); + } catch { + upstream = ''; + } + if (!upstream) { + failures.push('Current branch has no upstream; push the branch before close preflight.'); + } else { + const counts = git(['rev-list', '--left-right', '--count', '@{u}...HEAD']).split(/\s+/); + const ahead = Number(counts[1] || 0); + if (ahead > 0) { + failures.push(`Current branch is ${ahead} commit(s) ahead of upstream; push before close preflight.`); + } + } + } catch (err) { + failures.push(`Could not inspect git state: ${err.message}`); + } + + return failures; +} + +function main() { + const args = parseArgs(process.argv.slice(2)); + const pr = loadPrFromGh({ repo: args.repo, pr: args.pr }); + const contract = validatePrContract(pr, { + branchPattern: args['branch-pattern'], + docOnlyExtensions: args['doc-only-extensions'], + docOnlyPathPrefixes: args['doc-only-path-prefixes'], + }); + + const failures = contract.errors.map((item) => `[${item.code}] ${item.message}`); + if (!args['allow-ready'] && !pr.isDraft) { + failures.push('PR is already ready for review; close preflight expects a draft PR before promotion.'); + } + if (!args['skip-git']) { + failures.push(...collectGitFailures({ expectedBranch: pr.branch })); + } + + const ok = failures.length === 0; + const payload = { + ok, + pr: { + number: pr.number, + url: pr.url, + isDraft: pr.isDraft, + branch: pr.branch, + }, + contract, + failures, + }; + + if (args.json) { + process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); + } else { + process.stdout.write(`${formatPrContractResult(contract)}\n`); + if (failures.length > 0) { + process.stdout.write('\nClose preflight failed:\n'); + for (const failure of failures) { + process.stdout.write(`- ${failure}\n`); + } + } else { + process.stdout.write('Close preflight passed.\n'); + } + } + + process.exitCode = ok ? 0 : 1; +} + +main(); diff --git a/scripts/agent-pr-ready.mjs b/scripts/agent-pr-ready.mjs new file mode 100644 index 0000000..c7573be --- /dev/null +++ b/scripts/agent-pr-ready.mjs @@ -0,0 +1,86 @@ +#!/usr/bin/env node +import { execFileSync } from 'node:child_process'; +import { + formatPrContractResult, + loadPrFromGh, + validatePrContract, +} from './pr-contract.mjs'; + +function parseArgs(argv) { + const args = {}; + for (let i = 0; i < argv.length; i++) { + const item = argv[i]; + if (!item.startsWith('--')) continue; + + const key = item.slice(2); + if (key === 'json' || key === 'dry-run') { + args[key] = true; + continue; + } + args[key] = argv[i + 1]; + i += 1; + } + return args; +} + +function main() { + const args = parseArgs(process.argv.slice(2)); + const pr = loadPrFromGh({ repo: args.repo, pr: args.pr }); + const result = validatePrContract(pr, { + branchPattern: args['branch-pattern'], + docOnlyExtensions: args['doc-only-extensions'], + docOnlyPathPrefixes: args['doc-only-path-prefixes'], + }); + + const payload = { + ok: result.ok, + ready: result.ok && !args['dry-run'], + pr: { + number: pr.number, + url: pr.url, + isDraft: pr.isDraft, + branch: pr.branch, + }, + contract: result, + }; + + if (!result.ok) { + if (args.json) { + process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); + } else { + process.stderr.write(`${formatPrContractResult(result)}\n`); + process.stderr.write('\nRefusing to run `gh pr ready`; fix the PR contract first.\n'); + } + process.exitCode = 1; + return; + } + + if (args['dry-run']) { + if (args.json) { + process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); + } else { + process.stdout.write(`${formatPrContractResult(result)}\nDry run: would promote PR #${pr.number}.\n`); + } + return; + } + + if (!pr.isDraft) { + payload.ready = false; + if (args.json) { + process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); + } else { + process.stdout.write(`${formatPrContractResult(result)}\nPR #${pr.number} is already ready for review.\n`); + } + return; + } + + execFileSync('gh', ['pr', 'ready', String(pr.number), '--repo', args.repo], { stdio: 'inherit' }); + + if (args.json) { + process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); + } else { + process.stdout.write(`${formatPrContractResult(result)}\nPromoted PR #${pr.number} to ready for review.\n`); + } +} + +main(); diff --git a/scripts/pr-contract.mjs b/scripts/pr-contract.mjs new file mode 100644 index 0000000..ea945b9 --- /dev/null +++ b/scripts/pr-contract.mjs @@ -0,0 +1,426 @@ +#!/usr/bin/env node +import { execFileSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const DEFAULT_BRANCH_PATTERN = '^(agent|feat|fix|chore|docs|ci|test|refactor|perf|release|dependabot)\\/[A-Za-z0-9._\\/-]+$'; +const DEFAULT_DOC_EXTENSIONS = 'md|txt|png|jpg|jpeg|gif|svg|webp|bmp|ico|avif'; +const DEFAULT_DOC_PREFIXES = ['.changelog/']; +const DEFAULT_REQUIRED_HEADINGS = [ + { level: 2, text: 'Summary' }, + { level: 2, text: 'Verification' }, + { level: 3, text: 'Verification Notes' }, + { level: 2, text: 'Docs / Changelog' }, +]; + +const TITLE_RE = /^(feat|fix|refactor|test|docs|style|chore|perf|ci|build|revert)(\([^)]+\))?: .+/; +const ISSUE_RE = /\b(Closes|Fixes|Refs)\s+#\d+\b/i; +const PLACEHOLDER_RE = /\b(TODO|TBD|FIXME|FILL ME|FILL IN|REPLACE THIS|PLACEHOLDER|NOT YET|N\/A|NONE YET)\b|#\s*(?:___|<[^>]+>)|/i; +const CHECKED_RE = /^\s*-\s+\[[xX]\]\s+(.+?)\s*$/; +const UNCHECKED_RE = /^\s*-\s+\[\s\]\s+(.+?)\s*$/; +const GENERIC_VERIFICATION_RE = /\b(automated ci checks? green|ci[- ]?green|ci checks? pass(?:ed|es)?|checks? green|all checks? pass(?:ed|es)?|tests? pass(?:ed|es)?)\b/i; + +/** + * Validate PR metadata against the ArchonVII ready-for-review contract. + * + * @param {object} input + * @param {string} input.title + * @param {string} input.body + * @param {string} input.branch + * @param {string[]} input.files + * @param {object} [options] + * @returns {{ok:boolean, errors:Array<{code:string,message:string,path:string}>, warnings:Array<{code:string,message:string,path:string}>, facts:object}} + */ +export function validatePrContract(input, options = {}) { + const rules = normalizeRules(options); + const data = { + title: input.title || '', + body: input.body || '', + branch: input.branch || '', + files: Array.isArray(input.files) ? input.files : [], + }; + + const errors = []; + const warnings = []; + const docsOnly = isDocsOnly(data.files, rules); + + if (rules.requireTitle && !TITLE_RE.test(data.title)) { + errors.push(error( + 'invalid_title', + 'PR title must use Conventional Commits format, for example `feat(scope): summary`.', + 'title', + )); + } + + if (rules.rejectPlaceholderTitle && hasPlaceholder(data.title)) { + errors.push(error('placeholder_title', 'PR title contains placeholder text.', 'title')); + } + + if (rules.requireBranch && !rules.branchPattern.test(data.branch)) { + errors.push(error( + 'invalid_branch', + `Head branch \`${data.branch || '(empty)'}\` does not match the required branch pattern.`, + 'branch', + )); + } + + if (!docsOnly) { + validateBody(data.body, rules, errors, warnings); + } + + return { + ok: errors.length === 0, + errors, + warnings, + facts: { + docsOnly, + checkedVerificationCount: countCheckedVerificationItems(data.body), + }, + }; +} + +export function formatPrContractResult(result) { + if (result.ok) { + const suffix = result.facts.docsOnly ? ' (docs-only body ceremony skipped)' : ''; + return `PR contract passed${suffix}.`; + } + + const lines = ['PR contract failed.', '', 'Required fixes:']; + for (const item of result.errors) { + lines.push(`- [${item.code}] ${item.message}`); + } + if (result.warnings.length > 0) { + lines.push('', 'Warnings:'); + for (const item of result.warnings) { + lines.push(`- [${item.code}] ${item.message}`); + } + } + return lines.join('\n'); +} + +export function loadPrFromGh({ repo, pr }) { + if (!repo) throw new Error('Missing required --repo owner/name argument.'); + if (!pr) throw new Error('Missing required --pr number argument.'); + + const raw = execFileSync( + 'gh', + ['pr', 'view', String(pr), '--repo', repo, '--json', 'number,title,body,headRefName,isDraft,files,url'], + { encoding: 'utf8' }, + ); + const parsed = JSON.parse(raw); + return { + number: parsed.number, + url: parsed.url, + title: parsed.title || '', + body: parsed.body || '', + branch: parsed.headRefName || '', + isDraft: Boolean(parsed.isDraft), + files: (parsed.files || []).map((file) => file.path || file.filename).filter(Boolean), + }; +} + +function validateBody(body, rules, errors, warnings) { + const headings = parseHeadings(body); + + if (rules.requireIssueLink && !ISSUE_RE.test(body)) { + errors.push(error( + 'missing_issue_link', + 'PR body must link an issue with `Closes #N`, `Fixes #N`, or `Refs #N`.', + 'body', + )); + } + + if (rules.rejectPlaceholders && hasPlaceholder(body)) { + errors.push(error( + 'placeholder_text', + 'PR body contains placeholder text such as TODO, TBD, N/A, or an unset issue marker.', + 'body', + )); + } + + validateHeadingOrder(headings, rules.requiredHeadings, errors); + + const summary = sectionContent(body, headings, 'Summary'); + if (rules.requireSummary && !hasSubstantiveContent(summary)) { + errors.push(error('empty_summary', '`## Summary` must contain substantive content.', 'body')); + } + + const notes = sectionContent(body, headings, 'Verification Notes'); + if (rules.requireSubstantiveVerificationNotes && !hasSubstantiveContent(notes)) { + errors.push(error( + 'empty_verification_notes', + '`### Verification Notes` must contain substantive non-placeholder verification detail.', + 'body', + )); + } + if (rules.rejectGenericVerificationNotes && GENERIC_VERIFICATION_RE.test(notes.trim())) { + errors.push(error( + 'generic_verification_notes', + '`### Verification Notes` must not be a generic CI/tests-passed statement.', + 'body', + )); + } + + const docs = sectionContent(body, headings, 'Docs / Changelog'); + if (rules.requireDocsChangelog && !hasSubstantiveContent(docs)) { + errors.push(error('empty_docs_changelog', '`## Docs / Changelog` must describe docs or changelog handling.', 'body')); + } + + const verification = sectionContent(body, headings, 'Verification', { stopBefore: 'Verification Notes' }); + const checkedItems = verification.split(/\r?\n/).map((line, index) => { + const match = line.match(CHECKED_RE); + return match ? { claim: match[1].trim(), index } : null; + }).filter(Boolean); + const uncheckedItems = verification.split(/\r?\n/).map((line) => { + const match = line.match(UNCHECKED_RE); + return match ? match[1].trim() : null; + }).filter(Boolean); + + if (rules.requireCheckedVerification && checkedItems.length === 0) { + errors.push(error( + 'missing_checked_verification', + '`## Verification` must include at least one checked verification item.', + 'body', + )); + } + + for (const claim of uncheckedItems) { + errors.push(error( + 'unchecked_required_box', + `Unchecked verification item must be completed or removed before ready-for-review: "${claim}".`, + 'body', + )); + } + + for (const item of checkedItems) { + if (GENERIC_VERIFICATION_RE.test(item.claim)) { + errors.push(error( + 'generic_verification', + `Checked verification item is too generic: "${item.claim}". Record the actual command, check, or manual action.`, + 'body', + )); + } + if (rules.requireEvidenceBlocks && !hasEvidenceBlockAfter(verification, item.index)) { + errors.push(error( + 'missing_evidence_block', + `Checked verification item "${item.claim}" must be followed by a fenced \`\`\`evidence block.`, + 'body', + )); + } + } + + if (warnings.length === 0 && headings.length === 0 && body.trim() !== '') { + warnings.push(error('no_headings', 'PR body has no markdown headings.', 'body')); + } +} + +function normalizeRules(options) { + const docPrefixes = Array.isArray(options.docOnlyPathPrefixes) + ? options.docOnlyPathPrefixes + : String(options.docOnlyPathPrefixes || DEFAULT_DOC_PREFIXES.join('\n')) + .split(/\r?\n/) + .map((item) => item.trim()) + .filter(Boolean); + + return { + requireTitle: options.requireTitle !== false, + rejectPlaceholderTitle: options.rejectPlaceholderTitle !== false, + requireBranch: options.requireBranch !== false, + branchPattern: new RegExp(options.branchPattern || DEFAULT_BRANCH_PATTERN), + docExtensions: new RegExp(`\\.(${options.docOnlyExtensions || DEFAULT_DOC_EXTENSIONS})$`, 'i'), + docPrefixes, + requireIssueLink: options.requireIssueLink !== false, + rejectPlaceholders: options.rejectPlaceholders !== false, + requiredHeadings: normalizeHeadings(options.requiredHeadings || DEFAULT_REQUIRED_HEADINGS), + requireSummary: options.requireSummary !== false, + requireDocsChangelog: options.requireDocsChangelog !== false, + requireSubstantiveVerificationNotes: options.requireSubstantiveVerificationNotes !== false, + rejectGenericVerificationNotes: options.rejectGenericVerificationNotes !== false, + requireCheckedVerification: options.requireCheckedVerification !== false, + requireEvidenceBlocks: options.requireEvidenceBlocks !== false, + }; +} + +function normalizeHeadings(headings) { + return headings.map((heading) => { + if (typeof heading === 'string') { + const match = heading.match(/^(#{2,6})\s+(.+)$/); + if (!match) throw new Error(`Invalid required heading: ${heading}`); + return { level: match[1].length, text: cleanHeadingText(match[2]) }; + } + return { level: heading.level, text: cleanHeadingText(heading.text) }; + }); +} + +function parseHeadings(body) { + const headings = []; + const lines = (body || '').split(/\r?\n/); + let offset = 0; + + for (let lineNumber = 0; lineNumber < lines.length; lineNumber++) { + const line = lines[lineNumber]; + const match = line.match(/^(#{2,6})\s+(.+?)\s*#*\s*$/); + if (match) { + headings.push({ + level: match[1].length, + text: cleanHeadingText(match[2]), + lineNumber, + start: offset, + }); + } + offset += line.length + 1; + } + + for (let i = 0; i < headings.length; i++) { + headings[i].end = i + 1 < headings.length ? headings[i + 1].start : body.length; + } + + return headings; +} + +function validateHeadingOrder(headings, required, errors) { + let cursor = -1; + + for (const expected of required) { + const foundIndex = headings.findIndex( + (heading, index) => index > cursor && heading.level === expected.level && sameHeading(heading.text, expected.text), + ); + + if (foundIndex === -1) { + errors.push(error( + 'missing_heading', + `PR body must include \`${'#'.repeat(expected.level)} ${expected.text}\` in the required order.`, + 'body', + )); + return; + } + + cursor = foundIndex; + } +} + +function sectionContent(body, headings, name, options = {}) { + const start = headings.find((heading) => sameHeading(heading.text, name)); + if (!start) return ''; + + let end = start.end; + if (options.stopBefore) { + const stop = headings.find( + (heading) => heading.start > start.start && sameHeading(heading.text, options.stopBefore), + ); + if (stop) end = stop.start; + } + + const raw = body.slice(start.start, end); + return raw.split(/\r?\n/).slice(1).join('\n').trim(); +} + +function hasEvidenceBlockAfter(section, checkedLineIndex) { + const lines = section.split(/\r?\n/); + for (let i = checkedLineIndex + 1; i < lines.length; i++) { + const trimmed = lines[i].trim(); + if (trimmed === '') continue; + return /^```\s*evidence\s*$/i.test(trimmed); + } + return false; +} + +function countCheckedVerificationItems(body) { + const headings = parseHeadings(body || ''); + const verification = sectionContent(body || '', headings, 'Verification', { stopBefore: 'Verification Notes' }); + return verification.split(/\r?\n/).filter((line) => CHECKED_RE.test(line)).length; +} + +function isDocsOnly(files, rules) { + return files.length > 0 && files.every((file) => { + const normalized = file.replace(/\\/g, '/'); + return rules.docExtensions.test(normalized) + || rules.docPrefixes.some((prefix) => normalized.startsWith(prefix)); + }); +} + +function hasSubstantiveContent(text) { + const cleaned = text + .split(/\r?\n/) + .map((line) => line.replace(/^[-*]\s+\[[ xX]\]\s+/, '').trim()) + .filter((line) => line && !line.startsWith('', + '', + '## Summary', + '', + 'TODO: Fill in summary.', + '', + '## Verification', + '', + '- [x] Automated CI checks green on this PR', + '- [ ] TODO: Run required verification and replace this line.', + '', + '### Verification Notes', + '', + '_Auto-injected for bot-authored PR. CI-green is the verification surface._', + '', + '## Docs / Changelog', + '', + 'TODO: Closes #___', + ].join('\n'); + + const result = validatePrContract(input({ body })); + + expect(result.ok).toBe(false); + expect(result.errors.map((error) => error.code)).toEqual( + expect.arrayContaining([ + 'placeholder_text', + 'generic_verification', + 'unchecked_required_box', + 'missing_issue_link', + ]), + ); + }); + + it('allows docs-only PRs to skip body ceremony while keeping title and branch checks', () => { + const result = validatePrContract(input({ + body: 'Small README cleanup.', + files: ['README.md', '.changelog/unreleased/36-pr-contract.md'], + title: 'docs(readme): clarify PR contract', + branch: 'docs/36-pr-contract', + })); + + expect(result.ok).toBe(true); + expect(result.facts.docsOnly).toBe(true); + }); +}); diff --git a/scripts/workflow-structure.test.mjs b/scripts/workflow-structure.test.mjs index d29821e..901ea5a 100644 --- a/scripts/workflow-structure.test.mjs +++ b/scripts/workflow-structure.test.mjs @@ -3,6 +3,16 @@ import { describe, expect, it } from 'vitest'; const readWorkflow = (name) => readFileSync(`.github/workflows/${name}.yml`, 'utf8'); +const workflowJobBlock = (body, job) => { + const marker = ` ${job}:`; + const jobIndex = body.indexOf(marker); + if (jobIndex === -1) return ''; + + const rest = body.slice(jobIndex + marker.length); + const nextJob = rest.search(/\r?\n [a-zA-Z0-9_-]+:\r?\n/); + return body.slice(jobIndex, nextJob === -1 ? body.length : jobIndex + marker.length + nextJob); +}; + describe('node-ci workflow package-manager setup', () => { it('detects the lockfile before setup-node configures dependency caching', () => { const body = readWorkflow('node-ci'); @@ -37,4 +47,72 @@ describe('repo-required-gate workflow node delegation', () => { expect(body).toContain('pnpm-version: ${{ inputs.pnpm-version }}'); expect(body).not.toContain('cache: "npm"'); }); + + it('gates expensive PR jobs on the cheap PR contract', () => { + const body = readWorkflow('repo-required-gate'); + + for (const job of [ + 'workflow-validation', + 'policy-validation', + 'dependency-review', + 'node-ci', + 'python-ci', + 'snapshot-validation', + ]) { + const block = workflowJobBlock(body, job); + + expect(block, `${job} job exists`).not.toBe(''); + expect(block, `${job} waits for pr-contract`).toContain('pr-contract'); + expect(block, `${job} skips when pull-request contract failed`).toContain( + "needs.pr-contract.result == 'success'", + ); + } + }); + + it('lets explicit pr-contract opt-outs continue to downstream jobs', () => { + const body = readWorkflow('repo-required-gate'); + + for (const job of [ + 'workflow-validation', + 'policy-validation', + 'dependency-review', + 'node-ci', + 'python-ci', + 'snapshot-validation', + ]) { + const block = workflowJobBlock(body, job); + + expect(block, `${job} respects run-pr-contract=false`).toContain( + "github.event_name != 'pull_request' || inputs.run-pr-contract == false || needs.pr-contract.result == 'success'", + ); + } + }); + + it('declares the doc-only inputs passed to the shared PR contract validator', () => { + const body = readWorkflow('repo-required-gate'); + + expect(body).toContain('doc-only-extensions:'); + expect(body).toContain('doc-only-path-prefixes:'); + expect(body).toContain('DOC_EXT_LIST: ${{ inputs.doc-only-extensions }}'); + expect(body).toContain('DOC_PREFIXES: ${{ inputs.doc-only-path-prefixes }}'); + }); +}); + +describe('pr-policy workflow contract source', () => { + it('uses the shared PR contract validator instead of inline body regexes', () => { + const body = readWorkflow('pr-policy'); + + expect(body).toContain('__github-workflows__/scripts/pr-contract.mjs'); + expect(body).toContain('validatePrContract'); + }); +}); + +describe('pr-body-autoinject scaffold', () => { + it('does not inject checked verification that can satisfy the strict contract', () => { + const body = readWorkflow('pr-body-autoinject'); + + expect(body).toContain('TODO: Fill in summary.'); + expect(body).toContain('- [ ] TODO: Run required verification and replace this line.'); + expect(body).not.toContain('- [x] Automated CI checks green on this PR'); + }); });