Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions skills/review-loop/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ One cycle, in order:

It polls `gh api .../pulls/<pr>/reviews` for a Copilot review newer than `$SINCE` and exits 0 (printing the review) when one lands, or 124 on timeout. The harness re-invokes the session when it exits. Do **not** write a foreground `sleep`/`until` loop — foreground sleep is blocked and it freezes the session.
4. **Triage every open review comment** — Copilot's new ones plus any human comments already on the PR. **REQUIRED SUB-SKILL:** `superpowers:receiving-code-review`. Verify each against the codebase. No performative agreement, no blind implementation. Copilot is confidently wrong often enough that "apply all suggestions" is the wrong default.
5. **Address.** Apply the comments that are correct, one at a time, testing each; reply in the comment thread stating what changed; resolve the thread (`resolveReviewThread`). For a comment that is wrong or a judgment call, **do not silently apply or silently ignore** — handle the pushback per the calling context (below).
6. **Re-request and loop.** Capture a fresh watermark *first*, then commit and push the fixes (commit convention from the project's CLAUDE.md). Now **wait before re-requesting** — launch the background wait (Step 3) against that watermark:
5. **Address, then always reply and resolve.** Work one comment at a time, and before moving to the next, close the loop on it in this order: (a) for a comment that is correct, apply the fix, test it, and commit it; for a comment that is wrong or a judgment call, first handle the pushback per the calling context (below) to decide the resolution; (b) post a threaded reply stating the resolution — what you changed (name the fix or commit) for ones you applied, or why the comment does not apply for ones you reject; (c) resolve the thread (`resolveReviewThread`). The reply is mandatory and is never gated behind a confirmation prompt (see GitHub-mutation discipline) — every comment gets one, including rejected ones. **Never resolve a thread without a reply, and never silently apply or silently ignore** — a resolved-but-unexplained thread destroys the reasoning trail.
6. **Re-request and loop.** The fixes were committed per comment in Step 5 (commit convention from the project's CLAUDE.md). Capture a fresh watermark *first*, then push them. Now **wait before re-requesting** — launch the background wait (Step 3) against that watermark:

- **A repo that runs Copilot review on every push will produce the new review from the push alone.** Waiting first lets that auto-review land and counts it (it is newer than the pre-push watermark). Re-requesting on top of it would double the review — so do not re-request until the wait has had a chance to catch a push-triggered review.
- **What a wait timeout means depends on whether the repo auto-reviews on push — and only one case authorizes a re-trigger.** On a repo that auto-reviews on push, the push already queued the review, so a timeout means it is *latent, not absent* (auto-review can lag the push by hours). Re-triggering then stacks a second review of the same commit — do **not** re-trigger; stop and report that the push-triggered review is still pending and will land on its own schedule, and the user can resume the loop when it appears. Re-trigger explicitly with **remove-then-re-add** (`gh pr edit <pr> --remove-reviewer "@copilot"` then `--add-reviewer "@copilot"` — a plain re-add often no-ops once Copilot has already reviewed) **only** when the repo does *not* auto-review on push, so the push genuinely will not produce a review by itself; then launch the wait once more.
Expand Down Expand Up @@ -118,7 +118,10 @@ The skill is told its context when invoked. This controls Step 5 pushback only.

## GitHub-mutation discipline

Requesting the reviewer, replying in threads, resolving threads, and pushing are GitHub mutations. Follow the project rule: no mutation without a fresh confirmation against the specific thing about to land, in the interactive contexts. In autonomous fan-out, follow the autonomous-mode discipline already established for sub-PRs (the user opted into the mechanical bundle up front).
Not every GitHub mutation in the loop is the same kind of action, and conflating them is what makes the reasoning trail disappear. Separate them:

- **Replying in a thread and resolving it are how the loop records a decision you are already making — mandatory, and never confirmation-gated.** The reply's body is dictated by the comment it answers (what you changed, or why the comment does not apply), so there is nothing separate to confirm. Do **not** pause to ask permission before leaving a reply, and do **not** skip or batch-defer it to avoid pestering. A thread addressed without a reply, or resolved silently, destroys the human-readable trace — which is the entire audit value of the loop. This holds in every context, interactive or autonomous.
- **Requesting (or re-requesting) the reviewer and pushing commits are outward, harder-to-reverse mutations.** These follow the project rule: in interactive contexts, a fresh confirmation against the specific thing about to land; in autonomous fan-out, the autonomous-mode discipline already established for sub-PRs (the user opted into the mechanical bundle up front).

## What this skill does not do

Expand All @@ -142,3 +145,5 @@ Requesting the reviewer, replying in threads, resolving threads, and pushing are
| "The wait timed out, so re-trigger the review" | On an auto-review-on-push repo a timeout means the review is *latent* (it can lag hours), not absent — re-triggering stacks a second review. Stop and report it is pending; only re-trigger when the repo does not auto-review on push. |
| "Copilot raised this again, so it's a new problem" | A re-review may repeat comments you already addressed or dismissed. Re-verify against the current diff before acting. |
| "I'll auto-dismiss the comment I disagree with" | Wrong/judgment-call comments get pushback (interactive) or a bubble-up concern (fan-out) — never a silent drop. |
| "Replying is a mutation, so I'll confirm first / batch it" | The reply records a decision you're already authorized to make; it's mandatory and ungated. Confirmation is for pushing and (re-)requesting review, not for the audit trail. |
| "I fixed and resolved it; the diff shows what changed" | A resolved thread with no reply destroys the trace — the diff shows what, not why. Reply, then resolve. |
Loading