diff --git a/.agents/skills/opening-pr/SKILL.md b/.agents/skills/opening-pr/SKILL.md index 752cd0223..12ef9d1f8 100644 --- a/.agents/skills/opening-pr/SKILL.md +++ b/.agents/skills/opening-pr/SKILL.md @@ -1,12 +1,14 @@ --- name: opening-pr -description: Open a pull request against DouglasNeuroInformatics/OpenDataCapture, or push a follow-up to one — the body a reviewer reads in seconds, and the adversarial-review block without which `.github/workflows/pr-body.yaml` closes the PR. Use whenever a change is ready for a PR; it refuses a branch whose `pnpm check` is not green or whose review has not run on the current head. +description: Open a pull request against DouglasNeuroInformatics/OpenDataCapture, or push a follow-up to one — the body a reviewer reads in seconds, and the adversarial-review block without which `.github/workflows/pr-body.yaml` closes an agent-authored PR. Use whenever a change is ready for a PR; it refuses a branch whose `pnpm check` is not green or whose review has not run on the current head. --- The reviewer reads dozens of these a week. The body says what changed in effect terms and what they might disagree with, and nothing a green CI run already proves. `.github/workflows/pr-body.yaml` reads the body on -every open, edit and push, and closes the PR when the review block is missing, names a commit other than the -head, drops a finding, or when a forbidden section is present. +every open, edit and push of a PR that shows agent authorship (a bot author other than Dependabot, a +`Co-Authored-By` trailer naming Claude, GPT, Codex or Copilot in the author's commits or the body, the +`🤖 Generated with` footer, or any part of the review block), and closes the PR when the review block is +missing, names a commit other than the head, drops a finding, or when a forbidden section is present. ## Preconditions diff --git a/.github/workflows/pr-body.yaml b/.github/workflows/pr-body.yaml index 06553547e..ab6450472 100644 --- a/.github/workflows/pr-body.yaml +++ b/.github/workflows/pr-body.yaml @@ -17,11 +17,19 @@ jobs: uses: actions/setup-node@v7 with: node-version-file: '.nvmrc' + # the event payload carries no commits, and their trailers are one sign of agent authorship; only + # the author's own commits count, so carrying someone else's agent commits does not bind a person + - name: List the Commit Messages + env: + GH_TOKEN: ${{ github.token }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: gh api --paginate "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/commits" --jq '.[] | select(.author.login == env.PR_AUTHOR) | .commit.message' > "$RUNNER_TEMP/commit-messages.txt" - name: Check the Body id: check # the explicit shell adds pipefail, so tee cannot hide the script's exit code shell: bash - run: node scripts/check-pr-body.ts "$GITHUB_EVENT_PATH" | tee "$RUNNER_TEMP/pr-body-report.md" + run: node scripts/check-pr-body.ts "$GITHUB_EVENT_PATH" "$RUNNER_TEMP/commit-messages.txt" | tee "$RUNNER_TEMP/pr-body-report.md" - name: Close the Pull Request if: failure() && steps.check.outcome == 'failure' env: diff --git a/scripts/check-pr-body.ts b/scripts/check-pr-body.ts index 65db01237..35512fad5 100644 --- a/scripts/check-pr-body.ts +++ b/scripts/check-pr-body.ts @@ -4,10 +4,16 @@ type PullRequestEvent = { action: string; pull_request: { body: null | string; - head: { sha: string }; + head: { ref: string; sha: string }; + user: { login: string; type: string }; }; }; +type PullRequest = { + commitMessages: string; + event: PullRequestEvent; +}; + const SKILL_PATH = '.agents/skills/opening-pr/SKILL.md'; const FORBIDDEN_HEADING = /^#{1,6} +(verification|validation|implementation notes)\b/i; @@ -22,6 +28,15 @@ const STAMP = /^(?:Reviewed at commit|Follow-up) ([0-9a-f]{40})\b/gm; const ATTESTATION = /^I, [^,\n]+, (attest that .+|cannot attest: .+)$/m; +const AGENT_TRAILER = /^Co-Authored-By: *(?:Claude|GPT|Codex|Copilot)\b/im; + +const AGENT_FOOTER = /^🤖 Generated with /m; + +const EXEMPT_BOTS = new Set(['dependabot[bot]']); + +// A merge-back carries commits whose own pull requests already passed this check into the release branch. +const EXEMPT_HEAD_REF = /^release\/\d+$/; + function isRecord(value: unknown): value is { [key: string]: unknown } { return typeof value === 'object' && value !== null; } @@ -30,19 +45,52 @@ function parseEvent(contents: unknown): PullRequestEvent { if (!isRecord(contents) || typeof contents.action !== 'string' || !isRecord(contents.pull_request)) { throw new Error('Expected a pull_request event payload with an action and a pull_request'); } - const { body, head } = contents.pull_request; - if ((body !== null && typeof body !== 'string') || !isRecord(head) || typeof head.sha !== 'string') { - throw new Error('Expected pull_request.body to be a string or null and pull_request.head.sha a string'); - } - return { action: contents.action, pull_request: { body, head: { sha: head.sha } } }; + const { body, head, user } = contents.pull_request; + if ( + (body !== null && typeof body !== 'string') || + !isRecord(head) || + typeof head.ref !== 'string' || + typeof head.sha !== 'string' + ) { + throw new Error('Expected pull_request.body to be a string or null and pull_request.head.ref and sha strings'); + } + if (!isRecord(user) || typeof user.login !== 'string' || typeof user.type !== 'string') { + throw new Error('Expected pull_request.user to have a string login and type'); + } + return { + action: contents.action, + pull_request: { body, head: { ref: head.ref, sha: head.sha }, user: { login: user.login, type: user.type } } + }; } -function readEvent(): PullRequestEvent { - const [eventPath] = process.argv.slice(2); - if (!eventPath) { - throw new Error('Usage: node scripts/check-pr-body.ts '); +function readPullRequest(): PullRequest { + const [eventPath, commitMessagesPath] = process.argv.slice(2); + if (!eventPath || !commitMessagesPath) { + throw new Error('Usage: node scripts/check-pr-body.ts '); } - return parseEvent(JSON.parse(fs.readFileSync(eventPath, 'utf8'))); + return { + commitMessages: fs.readFileSync(commitMessagesPath, 'utf8'), + event: parseEvent(JSON.parse(fs.readFileSync(eventPath, 'utf8'))) + }; +} + +// The template binds agents, not people. An agent that leaves no trace cannot be told apart from a +// person, so a review block is a trace too: a body that carries one is checked whatever else it says. +function isAgentAuthored({ commitMessages, event }: PullRequest): boolean { + const { body, head, user } = event.pull_request; + if (EXEMPT_HEAD_REF.test(head.ref)) { + return false; + } + const lines = outsideFences((body ?? '').split(/\r?\n/)); + const text = lines.join('\n'); + return ( + (user.type === 'Bot' && !EXEMPT_BOTS.has(user.login)) || + AGENT_TRAILER.test(commitMessages) || + AGENT_TRAILER.test(text) || + AGENT_FOOTER.test(text) || + ATTESTATION.test(text) || + lines.some((line) => REVIEW_HEADING.test(line)) + ); } function outsideFences(lines: string[]): string[] { @@ -178,7 +226,12 @@ function report(failures: string[]): never { function main(): void { let failures: string[]; try { - failures = checkBody(readEvent()); + const pullRequest = readPullRequest(); + if (!isAgentAuthored(pullRequest)) { + process.stdout.write('The pull request shows no agent authorship, so the template does not apply.\n'); + return; + } + failures = checkBody(pullRequest.event); } catch (error) { report([`The check itself failed: ${error instanceof Error ? error.message : String(error)}`]); }