From 0515ffdbe5a2e294c5cd17cc84cd5ad383984d5b Mon Sep 17 00:00:00 2001 From: gavinbee <29419542+gavinbee@users.noreply.github.com> Date: Fri, 7 Aug 2026 03:20:18 -0400 Subject: [PATCH] Agents hand a change back for local review instead of opening the PR Both files had the agent open the PR and then hand off, which allows a pull request to be opened under the author's name carrying work the author has not read a line of. A PR is a claim that its author stands behind the change, and that claim is false if they have not seen it. Stated as an invariant rather than a procedure, so one rule covers humans and agents: nobody opens a pull request carrying work its author hasn't read. For a human it is nearly free but not empty -- reading your own diff is where debug output, stray files, and data or credentials that were never meant to be committed get caught. For an agent the same rule cannot be satisfied without stopping, because the author is somebody else. Also states that an agent raises open questions with the author rather than in the PR description. A question written into a PR reads as though the author is asking their reviewers: it sponsors a question they never asked and hands the agent's decision to their collaborators. This is the failure that prompted the rule. The pre-merge handoff stays as its own step, since "do not self-merge" is a different handoff from "do not open the PR". The bulk-mechanical-change exception is extended to cover opening as well as merging, on the grounds that twenty PRs needing individual local review is the same friction the exception already exists for. Closes #39 Co-Authored-By: Claude Fable 5 --- AGENTS.md | 15 ++++++++++----- CONTRIBUTING.md | 26 ++++++++++++++++++-------- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index adf03a5..b8a793e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,11 +50,16 @@ For **every** change: 2. **Branch** off `main`, named `-`. 3. **Make focused commits.** Coherent, single-purpose. Run `ruff check .` and `pytest -q` locally before pushing. -4. **Open a PR** whose description includes `Closes #` and explains the *why*. -5. **Wait for CI green** (lint + tests). -6. **Hand off.** Post a short summary to the human author — what changed, why, lint/test - results, PR link — then stop. **Agents do not self-merge.** *Exception:* a large mechanical - change spanning many repos may be agent-merged by prior agreement with the author. +4. **Hand off for local review — do not open the PR.** Nobody opens a pull request carrying + work its author hasn't read, and the author hasn't seen yours. Push the branch, post a + short summary (what changed, why, lint/test results), and stop. The author reviews + locally, then opens the PR or asks you to. + - **Raise open questions here, not in the PR description.** A question written into a PR + reads as though the author is asking their reviewers — it sponsors a question they never + asked and hands your decision to their collaborators. +5. **Wait for CI green** (lint + tests) once the PR exists. +6. **Do not self-merge.** Post the PR link and stop. *Exception:* a large mechanical change + spanning many repos may be agent-opened and agent-merged by prior agreement with the author. 7. The human reviews and squash-merges. Do not push directly to `main`. Do not use `--no-verify` or otherwise skip hooks. Create new diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ebcef00..005de91 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,15 +31,25 @@ explicitly overrides it. 1. **Open an issue first.** Describe the problem or feature. This is the unit of work. 2. **Branch** off `main`, named `-` (e.g. `42-fix-date-parsing`). 3. **Do the work.** Keep commits coherent. Run tests + lint locally before pushing. -4. **Open a PR** whose description includes `Closes #` and explains the *why*. -5. **CI must be green** (lint + tests) before merge. -6. **Hand off for review.** When an **agent** opens the PR, it stops here and gives the author - a short summary — what changed, why, test/lint results, and the PR link — then waits. Agents - do **not** self-merge. +4. **Read the change before it becomes a PR.** A pull request is a claim that its author + stands behind the change, so nobody opens one carrying work the author hasn't actually + read. Run `git diff main...` and read it. This is where debug output, stray files, and + data or credentials that were never meant to be committed get caught. + - When an **agent** made the change, the author hasn't seen it yet — so the agent stops + *before* opening the PR. It pushes the branch, posts a short summary (what changed, why, + lint/test results), and waits. The author reviews locally, then opens the PR or asks the + agent to. + - An agent that is unsure about a decision raises it with the author **here**, not in the + PR description. A question written into a PR reads as though the author is asking their + reviewers, which sponsors a question they never asked. +5. **Open a PR** whose description includes `Closes #` and explains the *why*. +6. **CI must be green** (lint + tests) before merge. +7. **Hand off for merge.** Agents do **not** self-merge: after CI is green, post the PR link + and stop. - *Exception:* large mechanical changes spanning many repos (e.g. an org-wide rename like - dropping the `canswim` prefix) may be agent-merged to avoid dozens of round-trips, by - prior agreement with the author. -7. **Review & squash-merge.** The author reviews the PR and squash-merges it themselves, then + dropping the `canswim` prefix) may be agent-opened and agent-merged to avoid dozens of + round-trips, by prior agreement with the author. +8. **Review & squash-merge.** The author reviews the PR and squash-merges it themselves, then deletes the branch. The squash commit message should carry the meaningful detail, not just the PR title.