diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 2f0e3eff..ab6c3c3e 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,3 +1,5 @@ + + ## Summary - @@ -14,14 +16,21 @@ - [ ] `npm test -- test/documentation.test.ts` - [ ] `npm run build` -## Docs and Governance Checklist +## Docs Impact + +Pick one: + +- [ ] No docs update needed +- [ ] Docs updated in this PR +- [ ] Follow-up docs work needed + +## Governance Review + +Pick one: -- [ ] README updated (if user-visible behavior changed) -- [ ] `docs/getting-started.md` updated (if onboarding flow changed) -- [ ] `docs/features.md` updated (if capability surface changed) -- [ ] relevant `docs/reference/*` pages updated (if commands/settings/paths changed) -- [ ] `docs/upgrade.md` updated (if migration behavior changed) -- [ ] `SECURITY.md` and `CONTRIBUTING.md` reviewed for alignment +- [ ] No CONTRIBUTING.md/SECURITY.md changes needed +- [ ] CONTRIBUTING.md and/or SECURITY.md reviewed or updated +- [ ] Follow-up CONTRIBUTING.md/SECURITY.md work needed ## Risk and Rollback diff --git a/.github/workflows/anti-slop.yml b/.github/workflows/anti-slop.yml new file mode 100644 index 00000000..e7085272 --- /dev/null +++ b/.github/workflows/anti-slop.yml @@ -0,0 +1,47 @@ +name: Anti Slop + +on: + # `pull_request_target` is intentional so this workflow can label/comment on + # PRs. It must stay metadata-only: never checkout or run untrusted PR code. + pull_request_target: + branches: [main] + types: [opened, synchronize, reopened, ready_for_review, edited] + +concurrency: + group: anti-slop-${{ github.event.pull_request.number }} + cancel-in-progress: true + +permissions: + contents: read + issues: write # required for GitHub's label API, which routes through Issues API even on PRs + pull-requests: write + +jobs: + anti-slop: + name: PR Quality Screening + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - name: Run anti-slop checks + uses: peakoss/anti-slop@85daca1880e9e1af197fc06ea03349daf08f4202 # v0.2.1 + with: + github-token: ${{ github.token }} + max-failures: 4 + require-pr-template: true + strict-pr-template-sections: Validation + optional-pr-template-sections: Additional Notes + # anti-slop strips HTML comments before blocked-term matching, so the + # hidden template comment only trips if copied into visible PR text. + blocked-terms: "WORKTREE_LANTERN_1455" + blocked-paths: "" + exempt-draft-prs: true + failure-add-pr-labels: needs-human-review + failure-pr-message: >- + This PR failed automated PR quality screening and was labelled + needs-human-review for maintainer review. Keep the PR template + intact, include real validation evidence, and remove any copied + hidden template trap text from the visible PR body before requesting + review again. + close-pr: false + lock-pr: false diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 295c5fbe..f16cbbc4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -59,8 +59,10 @@ For user-facing behavior changes, review these files at minimum: - `npm test` - `npm run build` 4. Include command output evidence in the PR description. -5. Document behavior changes and migration notes when needed. -6. Ensure no secrets or local runtime data are committed. +5. Keep the visible `.github/pull_request_template.md` section structure intact, fill every visible section, and do not copy hidden template comments into the PR description. +6. PRs are screened by automated PR quality checks; a `needs-human-review` label means maintainer follow-up is required before merge. +7. Document behavior changes and migration notes when needed. +8. Ensure no secrets or local runtime data are committed. Use `.github/pull_request_template.md` when opening the PR. diff --git a/package-lock.json b/package-lock.json index c71e670d..7d84a956 100644 --- a/package-lock.json +++ b/package-lock.json @@ -35,7 +35,8 @@ "lint-staged": "^16.2.7", "typescript": "^5.9.3", "typescript-language-server": "^5.1.3", - "vitest": "^4.0.18" + "vitest": "^4.0.18", + "yaml": "^2.8.2" }, "engines": { "node": ">=18.0.0" diff --git a/package.json b/package.json index 13083dba..a0a46fda 100644 --- a/package.json +++ b/package.json @@ -112,7 +112,8 @@ "lint-staged": "^16.2.7", "typescript": "^5.9.3", "typescript-language-server": "^5.1.3", - "vitest": "^4.0.18" + "vitest": "^4.0.18", + "yaml": "^2.8.2" }, "dependencies": { "@openauthjs/openauth": "^0.4.3", diff --git a/test/documentation.test.ts b/test/documentation.test.ts index 1c696d36..f73c08af 100644 --- a/test/documentation.test.ts +++ b/test/documentation.test.ts @@ -10,6 +10,7 @@ import { import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { describe, expect, it, vi } from "vitest"; +import { parse } from "yaml"; import { UI_COPY } from "../lib/ui/copy.js"; const projectRoot = resolve(process.cwd()); @@ -60,6 +61,47 @@ function read(filePath: string): string { return readFileSync(join(projectRoot, filePath), "utf-8"); } +interface AntiSlopWorkflowConfig { + on?: { + pull_request_target?: { + branches?: string[]; + types?: string[]; + }; + }; + concurrency?: { + group?: string; + "cancel-in-progress"?: boolean; + }; + permissions?: { + contents?: string; + issues?: string; + "pull-requests"?: string; + }; + jobs?: { + "anti-slop"?: { + "timeout-minutes"?: number; + steps?: Array<{ + name?: string; + uses?: string; + with?: { + "github-token"?: string; + "require-pr-template"?: boolean; + "strict-pr-template-sections"?: string; + "optional-pr-template-sections"?: string; + "max-failures"?: number; + "exempt-draft-prs"?: boolean; + "blocked-terms"?: string; + "blocked-paths"?: string; + "failure-add-pr-labels"?: string; + "failure-pr-message"?: string; + "close-pr"?: boolean; + "lock-pr"?: boolean; + }; + }>; + }; + }; +} + function extractInternalLinks(markdown: string): string[] { return [...markdown.matchAll(/\[[^\]]+\]\(([^)]+)\)/g)] .map((match) => match[1]) @@ -405,12 +447,17 @@ describe("Documentation Integrity", () => { }); it("keeps governance templates and security reporting guidance present", () => { + const antiSlopWorkflow = ".github/workflows/anti-slop.yml"; const prTemplate = ".github/pull_request_template.md"; const issueConfig = ".github/ISSUE_TEMPLATE/config.yml"; const bugTemplate = ".github/ISSUE_TEMPLATE/bug_report.md"; const featureTemplate = ".github/ISSUE_TEMPLATE/feature_request.md"; const codeOfConduct = "CODE_OF_CONDUCT.md"; + expect( + existsSync(join(projectRoot, antiSlopWorkflow)), + `${antiSlopWorkflow} should exist`, + ).toBe(true); expect( existsSync(join(projectRoot, prTemplate)), `${prTemplate} should exist`, @@ -432,12 +479,92 @@ describe("Documentation Integrity", () => { `${codeOfConduct} should exist`, ).toBe(true); + const antiSlop = read(antiSlopWorkflow); + const antiSlopConfig = parse(antiSlop) as AntiSlopWorkflowConfig; + const antiSlopJob = antiSlopConfig.jobs?.["anti-slop"]; + const antiSlopStep = antiSlopJob?.steps?.find( + (step) => step.name === "Run anti-slop checks", + ); + const hiddenDocsImpactHint = + ""; + expect( + antiSlopStep, + 'step "Run anti-slop checks" not found in anti-slop.yml', + ).toBeDefined(); + // pull_request_target runs with base-repo permissions, so the action must + // stay pinned to an immutable commit instead of a mutable tag or branch. + expect(antiSlopStep?.uses).toBe( + "peakoss/anti-slop@85daca1880e9e1af197fc06ea03349daf08f4202", + ); + expect(antiSlopConfig.on?.pull_request_target?.branches).toEqual(["main"]); + expect( + antiSlopConfig.on?.pull_request_target?.types ?? [], + ).toEqual( + [ + "opened", + "synchronize", + "reopened", + "ready_for_review", + "edited", + ], + ); + expect(antiSlopConfig.concurrency).toEqual({ + group: "anti-slop-${{ github.event.pull_request.number }}", + "cancel-in-progress": true, + }); + expect(antiSlopConfig.permissions).toEqual({ + contents: "read", + issues: "write", + "pull-requests": "write", + }); + expect(antiSlopJob?.["timeout-minutes"]).toBe(5); + expect(antiSlopStep?.with?.["github-token"]).toBe("${{ github.token }}"); + expect(antiSlopStep?.with?.["require-pr-template"]).toBe(true); + expect(antiSlopStep?.with?.["strict-pr-template-sections"]).toBe( + "Validation", + ); + expect(antiSlopStep?.with?.["optional-pr-template-sections"]).toBe( + "Additional Notes", + ); + expect(antiSlopStep?.with?.["max-failures"]).toBe(4); + expect(antiSlopStep?.with?.["exempt-draft-prs"]).toBe(true); + expect(antiSlopStep?.with?.["blocked-terms"]).toBe( + "WORKTREE_LANTERN_1455", + ); + expect(antiSlopStep?.with?.["blocked-paths"]).toBe(""); + expect(antiSlopStep?.with?.["failure-add-pr-labels"]).toBe( + "needs-human-review", + ); + expect(antiSlopStep?.with?.["failure-pr-message"]).toContain( + "needs-human-review", + ); + expect(antiSlopStep?.with?.["close-pr"]).toBe(false); + expect(antiSlopStep?.with?.["lock-pr"]).toBe(false); + const prBody = read(prTemplate); + expect(prBody).toContain("WORKTREE_LANTERN_1455"); + const visiblePrBody = prBody.replace(//g, ""); + expect(visiblePrBody).not.toContain("WORKTREE_LANTERN_1455"); + expect(prBody).toContain("## Summary"); + expect(prBody).toContain("## What Changed"); expect(prBody).toContain("npm run lint"); expect(prBody).toContain("npm run typecheck"); expect(prBody).toContain("npm test"); expect(prBody).toContain("npm test -- test/documentation.test.ts"); expect(prBody).toContain("npm run build"); + expect(prBody).toContain("## Docs Impact"); + expect(prBody).toMatch(/## Docs Impact\s*\n+\s*Pick one:/); + // The concrete docs file list lives in a hidden HTML hint so the raw + // template stays specific even though GitHub hides it in rendered PRs. + expect(prBody).toContain(hiddenDocsImpactHint); + expect(prBody).toContain("## Governance Review"); + expect(prBody).toMatch(/## Governance Review\s*\n+\s*Pick one:/); + expect(prBody).toContain( + "Follow-up CONTRIBUTING.md/SECURITY.md work needed", + ); + expect(prBody).toContain("## Risk and Rollback"); + expect(prBody).toContain("## Additional Notes"); + expect(prBody).not.toContain("## Docs and Governance Checklist"); const security = read("SECURITY.md").toLowerCase(); expect(security).toContain("do not open a public issue"); @@ -450,6 +577,8 @@ describe("Documentation Integrity", () => { expect(contributing).toContain("npm run lint"); expect(contributing).toContain("npm test"); expect(contributing).toContain("npm run build"); + expect(contributing).toContain("needs-human-review"); + expect(contributing).toContain("pull_request_template.md"); const conduct = read("CODE_OF_CONDUCT.md").toLowerCase(); expect(conduct).toContain("respectful");