diff --git a/.github/workflows/pr-title.yml b/.github/workflows/pr-title.yml index d855fb1..24688d7 100644 --- a/.github/workflows/pr-title.yml +++ b/.github/workflows/pr-title.yml @@ -1,7 +1,10 @@ -# Validate every pull request title (and, when available, its commits) against -# the Conventional Commits shape release-please reads. Runs on every pull -# request but gates nothing; the same script is runnable locally, so a -# contributor sees the identical failure before pushing. +# Validate every pull request title (and, when available, its commits) +# against the Conventional Commits shape release-please reads, and check the +# pull request body and every commit message in it for agent attribution: a +# Co-Authored-By trailer naming a coding agent, a "generated with" banner +# naming one, or a link back to an agent session. Runs on every pull request +# but gates nothing; the same script is runnable locally, so a contributor +# sees the identical failure before pushing. 'name': 'PR Title Validation' 'on': @@ -35,14 +38,16 @@ - 'name': 'Install' 'run': 'npm ci' - - 'name': 'Check PR title and commits' + - 'name': 'Check PR title, commits and attribution' 'shell': 'bash' 'env': 'PR_TITLE': '${{ github.event.pull_request.title }}' + 'PR_BODY': '${{ github.event.pull_request.body }}' 'run': | set -euo pipefail upstream=refs/remotes/origin/${GITHUB_BASE_REF:-main} base_sha=$(git merge-base "${upstream}" "${GITHUB_SHA}") node scripts/check-pr-title.ts \ --title "${PR_TITLE}" \ - --range "${base_sha}..${GITHUB_SHA}" || exit 1 \ No newline at end of file + --range "${base_sha}..${GITHUB_SHA}" \ + --body "${PR_BODY}" || exit 1 \ No newline at end of file diff --git a/README.md b/README.md index 55a2c22..8acc7fb 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ npm run verify # lint, format, typecheck, ADR contract, tests + coverage `npm run lint:adrs` alone runs the decision-record contract, and `npm test` runs the suite without enforcing coverage. `npm run test:coverage` (part of `npm run verify`) enforces the ratchet in `vitest.config.ts`: statements -97.71%, branches 89.78%, functions 100%, lines 97.51%. +97.88%, branches 90.27%, functions 100%, lines 97.69%. ## Conventions diff --git a/docs/requirements.md b/docs/requirements.md index 52dfd43..bea2eeb 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -20,7 +20,7 @@ test file and holds at least one test; ids are unique; the count this document states matches the number of rows it holds; and every id cited anywhere in the tracked tree resolves to a row here. -This ledger holds **11** rows. The compiler's behaviours join it as they land. +This ledger holds **12** rows. The compiler's behaviours join it as they land. | id | a contributor or a consumer can rely on | proved by | |---|---|---| @@ -35,3 +35,4 @@ This ledger holds **11** rows. The compiler's behaviours join it as they land. | REQ-009 | The npm package ships nothing outside `docs/adr/` and `spec/`, checked against what npm would really pack rather than the advisory `files` field | [test/package-contents-contract.test.ts](../test/package-contents-contract.test.ts) | | REQ-010 | A gate's npm script and the CI job that runs it land in the same pull request, so neither can drift from the other unnoticed | [test/pipeline-wiring.test.ts](../test/pipeline-wiring.test.ts) | | REQ-011 | Every script, path, coverage number and Node version README.md and CONTRIBUTING.md name matches the repository they describe | [test/docs-contract.test.ts](../test/docs-contract.test.ts) | +| REQ-012 | A pull request's title, body and every commit in it carry no agent attribution: no Co-Authored-By trailer naming a coding agent, no "generated with" banner naming one, no link back to an agent session | [test/pr-title-contract.test.ts](../test/pr-title-contract.test.ts) | diff --git a/scripts/check-pr-title.ts b/scripts/check-pr-title.ts index 5dc2833..cc9196f 100644 --- a/scripts/check-pr-title.ts +++ b/scripts/check-pr-title.ts @@ -1,6 +1,9 @@ // Validate a pull request title, or every commit in a range, against the // Conventional Commits shape release-please reads, and explain the expected -// shape when one does not match. +// shape when one does not match. The same pass also scans the pull request +// body and every commit message in the range for agent attribution: a +// Co-Authored-By trailer naming a coding agent rather than a person, a +// "generated with" banner naming one, or a link back to an agent session. // // The set of accepted types is this script's single source of truth, and a // test keeps it equal to the set the vendored commit-msg hook accepts. The @@ -8,7 +11,8 @@ // command before pushing, so the workflow enforces nothing that cannot be // reproduced on a laptop: // -// node scripts/check-pr-title.ts --title "feat(ci): add a workflow" [--range a..b] +// node scripts/check-pr-title.ts --title "feat(ci): add a workflow" \ +// [--range a..b] [--body "pull request body text"] import { execFileSync } from "node:child_process"; import { isEntrypoint } from "./lib/entrypoint.ts"; import { processOutput, type GateOutput } from "./lib/output.ts"; @@ -51,20 +55,160 @@ export function failureReason(subject: string): string | null { ); } +// Coding-agent identifiers seen in real trailers, banners and links. Brand +// terms only: no bare human first name, because Cody, Devin and Jules are +// all names real people carry, so matching them alone would fail a genuine +// human co-author on a coincidence. Where an agent's own commits identify it +// through a distinctive bot account (`devin-ai-integration`, +// `google-labs-jules[bot]`) that account name is the marker instead of the +// plain name. +const AGENT_MARKERS: readonly string[] = [ + "claude", + "anthropic", + "copilot", + "chatgpt", + "openai", + "codex", + "cursor", + "windsurf", + "aider", + "codeium", + "tabnine", + "devin-ai-integration", + "devin.ai", + "google-labs-jules", + "jules[bot]", + "amazon-q-developer", + "codewhisperer", +]; + +/** The first agent marker `text` contains, case-insensitively, or undefined. */ +function agentMarker(text: string): string | undefined { + const lower = text.toLowerCase(); + return AGENT_MARKERS.find((marker) => lower.includes(marker)); +} + +const CO_AUTHOR_LINE = /^co-authored-by:\s*(.+)$/i; +const BANNER_LINE = /\bgenerated\s+(with|by|using)\b/i; + +/** + * Escape every regex metacharacter in `text` (including backslash itself), + * so it can be dropped into a `RegExp` and only ever match itself. The + * canonical escape-string-regexp shape: escaping only some characters (a + * dot, say) leaves the rest live, which is exactly what let a bare + * `chat.openai.com` also match `chatXopenai.com`. + */ +function escapeRegExp(text: string): string { + return text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +const SESSION_HOSTS: readonly string[] = [ + "claude.ai", + "chatgpt.com", + "chat.openai.com", + "devin.ai", + "cursor.sh", + "windsurf.com", + "windsurf.ai", + "aider.chat", + "jules.google.com", +]; + +// A host matches only at a real host boundary: right after `scheme://`, an +// optional run of `label.` subdomain segments (so `app.devin.ai` still +// matches `devin.ai`), the escaped host itself, and then `/`, `:`, +// whitespace or the end of the string. Unanchored, a bare host substring +// also matches a banned host sitting in another host's path +// (`https://evil.com/claude.ai/x`), which is a spoof in one direction, and +// an unescaped dot lets a host match one character off +// (`chatXopenai.com` for `chat.openai.com`), a spoof in the other. +const SESSION_LINK = new RegExp( + "https?://(?:[a-z0-9-]+\\.)*(?:" + + SESSION_HOSTS.map(escapeRegExp).join("|") + + ")(?=[/:\\s]|$)", + "i", +); + +/** One line of a pull request body or commit message that carries agent attribution. */ +export interface AttributionFinding { + readonly line: string; + readonly reason: string; +} + /** - * Every failure in a pull request title and, when a range is given, in each - * commit subject in that range of the repository at `cwd`. Empty means clean. + * Every line in `text` that carries agent attribution: a Co-Authored-By + * trailer naming a coding agent, a "generated with" banner naming one, or a + * link back to an agent session. A human co-author, an ordinary banner-free + * message, and a link to anything else all produce nothing here. + */ +export function attributionFindings(text: string): AttributionFinding[] { + const findings: AttributionFinding[] = []; + for (const rawLine of text.split("\n")) { + const line = rawLine.trim(); + if (line === "") continue; + + const coAuthor = CO_AUTHOR_LINE.exec(line); + if (coAuthor) { + // The `.+` group cannot be empty when the outer regex has matched. + const marker = agentMarker(coAuthor[1] as string); + if (marker) { + findings.push({ + line, + reason: + `a Co-Authored-By trailer names a coding agent ("${marker}"); ` + + `credit a person instead, or remove the trailer`, + }); + continue; + } + } + + if (BANNER_LINE.test(line)) { + const marker = agentMarker(line); + if (marker) { + findings.push({ + line, + reason: + `a "generated with" banner names a coding agent ("${marker}"); ` + + `remove the banner`, + }); + continue; + } + } + + if (SESSION_LINK.test(line)) + findings.push({ + line, + reason: "a link back to a coding agent session; remove the link", + }); + } + return findings; +} + +/** Every attribution finding in `text`, each named as coming from `context`. */ +function attributionFailures(context: string, text: string): string[] { + return attributionFindings(text).map( + (finding) => `${context}: ${finding.reason} ("${finding.line}")`, + ); +} + +/** + * Every failure in a pull request title and body and, when a range is given, + * in each commit subject and full message in that range of the repository at + * `cwd`. Empty means clean. */ export function check( title: string, range?: string, cwd: string = process.cwd(), + body?: string, ): string[] { const failures: string[] = []; const titleFailure = failureReason(title); if (titleFailure !== null) failures.push(`pull request title: ${titleFailure}`); + if (body) failures.push(...attributionFailures("pull request body", body)); + if (range) { const subjects = execFileSync("git", ["log", "--format=%s", range], { cwd, @@ -76,11 +220,31 @@ export function check( const why = failureReason(subject); if (why !== null) failures.push(`commit "${subject}": ${why}`); } + + // %B carries the full, unwrapped commit message (subject and body), so a + // trailer or banner in the body is only visible here. \x00 separates + // records, since a commit message may itself hold a blank line. + const messages = execFileSync("git", ["log", "--format=%B%x00", range], { + cwd, + encoding: "utf8", + }) + .split("\0") + .map((message) => message.replace(/\n+$/, "")) + .filter((message) => message !== ""); + for (const message of messages) { + // `messages` is filtered to non-empty strings, so splitting one always + // yields at least one element. + const subject = message.split("\n")[0] as string; + failures.push(...attributionFailures(`commit "${subject}"`, message)); + } } return failures; } -/** Check `--title` and an optional `--range`; exit 0 when clean, 1 when not, 2 on bad usage. */ +/** + * Check `--title`, an optional `--range` and an optional `--body`; exit 0 + * when clean, 1 when not, 2 on bad usage. + */ export function main( argv: readonly string[], output: GateOutput = processOutput, @@ -92,11 +256,12 @@ export function main( const title = flag("--title"); if (!title) { output.err( - 'usage: node scripts/check-pr-title.ts --title "..." [--range a..b]\n', + 'usage: node scripts/check-pr-title.ts --title "..." ' + + '[--range a..b] [--body "..."]\n', ); return 2; } - const failures = check(title, flag("--range")); + const failures = check(title, flag("--range"), process.cwd(), flag("--body")); if (failures.length > 0) { output.err(failures.map((failure) => `${failure}\n`).join("")); return 1; diff --git a/test/pr-title-contract.test.ts b/test/pr-title-contract.test.ts index 77a887b..57fb194 100644 --- a/test/pr-title-contract.test.ts +++ b/test/pr-title-contract.test.ts @@ -6,12 +6,16 @@ // // REQ-005 (docs/requirements.md): a pull request title and its commits use a // conventional-commit type release-please reads. +// +// REQ-012 (docs/requirements.md): a pull request's title, body and commits +// carry no agent attribution. import { execFileSync, spawnSync } from "node:child_process"; import { mkdtempSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { COMMIT_TYPES, + attributionFindings, check, failureReason, isConventional, @@ -22,8 +26,12 @@ import { temporary } from "./setup.ts"; const REPOSITORY = join(import.meta.dirname, ".."); -/** A repository whose commits carry `subjects`, and the range that spans them. */ -function history(subjects: readonly string[]): { root: string; range: string } { +/** + * A repository whose commits carry `messages` (each a full commit message, + * subject only or subject plus a trailer-bearing body), and the range that + * spans them. + */ +function history(messages: readonly string[]): { root: string; range: string } { const root = mkdtempSync(join(temporary(), "history-")); const git = (...args: string[]): string => execFileSync( @@ -44,8 +52,8 @@ function history(subjects: readonly string[]): { root: string; range: string } { // this fixture exists to hold. git("commit", "-q", "--allow-empty", "--no-verify", "-m", "chore: the base"); const base = git("rev-parse", "HEAD"); - for (const subject of subjects) - git("commit", "-q", "--allow-empty", "--no-verify", "-m", subject); + for (const message of messages) + git("commit", "-q", "--allow-empty", "--no-verify", "-m", message); return { root, range: `${base}..HEAD` }; } @@ -121,6 +129,170 @@ describe("check", () => { expect(failures).toHaveLength(1); expect(failures[0]).toMatch(/^commit "plain words": /); }); + + it("reports agent attribution in the pull request body, naming the line", () => { + const failures = check( + "feat: add a workflow", + undefined, + undefined, + "Thanks!\n\nCo-Authored-By: Claude ", + ); + expect(failures).toHaveLength(1); + expect(failures[0]).toBe( + "pull request body: a Co-Authored-By trailer names a coding agent " + + '("claude"); credit a person instead, or remove the trailer ' + + '("Co-Authored-By: Claude ")', + ); + }); + + it("passes a pull request body with no attribution", () => { + expect( + check( + "feat: add a workflow", + undefined, + undefined, + "Nothing to see here.", + ), + ).toStrictEqual([]); + }); + + it("reports agent attribution in a commit message, naming the commit", () => { + const { root, range } = history([ + "feat: a fine one", + "fix: patch\n\nCo-Authored-By: Claude ", + ]); + const failures = check("feat: a fine title", range, root); + expect(failures).toHaveLength(1); + expect(failures[0]).toMatch( + /^commit "fix: patch": a Co-Authored-By trailer names a coding agent \("claude"\)/, + ); + }); + + it("passes a commit with a human co-author", () => { + const { root, range } = history([ + "fix: patch\n\nCo-Authored-By: Jane Doe ", + ]); + expect(check("feat: a fine title", range, root)).toStrictEqual([]); + }); + + it("passes a clean pull request end to end", () => { + const { root, range } = history(["fix: patch", "docs: update"]); + expect( + check("feat: add a workflow", range, root, "Nothing to see here."), + ).toStrictEqual([]); + }); +}); + +describe("attributionFindings", () => { + it("passes a human co-author", () => { + expect( + attributionFindings("Co-Authored-By: Jane Doe "), + ).toStrictEqual([]); + }); + + it("finds a Co-Authored-By trailer naming a coding agent", () => { + const findings = attributionFindings( + "fix: patch\n\nCo-Authored-By: Claude ", + ); + expect(findings).toHaveLength(1); + expect(findings[0]?.line).toBe( + "Co-Authored-By: Claude ", + ); + expect(findings[0]?.reason).toMatch( + /a Co-Authored-By trailer names a coding agent \("claude"\)/, + ); + }); + + it("finds a Co-Authored-By trailer naming a known agent bot account", () => { + const findings = attributionFindings( + "Co-authored-by: google-labs-jules[bot] " + + "<161369871+google-labs-jules[bot]@users.noreply.github.com>", + ); + expect(findings).toHaveLength(1); + expect(findings[0]?.reason).toMatch(/"google-labs-jules"/); + }); + + it("finds a generated-with banner naming a coding agent", () => { + const findings = attributionFindings( + "fix: patch\n\n🤖 Generated with [Claude Code](https://claude.ai/code)", + ); + expect(findings).toHaveLength(1); + expect(findings[0]?.reason).toMatch( + /a "generated with" banner names a coding agent \("claude"\)/, + ); + }); + + it("passes a banner that names nothing agent-shaped", () => { + expect( + attributionFindings("Generated with love, by the whole team"), + ).toStrictEqual([]); + }); + + it("finds a link back to a coding agent session", () => { + const findings = attributionFindings( + "See https://claude.ai/chat/abc123 for the transcript", + ); + expect(findings).toHaveLength(1); + expect(findings[0]?.reason).toBe( + "a link back to a coding agent session; remove the link", + ); + }); + + it("finds a session link through an arbitrary subdomain", () => { + const findings = attributionFindings( + "See https://app.devin.ai/session/9 for the transcript", + ); + expect(findings).toHaveLength(1); + }); + + it("finds every genuine multi-label host, dot and all", () => { + for (const url of [ + "https://chat.openai.com/c/abc", + "https://jules.google.com/task/1", + "https://windsurf.com/session/1", + ]) + expect( + attributionFindings(`See ${url} for the transcript`), + url, + ).toHaveLength(1); + }); + + it("passes a link to anything that is not an agent session", () => { + expect( + attributionFindings("See https://github.com/org/repo/pull/1"), + ).toStrictEqual([]); + }); + + it("does not match a banned host sitting in another host's path", () => { + // Unanchored, "claude.ai" as a bare substring would also match here; + // the real host is evil.com, and claude.ai is only a path segment. + expect( + attributionFindings("See https://evil.com/claude.ai/x for details"), + ).toStrictEqual([]); + }); + + it("does not match a host name one character off from a real host", () => { + // An unescaped "." in "chat.openai.com" is a wildcard, so it would also + // match "chatXopenai.com". It must not. + expect( + attributionFindings("See https://chatXopenai.com/session for details"), + ).toStrictEqual([]); + expect( + attributionFindings("See https://julesXgoogle.com/session for details"), + ).toStrictEqual([]); + }); + + it("does not match a real host used as a prefix of someone else's domain", () => { + // claude.ai followed by another label, rather than a real host + // boundary, is claude.ai.evil.com, not a link to claude.ai. + expect( + attributionFindings("See https://claude.ai.evil.com/x for details"), + ).toStrictEqual([]); + }); + + it("ignores blank lines", () => { + expect(attributionFindings("\n\n")).toStrictEqual([]); + }); }); describe("the command", () => { @@ -142,6 +314,37 @@ describe("the command", () => { expect(output.text()).toMatch(/^pull request title: "Fix bug" is not/); }); + it("fails, naming the line, when the body carries agent attribution", () => { + const output = collect(); + const code = main( + [ + "--title", + "feat: add a workflow", + "--body", + "Co-Authored-By: Claude ", + ], + output, + ); + expect(code).toBe(1); + expect(output.text()).toMatch( + /^pull request body: a Co-Authored-By trailer names a coding agent \("claude"\)/, + ); + }); + + it("passes when the body's Co-Authored-By names a person", () => { + const output = collect(); + const code = main( + [ + "--title", + "feat: add a workflow", + "--body", + "Co-Authored-By: Jane Doe ", + ], + output, + ); + expect(code).toBe(0); + }); + it("runs when Node starts the script, which is how CI runs it", () => { const run = spawnSync( process.execPath, diff --git a/vitest.config.ts b/vitest.config.ts index 2b4df6c..e86d3c9 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -22,21 +22,23 @@ export default defineConfig({ // A ratchet, per docs/adr/architecture/0101-coverage-is-a-ratchet.md: // set from what the suite reaches, and only ever raised. // - // Measured 2026-09-14, after the docs contract gate's own tests - // covered it (its `?? ""` capture-group fallbacks were replaced by one - // cast, since a `+`-quantified group cannot be absent when `matchAll` - // yields a match for it; the missing-scripts-field branch is real and - // is tested) and, like the other gates, left only its bottom-of-file - // entrypoint guard uncovered, two runs of one tree, identical both - // times: statements 470/481, branches 246/274, functions 74/74, lines - // 432/443. What is left uncovered elsewhere is mostly the one-line - // command guard at the bottom of each other gate and the branches for - // a tool that cannot be started at all. + // Measured 2026-09-14, after the pull request attribution check + // (scripts/check-pr-title.ts) landed. Its two `?? ""` capture-group + // fallbacks were replaced by a cast each, the same way the docs + // contract gate's were: a `.+` group cannot be absent once the outer + // regex has matched, and once `messages` is filtered to non-empty + // strings, splitting one always yields a first element. Like the other + // gates, it left only its bottom-of-file entrypoint guard uncovered. + // Two runs of one tree, identical both times: statements 508/519, + // branches 260/288, functions 82/82, lines 467/478. What is left + // uncovered elsewhere is mostly the one-line command guard at the + // bottom of each other gate and the branches for a tool that cannot be + // started at all. thresholds: { - statements: 97.71, - branches: 89.78, + statements: 97.88, + branches: 90.27, functions: 100, - lines: 97.51, + lines: 97.69, }, }, },