From 14c04d78b049dbc357256e2c7ef76d12b0cd8308 Mon Sep 17 00:00:00 2001 From: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Date: Tue, 10 Nov 2026 18:23:56 -0500 Subject: [PATCH] CLAUDE.md: describe the Opus review loop for advisory forks (#957) Copilot cannot review a temporary private advisory fork, so those PRs get an Opus reviewer treated the same way: review, fix, review the new head with a new reviewer, and repeat until a review of the final head has no open issues. Also records when to stop patching a fix and redesign it. Co-authored-by: Claude Fable 5.2 --- CLAUDE.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 49e97891..0714bd6a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,6 +17,20 @@ Any new feature or modification to an existing feature must follow this model sp The Fable review above is the author's side. Before a PR merges, a second, independent reviewer must also have read its final head, and every finding must be fixed or declined with a reason on its thread. - **Public PRs: request a Copilot review** when the PR is opened, and re-request it after every push that answers a finding (the `address-copilot-review` skill covers the loop). A review of an earlier commit does not count for a later head. -- **Temporary private advisory forks (`domainaware/parsedmarc-ghsa-*`): Copilot cannot review there.** The request is accepted and Copilot is listed as a reviewer, but the review fails with "Copilot encountered an error and was unable to review this pull request", on a retry too (tried twice on 2026-11-10). Do not open the diff as a public PR to get a review: that discloses the fix before the advisory is published. Instead run a second fresh-context review with a different model than the first one (Opus if the first was Fable), given only the diff, and make sure it covers commits added after the first review, such as review fixes. +- **Temporary private advisory forks (`domainaware/parsedmarc-ghsa-*`): Copilot cannot review there.** The request is accepted and Copilot is listed as a reviewer, but the review fails with "Copilot encountered an error and was unable to review this pull request", on a retry too (tried twice on 2026-11-10). Do not open the diff as a public PR to get a review: that discloses the fix before the advisory is published. Use the Opus review loop below instead. + +### The Opus review loop for advisory forks + +Treat an Opus reviewer exactly as Copilot is treated on a public PR: review, fix, review the new head, and repeat until a review of the **final head** has no open issues. Only then push and merge. A clean review of an earlier head does not count. + +1. **Review.** Start a fresh Opus subagent that has seen nothing but the code: read-only, given the diff that will merge (`git diff origin/master...HEAD`) and `AGENTS.md` as its rubric. Tell it which commits no reviewer has read yet, and to reproduce every suspected bug with a script before reporting it. Ask for findings ranked "must fix", "should fix" and "nit", design preferences under a separate "optional" heading, and a last line that is exactly `VERDICT: no open issues` or `VERDICT: changes recommended`. +2. **Triage** each finding like an external review: fix what is real, decline the rest with a reason. Tell the maintainer what the round found. +3. **Fix** through an implementer subagent, one commit per round, with tests that fail on the old code, then CI's commands and patch coverage. +4. **Review again with a new reviewer**, never the one from the previous round. Point it at the new commit first and tell it to try to break that commit: a fix made during review is the least-reviewed code on the branch. +5. **Stop when** a round on the final head ends `VERDICT: no open issues`. + +Work in a separate worktree when master has to be merged in, so a running reviewer's tree does not change under it. + +**When a fix has been defeated twice, change the approach.** If the second review of a fix finds another input of the same kind getting past it, stop adding cases. A check that recognises bad inputs one shape at a time will keep losing to the next shape; redesign so the code no longer depends on recognising them (for a credential leak, log and raise nothing derived from the secret-bearing value at all). In the 2026-11-10 round, three reviews in a row each defeated a shape-recognising check before that redesign held. @AGENTS.md