From 790f348912be24d5c6d569e33f9bc3c11e7a3d40 Mon Sep 17 00:00:00 2001 From: Akanksha Trehun Date: Fri, 23 Oct 2026 23:38:34 +0530 Subject: [PATCH 1/2] ci: Claude PR automation never merges; skip drafts; enforce statically - claude-manager.yml (claude-pr-review): skip draft PRs (`github.event.pull_request.draft == false`), keep the event and author restrictions, rename the step, and replace "squash-merge the PR" with a comment that the PR is ready for a maintainer. The prompt now forbids merging, approving, enabling auto-merge, pushing and marking drafts ready. Permissions unchanged (contents: read). - CLAUDE.md: PR Automation no longer merges; automatic conflict resolution (git merge + push to the PR branch) becomes guidance in a comment; General Rules no longer list "merge PRs". - scripts/ci/check-pr-automation.mjs: fails on merge commands/APIs in any workflow, contents: write / write-all in a Claude job, a PR-writing Claude job without a draft guard, and affirmative merge/push instructions in prompts or CLAUDE.md. Run in lint.yml; unit-tested with passing and failing fixtures (it flags all six items on main today). Co-Authored-By: Claude Opus 5.5 --- .github/workflows/claude-manager.yml | 13 ++- .github/workflows/lint.yml | 26 +++--- CLAUDE.md | 53 +++++------ scripts/ci/check-pr-automation.mjs | 113 +++++++++++++++++++++++ src/lib/tests/prAutomationSafety.test.ts | 63 +++++++++++++ 5 files changed, 222 insertions(+), 46 deletions(-) create mode 100644 scripts/ci/check-pr-automation.mjs create mode 100644 src/lib/tests/prAutomationSafety.test.ts diff --git a/.github/workflows/claude-manager.yml b/.github/workflows/claude-manager.yml index e157b470d..5bad5c459 100644 --- a/.github/workflows/claude-manager.yml +++ b/.github/workflows/claude-manager.yml @@ -9,9 +9,14 @@ on: types: [created] jobs: + # Reviews and comments only. It never merges: a human maintainer performs + # every merge (CLAUDE.md, "PR Automation"). The token can't merge anyway + # (contents: read), and drafts are skipped so work in progress isn't + # reviewed. scripts/ci/check-pr-automation.mjs enforces all of this. claude-pr-review: if: | github.event_name == 'pull_request' && + github.event.pull_request.draft == false && (github.event.pull_request.author_association == 'OWNER' || github.event.pull_request.author_association == 'MEMBER' || github.event.pull_request.author_association == 'COLLABORATOR') @@ -30,7 +35,7 @@ jobs: - name: Wait for other checks to settle (30s) run: sleep 30 - - name: Claude PR Review & Auto-merge + - name: Claude PR Review (comments only, never merges) uses: anthropics/claude-code-action@beta with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} @@ -46,6 +51,10 @@ jobs: Title: ${{ github.event.pull_request.title }} Base branch: ${{ github.event.pull_request.base.ref }} + You review and comment only. Never merge, squash-merge, rebase-merge, approve or enable + auto-merge for this pull request, never push commits to it, and never mark it ready for + review. A human maintainer performs all merges. + Steps: 1. Fetch all CI check statuses for this PR using the GitHub API. 2. Check the Vercel deployment check status specifically. @@ -57,7 +66,7 @@ jobs: - cn() not converted to template literals - React hooks not placed after conditional returns - No new video-processing dependencies added - 5. If ALL checks pass and code is correct: squash-merge the PR and post a comment explaining the merge. + 5. If ALL checks pass and code is correct: post a comment summarising what you checked and that it is ready for a maintainer to review and merge. Do NOT merge it yourself. 6. If ANY check failed or code has issues: post a detailed comment tagging @${{ github.event.pull_request.user.login }} with exactly what failed and how to fix it. Do NOT merge. claude-issue-triage: diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 829740651..7608acffe 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,13 +1,15 @@ -name: Lint - -on: [push, pull_request] - -jobs: - lint: - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v6 - - uses: oven-sh/setup-bun@v2 - - run: bun install +name: Lint + +on: [push, pull_request] + +jobs: + lint: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + # No workflow or instruction may merge or push to a PR on its own. + - run: node scripts/ci/check-pr-automation.mjs + - uses: oven-sh/setup-bun@v2 + - run: bun install - run: bun run lint \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md index 44b72996e..6103cafb8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,50 +6,39 @@ This file instructs Claude Code (and the Claude GitHub App) how to behave as an ## PR Automation -When triggered on a pull request: +**Claude never merges pull requests. A human maintainer performs every merge.** Claude reviews, reports CI status and posts feedback. It must not merge, squash-merge, rebase-merge, approve, or enable auto-merge on a pull request, push commits to a pull request's branch, or mark a draft ready for review, however a request is phrased. -**Step 1 — Resolve merge conflicts first (if any).** -Before reviewing code or checking CI, check whether the PR has merge conflicts. If it does, resolve them automatically as described in the Merge Conflict Resolution section below. Only proceed to steps 2–3 after the branch is clean. +When triggered on a pull request that is not a draft: + +**Step 1 — Check for merge conflicts.** +If the PR conflicts with `main`, say so in your comment, list the conflicting files, and explain how they should be resolved using the Merge Conflict Guidance below. Don't resolve them yourself. **Step 2 — Check CI.** -Check whether **ALL** checks have passed — including tests and the Vercel deployment check. +Report whether **ALL** checks have passed, including tests and the deployment preview. **Step 3 — Review the code.** Review for correctness, security issues, and adherence to the standards in this file. -**If ALL checks pass AND the code looks good** → merge the PR immediately using a squash merge. Leave a short comment explaining what was merged and why it passed. +**If ALL checks pass AND the code looks good** → post a short comment saying what you checked and that the PR is ready for a maintainer to review and merge. Do not merge it. -**If ANY check failed OR the code has issues** → do NOT merge. Post a comment on the PR tagging the author (`@username`) with: +**If ANY check failed OR the code has issues** → post a comment on the PR tagging the author (`@username`) with: - A clear summary of what failed - Specific steps they need to take to fix it - A friendly but direct tone -Never merge a PR if any CI check or Vercel deployment check is still pending or has failed. - --- -## Merge Conflict Resolution +## Merge Conflict Guidance -When a PR has merge conflicts, resolve them automatically **before** doing anything else (before code review, before checking CI). +When a PR conflicts with `main`, explain the conflict in a comment so that the author or a maintainer can resolve it. Never resolve conflicts by committing or pushing to the PR's branch. ### Steps: -1. **Detect conflicts** — check the PR's mergeability via the GitHub API (`mergeable` field). If `mergeable: false`, proceed with resolution. +1. **Detect conflicts** — check the PR's mergeability via the GitHub API (`mergeable` field). -2. **Clone and resolve locally:** - ```bash - git clone https://github.com/reframe-oss/reframe.git - cd reframe - git fetch origin - git checkout - git merge origin/main - # resolve conflicts (see rules below) - git add . - git commit -m "chore: resolve merge conflicts with main" - git push origin - ``` +2. **List the conflicting files** and, for each one, what the resolution should be. -3. **Conflict resolution rules — apply these in order:** +3. **Conflict resolution rules — recommend these, in order:** - **`src/app/page.tsx`** — Always keep the `main` version exactly. This file has a known stable structure; contributor changes that add a `
` wrapper or duplicate JSX must be discarded in favour of main. - **`bun.lock` / `package-lock.json`** — Keep `bun.lock` from `main`. Delete any `package-lock.json` the PR introduced. @@ -58,17 +47,17 @@ When a PR has merge conflicts, resolve them automatically **before** doing anyth - **`.github/workflows/`** — Keep `main` versions of all workflow files. Only accept new workflow files from the PR if they don't conflict. - **All other files** — Use standard semantic merge: keep both sets of changes if they don't logically conflict. If two edits touch the same line and both are meaningful, apply the PR's change on top of `main`'s version (i.e., prefer preserving contributor intent where safe). -4. **Post a comment** on the PR after pushing the resolved branch, tagging the author: +4. **Post a comment** tagging the author, for example: - > Hey @username! This PR had merge conflicts with `main` — I've resolved them automatically and pushed the result. Here's what was changed: + > Hey @username! This PR has merge conflicts with `main`. Here's how to resolve them: > - > - **`src/app/page.tsx`**: kept the `main` version (your `
` wrapper was removed — this file has a fixed structure) - > - **`bun.lock`**: regenerated from `main` (removed `package-lock.json`) - > - _(list each file and what decision was made)_ + > - **`src/app/page.tsx`**: keep the `main` version (this file has a fixed structure) + > - **`bun.lock`**: take `main`'s version and delete `package-lock.json` + > - _(list each file and the recommended decision)_ > - > CI has been re-triggered. If anything looks wrong, feel free to push additional changes. + > Run `git merge origin/main` on your branch, resolve these, and push. A maintainer will review it afterwards. -5. **Do not resolve conflicts that require domain judgement** — if two branches both meaningfully modify the same function body in `src/lib/ffmpeg.ts` or a complex component, do not guess. Instead post a comment asking the author to rebase manually and explain exactly which lines conflict. +5. **Flag conflicts that need domain judgement** — if two branches both meaningfully modify the same function body in `src/lib/ffmpeg.ts` or a complex component, don't guess a resolution. Explain exactly which lines conflict and ask the author to resolve them. --- @@ -198,7 +187,7 @@ Every PR must have: ## General Rules -- Always take real actions via the GitHub API — post comments, merge PRs, assign issues, apply labels. +- Always take real actions via the GitHub API — post comments, assign issues, apply labels. Never merge PRs, push to PR branches, or enable auto-merge; a human maintainer merges. - Follow the code style and architecture patterns visible in the existing codebase. - Tag contributors by their GitHub username when addressing them in comments. - If uncertain about something, post a comment asking for clarification rather than guessing. diff --git a/scripts/ci/check-pr-automation.mjs b/scripts/ci/check-pr-automation.mjs new file mode 100644 index 000000000..a304bd90e --- /dev/null +++ b/scripts/ci/check-pr-automation.mjs @@ -0,0 +1,113 @@ +// Guards against PR automation that merges or pushes on its own. A human +// maintainer performs every merge (CLAUDE.md, "PR Automation"). +// +// Fails when: +// 1. any workflow runs a merge: `gh pr merge`, the REST merge endpoint, +// GraphQL mergePullRequest / enablePullRequestAutoMerge, merge_method; +// 2. a job using anthropics/claude-code-action can write repository +// contents (`contents: write`, `write-all`), which merging and pushing need; +// 3. such a job can write to pull requests but doesn't skip drafts +// (`github.event.pull_request.draft == false` in its `if`); +// 4. a Claude prompt or CLAUDE.md tells the agent to merge, squash-merge, +// enable auto-merge, or push to a PR branch, without a negation +// ("never", "do not", …) on the same line. +// +// Usage: node scripts/ci/check-pr-automation.mjs (exit 0 ok, 1 problems) +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const MERGE_COMMANDS = [ + /\bgh\s+pr\s+merge\b/i, + /\/pulls\/[^\s/]+\/merge\b/i, + /\bmergePullRequest\b/, + /\benablePullRequestAutoMerge\b/, + /\bmerge_method\b/, + /\bgh\s+pr\s+ready\b/i, +]; + +// Affirmative instructions that would make an agent merge or push. +const MERGE_INSTRUCTION = + /\b(squash[- ]?merge|rebase[- ]?merge|auto[- ]?merge|merge (it|the pr|the pull request|this pr|this pull request|prs|pull requests)|git push)\b/i; +const NEGATION = /\b(never|not|don't|do not|no|must not|cannot|can't)\b/i; + +/** Splits a workflow's text into { name, text } blocks per job (2-space keys under `jobs:`). */ +export function jobsOf(workflowText) { + const lines = workflowText.split("\n"); + const start = lines.findIndex((l) => /^jobs:\s*$/.test(l)); + if (start < 0) return []; + const jobs = []; + let current = null; + for (const line of lines.slice(start + 1)) { + if (/^\S/.test(line) && line.trim()) break; + const m = line.match(/^ {2}([A-Za-z0-9_-]+):\s*$/); + if (m) { + current = { name: m[1], lines: [] }; + jobs.push(current); + } else if (current) { + current.lines.push(line); + } + } + return jobs.map((j) => ({ name: j.name, text: j.lines.join("\n") })); +} + +/** Lines that tell an agent to merge or push, ignoring negated ones. */ +export function affirmativeMergeLines(text) { + return text + .split("\n") + .map((line, i) => ({ line: line.trim(), n: i + 1 })) + .filter(({ line }) => MERGE_INSTRUCTION.test(line) && !NEGATION.test(line)); +} + +export function checkWorkflow(file, text) { + const problems = []; + text.split("\n").forEach((line, i) => { + if (line.trim().startsWith("#")) return; + for (const re of MERGE_COMMANDS) if (re.test(line)) problems.push(`${file}:${i + 1}: runs a merge operation (${re})`); + }); + if (/^permissions:\s*write-all/m.test(text)) problems.push(`${file}: workflow-level permissions: write-all`); + + for (const job of jobsOf(text)) { + if (!/anthropics\/claude-code-action/.test(job.text)) continue; + const where = `${file} › ${job.name}`; + if (/^\s+contents:\s*write\b/m.test(job.text) || /permissions:\s*write-all/.test(job.text)) { + problems.push(`${where}: Claude job can write repository contents`); + } + const prWrite = /^\s+pull-requests:\s*write\b/m.test(job.text); + const prTriggered = /pull_request/.test(job.text) || /^\s+if:[\s\S]*?pull_request/m.test(job.text); + if (prWrite && prTriggered && !/github\.event\.pull_request\.draft\s*==\s*false/.test(job.text)) { + problems.push(`${where}: can write to pull requests but doesn't skip drafts`); + } + for (const { line, n } of affirmativeMergeLines(job.text)) { + problems.push(`${where} (job line ${n}): instruction to merge or push: "${line.slice(0, 100)}"`); + } + } + return problems; +} + +export function checkInstructions(file, text) { + return affirmativeMergeLines(text).map(({ line, n }) => `${file}:${n}: instruction to merge or push: "${line.slice(0, 100)}"`); +} + +export function checkRepository(root) { + const problems = []; + const dir = path.join(root, ".github/workflows"); + for (const name of fs.readdirSync(dir).filter((f) => /\.ya?ml$/.test(f) && !/ \d+\.ya?ml$/.test(f))) { + problems.push(...checkWorkflow(`.github/workflows/${name}`, fs.readFileSync(path.join(dir, name), "utf8"))); + } + for (const name of ["CLAUDE.md", ".github/CLAUDE.md"]) { + const p = path.join(root, name); + if (fs.existsSync(p)) problems.push(...checkInstructions(name, fs.readFileSync(p, "utf8"))); + } + return problems; +} + +if (import.meta.url === `file://${process.argv[1]}`) { + const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); + const problems = checkRepository(root); + if (problems.length) { + console.error("PR automation safety check failed:\n" + problems.map((p) => ` - ${p}`).join("\n")); + process.exit(1); + } + console.log("PR automation safety check passed: no workflow or instruction merges or pushes on its own."); +} diff --git a/src/lib/tests/prAutomationSafety.test.ts b/src/lib/tests/prAutomationSafety.test.ts new file mode 100644 index 000000000..3acfa7cde --- /dev/null +++ b/src/lib/tests/prAutomationSafety.test.ts @@ -0,0 +1,63 @@ +import { describe, it, expect } from "vitest"; +import path from "node:path"; +import { checkInstructions, checkRepository, checkWorkflow } from "../../../scripts/ci/check-pr-automation.mjs"; + +const root = path.resolve(__dirname, "../../.."); + +const reviewJob = (body: { if?: string; perms?: string; prompt?: string; name?: string } = {}) => `name: Test +on: + pull_request: + types: [opened, synchronize] +jobs: + review: + if: | +${body.if ?? " github.event_name == 'pull_request' &&\n github.event.pull_request.draft == false"} + runs-on: ubuntu-latest + permissions: +${body.perms ?? " contents: read\n pull-requests: write"} + steps: + - name: ${body.name ?? "Review"} + uses: anthropics/claude-code-action@beta + with: + direct_prompt: | +${body.prompt ?? " Review the PR and post a comment. Never merge it."} +`; + +describe("PR automation safety check", () => { + it("passes this repository's workflows and CLAUDE.md", () => { + expect(checkRepository(root)).toEqual([]); + }); + + it("accepts a comment-only, draft-skipping review job", () => { + expect(checkWorkflow("w.yml", reviewJob())).toEqual([]); + }); + + it.each([ + ["an instruction to squash-merge", { prompt: " 5. If all checks pass: squash-merge the PR." }, /instruction to merge/], + ["an instruction to merge the PR", { prompt: " Then merge the PR and comment." }, /instruction to merge/], + ["enabling auto-merge", { prompt: " Enable auto-merge when green." }, /instruction to merge/], + ["pushing to the PR branch", { prompt: " git push origin the-branch" }, /instruction to merge or push/], + ["an auto-merge step name", { name: "Review & Auto-merge" }, /instruction to merge/], + ["contents: write", { perms: " contents: write\n pull-requests: write" }, /write repository contents/], + ["write-all", { perms: " write-all" }, /write repository contents/], + ["no draft guard", { if: " github.event_name == 'pull_request'" }, /doesn't skip drafts/], + ])("rejects %s", (_label, body, message) => { + expect(checkWorkflow("w.yml", reviewJob(body)).join("\n")).toMatch(message); + }); + + it.each([ + "run: gh pr merge --squash ${{ github.event.pull_request.number }}", + "run: gh api -X PUT repos/o/r/pulls/1/merge", + "run: gh pr ready 12", + "query: mutation { enablePullRequestAutoMerge(input: {}) { clientMutationId } }", + ])("rejects a merge operation in any workflow: %s", (line) => { + const wf = `name: X\non: push\njobs:\n a:\n runs-on: ubuntu-latest\n steps:\n - ${line}\n`; + expect(checkWorkflow("x.yml", wf).join("\n")).toMatch(/merge operation/); + }); + + it("flags affirmative merge instructions in CLAUDE.md but allows prohibitions", () => { + expect(checkInstructions("CLAUDE.md", "If checks pass, merge the PR immediately using a squash merge.")).toHaveLength(1); + expect(checkInstructions("CLAUDE.md", "Never merge PRs or enable auto-merge; a maintainer merges.")).toEqual([]); + expect(checkInstructions("CLAUDE.md", "Uses `clsx` + `tailwind-merge`.")).toEqual([]); + }); +}); From bdff6535d86ad00b0cbd6bc20818605b7db1e713 Mon Sep 17 00:00:00 2001 From: Akanksha Trehun Date: Fri, 23 Oct 2026 23:38:57 +0530 Subject: [PATCH 2/2] ci: keep lint.yml's original CRLF line endings The previous commit rewrote the file with LF; the net change against main is now only the two added lines. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/lint.yml | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 7608acffe..da6254b7f 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,15 +1,15 @@ -name: Lint - -on: [push, pull_request] - -jobs: - lint: - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v6 - # No workflow or instruction may merge or push to a PR on its own. - - run: node scripts/ci/check-pr-automation.mjs - - uses: oven-sh/setup-bun@v2 - - run: bun install +name: Lint + +on: [push, pull_request] + +jobs: + lint: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + # No workflow or instruction may merge or push to a PR on its own. + - run: node scripts/ci/check-pr-automation.mjs + - uses: oven-sh/setup-bun@v2 + - run: bun install - run: bun run lint \ No newline at end of file