diff --git a/.claude/skills/open-pr/SKILL.md b/.claude/skills/open-pr/SKILL.md index 0e8069f6098..1531ad4a6cb 100644 --- a/.claude/skills/open-pr/SKILL.md +++ b/.claude/skills/open-pr/SKILL.md @@ -189,13 +189,16 @@ re-reviewed. Two consequences for this loop. A FIX push starts no new Copilot run, so a `copilot-*` check on the new head SHA is legitimately ABSENT; waiting for one that will never come is the failure mode to avoid — see §3e for what to require instead. And a -fresh review is requested only after a SUBSTANTIVE rework (new -behaviour, a reworked mechanism), never after every push: each -request re-reads the whole diff and surfaces "previously missed" -findings in files the push never touched, which draws another push -(kurrentschrift#406 collected ~15 requests in a day over a one-line -docstring fix). Stop once a round yields no new inline comments but -only carried-over items. +further review is requested only while the previous round found +something high or critical — a real bug, a security or correctness +issue, a wrong mechanism — whose fix you pushed; a round with only +low and medium findings gets its fixes pushed and its threads +resolved, and that is the end (owner, 2026-10-23: each round costs +premium requests, and a docs PR had collected three over wording). +Each request re-reads the whole diff and +surfaces "previously missed" findings in files the push never +touched, which draws another push (kurrentschrift#406 collected ~15 +requests in a day over a one-line docstring fix). Fetch all three comment surfaces — they carry different content: @@ -286,7 +289,8 @@ stale wait loop with the bracket trick (`pkill -f "x[.]y"`), or `.claude/skills/open-pr/scripts/` cover the loops, run from the repository root: `watch-checks.sh ` (§3a), `watch-copilot.sh [count]` (§3b–c: waits for `count` Copilot -reviews, 2 for the extra round after fixes, then prints the latest +reviews, 2 only when a high finding was fixed and another round +was requested, then prints the latest summary, every inline comment and the unresolved thread ids) and `watch-deploys.sh ` (§4). @@ -392,11 +396,12 @@ worktree stay; deleting those still needs the owner's OK. thread while it stays unresolved; outdated threads still count against review-clean — resolve them explicitly. - **A fix push no longer starts a review round.** `review_on_push` is - `false` since 2026-09-03 (§3b), so only an explicit — and - substantive — re-request opens another one. When a round does run, - new threads on the changed lines are the loop working, not noise — - but don't chase cosmetic nits past a couple of rounds; surface - stalemates to the user. + `false` since 2026-09-03 (§3b), so only an explicit re-request opens + another one, and that only while the previous round had high or + critical findings whose fixes you pushed (§3b). A round with only + low and medium findings ends the loop: push the fixes, resolve the + threads, never chase nits into another round; surface a stalemate + to the user instead. - **Stacked PRs die when their base squash-merges.** Don't stack; if work depends on an unmerged PR, wait for its merge (or do the work and rebase before opening). diff --git a/.claude/skills/open-pr/scripts/watch-copilot.sh b/.claude/skills/open-pr/scripts/watch-copilot.sh index d23b881b67f..dd7e99c98d5 100755 --- a/.claude/skills/open-pr/scripts/watch-copilot.sh +++ b/.claude/skills/open-pr/scripts/watch-copilot.sh @@ -7,7 +7,7 @@ # than a PR collects. # # Usage: watch-copilot.sh [count, default 1] -# count 2 waits for the one extra round requested after fixing the first round's findings. +# count 2 waits for a further round, requested only after a high or critical finding was fixed. set -uo pipefail PR=${1:?usage: watch-copilot.sh [count]} WANT=${2:-1} diff --git a/CLAUDE.md b/CLAUDE.md index 37845459458..7de9880c717 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,7 +42,7 @@ After opening a PR, the work is **not** complete. Stay with the PR until both th 1. **Watch the pipeline.** Poll `gh pr checks ` (and Cloud Build for triggered deploys) until every required check has finished. Use a background bash poll so other work can continue. Default: poll every 20 s, up to ~10 min per check. 2. **Fix CI failures.** If any check fails, read the relevant log (`gh run view --log-failed`, `gcloud builds log `), push a fix commit to the same branch, then keep watching. Repeat until green. -3. **Wait for the Copilot PR Reviewer bot — ONE review per PR, not one per push.** Review-on-push is off in the "Automated Copilot Code Review" ruleset (owner, 2026-09-03): the bot runs once when the PR opens or leaves draft, and a push triggers nothing. Don't re-request a review after every push. Each request is a full re-read of the whole diff, and the bot then surfaces "previously missed" findings in files the push never touched — which draws another push, which draws another request (the sibling repo's [kurrentschrift#406](https://github.com/MarkusNeusinger/kurrentschrift/pull/406) collected ~15 requests in a day over a one-line docstring fix). Request a fresh review explicitly only after a SUBSTANTIVE rework (new behaviour, a reworked mechanism), and stop once a round yields no new inline comments but only carried-over items: the field is grazed. A PR that is green with no open threads needs no further round — say so and let the owner merge. The first review typically lands within ~2 min of PR open. Fetch with `gh pr view --comments`, plus the three GitHub APIs that surface different comment types — `gh api` resolves `{owner}/{repo}` from the current git remote so these are copy/paste-portable: +3. **Wait for the Copilot PR Reviewer bot — ONE review per PR, not one per push.** Review-on-push is off in the "Automated Copilot Code Review" ruleset (owner, 2026-09-03): the bot runs once when the PR opens or leaves draft, and a push triggers nothing. Don't re-request a review after every push. Each request is a full re-read of the whole diff, and the bot then surfaces "previously missed" findings in files the push never touched — which draws another push, which draws another request (the sibling repo's [kurrentschrift#406](https://github.com/MarkusNeusinger/kurrentschrift/pull/406) collected ~15 requests in a day over a one-line docstring fix). Request another review only while the previous round found something high or critical — a real bug, a security or correctness issue, a wrong mechanism — whose fix you pushed; a round with only low and medium findings gets its fixes pushed and its threads resolved, and that is the end (owner, 2026-10-23: every round costs premium requests; a docs PR had collected three over wording). A PR that is green with no open threads needs no further round — say so and let the owner merge. The first review typically lands within ~2 min of PR open. Fetch with `gh pr view --comments`, plus the three GitHub APIs that surface different comment types — `gh api` resolves `{owner}/{repo}` from the current git remote so these are copy/paste-portable: - `gh api repos/{owner}/{repo}/pulls//reviews` — top-level review summaries (Copilot's overall comment lives here) - `gh api repos/{owner}/{repo}/pulls//comments` — inline review comments tied to file/line - `gh api repos/{owner}/{repo}/issues//comments` — generic PR conversation comments (codecov, deployment bots, humans) diff --git a/agentic/commands/pull_request.md b/agentic/commands/pull_request.md index 85145b4a2a5..a49d4b99b38 100644 --- a/agentic/commands/pull_request.md +++ b/agentic/commands/pull_request.md @@ -65,7 +65,8 @@ EOF 8. **The Copilot review runs ONCE**, when the PR opens (or leaves draft) — review-on-push is off in the "Automated Copilot Code Review" ruleset (owner, 2026-09-03), so step 7's follow-up commit and every later push trigger nothing. Do not re-request a review per push; the rule and its one - exception (a substantive rework) are in `CLAUDE.md` § "PR Follow-Through", step 3. + exception (another round only while the previous one had high or critical findings) are in + `CLAUDE.md` § "PR Follow-Through", step 3. ## Report diff --git a/changelog.d/copilot-one-round.md b/changelog.d/copilot-one-round.md new file mode 100644 index 00000000000..0eebe54ef78 --- /dev/null +++ b/changelog.d/copilot-one-round.md @@ -0,0 +1,9 @@ +### Changed + +- **Copilot reviews a PR once; another round only while the previous one + had high or critical findings.** `CLAUDE.md`, the `open-pr` skill and + `/pull_request` replace "stop once a round yields nothing new", which let + a round with any findings draw the next one, with the owner's rule of + 2026-10-23: a round with only low and medium findings gets its fixes + pushed and its threads resolved, and that is the end, because every round + re-reads the whole diff and costs premium requests. (#12250)