diff --git a/skills/developing-a-feature/SKILL.md b/skills/developing-a-feature/SKILL.md index cf6820f..229ba75 100644 --- a/skills/developing-a-feature/SKILL.md +++ b/skills/developing-a-feature/SKILL.md @@ -46,6 +46,13 @@ For sequential single-PR work, skip to Step 4. For multi-PR work, dispatch paral Record the choice in the state file's frontmatter as `sub_pr_approval: autonomous` or `sub_pr_approval: manual`. The fan-out skill reads this field at every sub-PR ripening to decide whether to gate on user approval. Default if the field is missing in an older state file: `autonomous` (preserves the original behaviour). +**Sub-PR review-loop (multi-PR only).** Immediately after the approval-mode choice, ask a second `AskUserQuestion`: should each sub-PR run an automated review-loop and come back clean before it is self-merged? This is opt-in and independent of the approval mode. + +- **On** — at each sub-PR's ripening, the orchestrator runs `feature-dev-workflow:review-loop` against the open sub-PR before the existing `review`-skill pass and self-merge. A comment the loop wants to push back on does not pause the fan-out; it is logged as a bubble-up concern and surfaced at the wave checkpoint. +- **Off** (default) — ripening is unchanged. + +Record the choice as `sub_pr_review_loop: on` or `sub_pr_review_loop: off`. The fan-out skill reads it at every ripening. Default if the field is missing in an older state file: `off` (preserves the original behaviour). + ### 3. Set up the implementation environment - **Multi-PR (feature-branch model)** — `feature/` already exists, created and pushed by `feature-dev-workflow:planning-a-feature` and carrying the committed spec/plan/state. **Reuse it; never re-create it** off `origin/main` — that errors (`fatal: a branch named 'feature/' already exists`) and would orphan the planning artifacts. If planning already made the integration worktree at `.claude/worktrees/`, just `cd` into it. Otherwise attach one to the existing branch: @@ -102,6 +109,10 @@ Record the choice in the state file's frontmatter as `sub_pr_approval: autonomou Sub-PRs into the feature branch are owned by `feature-dev-workflow:fanning-out-with-worktrees`, not this step. +Once the PR to main is open — the single-PR PR, or the multi-PR integration PR — ask the user (via `AskUserQuestion`) whether to run an automated review-loop on it before handing off for external human review. If yes, **OPTIONAL SUB-SKILL:** `feature-dev-workflow:review-loop` against this PR; it drives the PR's automated (Copilot) review to clean. This is the interactive context, so a comment the loop wants to push back on pauses for the user. If no, hand off as-is. + +Run the loop on the **final** PR diff. The Step 7 teardown is the last commit on the branch, so a review run before it goes stale against what external reviewers actually see. If the teardown will land after a clean review, defer the loop until after the teardown commit (or re-run it afterward) — and when CI gates the teardown, that means running the loop once the teardown commit is pushed and green, not at PR-open. + ### 7. Tear down the planning artifacts Delete the plan + state file once the work is genuinely done. The spec stays — it's the durable ADR. The plan and state file are scratch; leaving them committed past readiness pollutes the repo with stale operational state that future `grep`s have to wade through. diff --git a/skills/fanning-out-with-worktrees/SKILL.md b/skills/fanning-out-with-worktrees/SKILL.md index fbae2b4..4b3f89a 100644 --- a/skills/fanning-out-with-worktrees/SKILL.md +++ b/skills/fanning-out-with-worktrees/SKILL.md @@ -93,16 +93,17 @@ When a sub-PR is ready (subagent reports `ready` and the relevant verification c Per sub-PR, in order: -1. **Two-stage review — spec-compliance first, then code quality.** Both stages are the `review` skill, run against the sub-PR number with full PR context, scoped differently. This review is weaker than external review (which lands at the integration PR) but stronger than nothing; it catches issues that would otherwise pile onto the integration PR reviewer. The order is a gate, not a preference: a PR that doesn't yet implement its sub-issue is going back regardless, so quality findings on code about to change are wasted effort and noise. One blended "looks good" pass is the failure this split exists to prevent. +1. **Automated review-loop, if the state file's `sub_pr_review_loop` is `on`.** **OPTIONAL SUB-SKILL:** `feature-dev-workflow:review-loop` against the sub-PR number, in fan-out mode — it requests the automated (Copilot) review, waits for it in the background, and drives the PR to review-clean. In fan-out mode a comment the loop wants to push back on does **not** pause the fan-out: log it as a bubble-up concern in the `## Bubble-up log` (surfaced at the wave checkpoint by `feature-dev-workflow:reviewing-feature-progress`) and continue. Skip this step when `sub_pr_review_loop` is `off` or missing. It runs before the orchestrator's own review so the diff reviewed below is the post-automated-review diff that will actually merge. +2. **Two-stage review — spec-compliance first, then code quality.** Both stages are the `review` skill, run against the sub-PR number with full PR context, scoped differently. This review is weaker than external review (which lands at the integration PR) but stronger than nothing; it catches issues that would otherwise pile onto the integration PR reviewer. The order is a gate, not a preference: a PR that doesn't yet implement its sub-issue is going back regardless, so quality findings on code about to change are wasted effort and noise. One blended "looks good" pass is the failure this split exists to prevent. 1. **Spec-compliance pass (the gate).** **REQUIRED SUB-SKILL:** the `review` skill, scoped to *intent*: walk the sub-issue's acceptance criteria one by one and map each to real code in the diff **and** a test that exercises it. Confirm nothing is missing and nothing extra was built — unrequested scope is a finding, not a bonus (it may collide with a sibling PR or belong elsewhere). This pass must come back clean before the quality pass starts. 2. **Code-quality pass.** **REQUIRED SUB-SKILL:** the `review` skill, scoped to *craft*: bugs the criteria didn't name, edge cases, error handling, test quality, simplification/reuse. Beyond bugs, check the diff against the `## Conventions` block — does its layout, naming, and vocabulary match the block and the sibling PRs already merged? A convention violation caught here is one the integration reviewer never sees. - **Fix-loop.** When either pass returns findings, the orchestrator does **not** fix them in place — it routes them back to the worktree subagent via `SendMessage` (the author has the context, and silent orchestrator fixes rob the parallel agents of shared understanding), then re-runs the *same* pass against the changed surface. Spec-compliance findings re-run the spec pass; only once it is clean does the quality pass run. Repeat until both passes are clean. The worktree subagent fixes; the orchestrator reviews — never the reverse. -2. **Approval gate, per the state file's `sub_pr_approval` mode.** Every gate covers the **bundle**: merge + sub-issue close + state-file update. The close is bodyless (no `--comment` flag) — GitHub automatically cross-references the sub-issue from the merge commit via the sub-PR's body keyword, so no custom comment is needed and there's no "specific body about to land" for the close mutation. - - **`autonomous`** (default) — proceed straight through the bundle in steps 3-5. The user opted into the mechanical bundle (review → merge → bodyless close → state update) in `feature-dev-workflow:developing-a-feature` Step 2. +3. **Approval gate, per the state file's `sub_pr_approval` mode.** Every gate covers the **bundle**: merge + sub-issue close + state-file update. The close is bodyless (no `--comment` flag) — GitHub automatically cross-references the sub-issue from the merge commit via the sub-PR's body keyword, so no custom comment is needed and there's no "specific body about to land" for the close mutation. + - **`autonomous`** (default) — proceed straight through the bundle in steps 4-6. The user opted into the mechanical bundle (review → merge → bodyless close → state update) in `feature-dev-workflow:developing-a-feature` Step 2. - **`manual`** — pause and ask the user for explicit approval before the bundle. The prompt MUST surface: a one-line summary of the review findings ("review clean" / " findings, none blocking" / specific concerns), the PR's title and diff size, and a note that closing sub-issue `#` follows the merge. Wait for an explicit yes. On push-back, route the concern back to the worktree subagent via `SendMessage` instead of merging. -3. **Merge into the feature branch.** First **push any local state-file commits to `origin/feature/`** — the merge happens on GitHub's side and lands on origin's current tip, so unpushed local commits make the post-merge `git -C pull --ff-only` fail with "Not possible to fast-forward" (local and origin have diverged; recover with `git rebase origin/feature/`). Keep local == origin at every merge boundary. Then `gh pr merge --merge` (or `--squash` / `--rebase` per project preference), and `git -C fetch origin && git merge --ff-only origin/feature/` to bring the merge back. -4. **Close the sub-issue.** `gh issue close `. Sub-PRs into a non-default branch don't trigger `Fixes`/`Closes` — manual close is the workaround. The body's `Towards #` keyword left the issue open precisely so the orchestrator can close it here; the cross-reference from the merge commit (which references `#`, which references `#`) is preserved automatically without a custom comment. -5. **Update the state file.** Flip the row's status to `self-merged`. If the sub-PR was the realization of a contract (e.g. a pre-merge stub PR), flip the contract row's status to `locked` and fill in the `Realized in` pointer. +4. **Merge into the feature branch.** First **push any local state-file commits to `origin/feature/`** — the merge happens on GitHub's side and lands on origin's current tip, so unpushed local commits make the post-merge `git -C pull --ff-only` fail with "Not possible to fast-forward" (local and origin have diverged; recover with `git rebase origin/feature/`). Keep local == origin at every merge boundary. Then `gh pr merge --merge` (or `--squash` / `--rebase` per project preference), and `git -C fetch origin && git merge --ff-only origin/feature/` to bring the merge back. +5. **Close the sub-issue.** `gh issue close `. Sub-PRs into a non-default branch don't trigger `Fixes`/`Closes` — manual close is the workaround. The body's `Towards #` keyword left the issue open precisely so the orchestrator can close it here; the cross-reference from the merge commit (which references `#`, which references `#`) is preserved automatically without a custom comment. +6. **Update the state file.** Flip the row's status to `self-merged`. If the sub-PR was the realization of a contract (e.g. a pre-merge stub PR), flip the contract row's status to `locked` and fill in the `Realized in` pointer. ### 6. Checkpoint review, then dispatch wave N+1 @@ -129,7 +130,7 @@ When every wave is complete (every sub-issue closed, every row `self-merged`, ev - **Letting the worktree subagent review its own PR.** A subagent reviewing the code it just wrote has the same blind spots in review that it had in implementation. The orchestrator owns sub-PR review precisely because it didn't write the code. - **Letting a bubble-up concern die in one subagent.** Propagation is the whole point of the watch loop. - **Forgetting to manually close the sub-issue.** The keyword doesn't fire on non-default-branch merges; the issue page silently shows "open" even though the work shipped. -- **Letting the state file go stale.** A resumed session reads it as ground truth. Update every row as reality moves; commit **and push** the state-file diff per phase transition — an unpushed local commit diverges the feature branch the moment the next sub-PR squash-merges on GitHub (see Step 5.3). +- **Letting the state file go stale.** A resumed session reads it as ground truth. Update every row as reality moves; commit **and push** the state-file diff per phase transition — an unpushed local commit diverges the feature branch the moment the next sub-PR squash-merges on GitHub (see Step 5.4). ## Red flags diff --git a/skills/review-loop/SKILL.md b/skills/review-loop/SKILL.md new file mode 100644 index 0000000..93cc137 --- /dev/null +++ b/skills/review-loop/SKILL.md @@ -0,0 +1,129 @@ +--- +name: review-loop +description: + Use when asked to run a review loop on a PR, drive a PR's automated + (Copilot) review to clean, or repeatedly request-and-address review + until nothing new comes back. +--- + +# review-loop + +## When to invoke + +When the user asks to loop a PR through automated review until it is clean — "run a review loop on #N", "get Copilot to review this and address everything", "keep requesting review until it's resolved". Also invoked from the two opt-in gates in `feature-dev-workflow:developing-a-feature` (sub-PRs, and the PR to main). + +Not for a single one-off review pass — that is the `review` skill (local diff review) or a single `gh pr edit --add-reviewer`. This skill is the *loop*: request, wait, triage, address, re-request, until clean. + +## The mechanism (verified — do not improvise the commands) + +Copilot review is requested **only** through `gh pr edit`. The REST `requested_reviewers` endpoint rejects Copilot (it accepts human logins and team slugs only), so `gh api .../requested_reviewers -f "reviewers[]=copilot-..."` returns 422. Use: + +``` +gh pr edit --add-reviewer "@copilot" +``` + +Requires `gh` ≥ 2.88.0 (`gh --version`). Not available on GitHub Enterprise Server. + +| Need | Command / identity | +| --- | --- | +| Request the review (first round) | `gh pr edit --add-reviewer "@copilot"` | +| Re-request after Copilot already reviewed | `gh pr edit --remove-reviewer "@copilot"` then `--add-reviewer "@copilot"`. Re-requesting is **unreliable** — it sometimes produces a fresh review (even on the same commit) and sometimes silently does nothing. Never assume it fired; confirm by watermark (next row). | +| Confirm a *new* review actually arrived | a Copilot review in `/pulls//reviews` with `submitted_at` newer than the watermark you captured before re-requesting. This is the only reliable signal — not the exit code of `gh pr edit`. | +| Confirm it attached (gh can exit 0 without attaching) | `gh pr view --json reviewRequests` → expect a reviewer with login `Copilot` | +| Copilot's author login in `/pulls//reviews` | `copilot-pull-request-reviewer[bot]` (this is the watermark filter) | +| List open review threads | `gh api graphql` on `PullRequest.reviewThreads` → `id`, `isResolved`, `viewerCanResolve` | +| Resolve a thread | `gh api graphql` mutation `resolveReviewThread(input:{threadId:""})` (GraphQL only; no REST) | +| Reply in a comment thread | `gh api repos/{owner}/{repo}/pulls/{pr}/comments/{id}/replies -f body=...` | + +## The loop + +```dot +digraph review_loop { + "Capture watermark, request Copilot" [shape=box]; + "Attached?" [shape=diamond]; + "Copilot unavailable:\ntriage existing human comments, exit" [shape=box]; + "Wait in BACKGROUND for new review" [shape=box]; + "Triage every open comment\n(receiving-code-review)" [shape=box]; + "Address: apply correct, push back on wrong" [shape=box]; + "New actionable comments\nOR unresolved threads?" [shape=diamond]; + "Cap reached?" [shape=diamond]; + "Done: clean" [shape=doublecircle]; + "Stop, report what remains" [shape=box]; + + "Capture watermark, request Copilot" -> "Attached?"; + "Attached?" -> "Copilot unavailable:\ntriage existing human comments, exit" [label="no"]; + "Attached?" -> "Wait in BACKGROUND for new review" [label="yes"]; + "Wait in BACKGROUND for new review" -> "Triage every open comment\n(receiving-code-review)"; + "Triage every open comment\n(receiving-code-review)" -> "Address: apply correct, push back on wrong"; + "Address: apply correct, push back on wrong" -> "New actionable comments\nOR unresolved threads?"; + "New actionable comments\nOR unresolved threads?" -> "Cap reached?" [label="yes"]; + "Cap reached?" -> "Capture watermark, request Copilot" [label="no"]; + "Cap reached?" -> "Stop, report what remains" [label="yes"]; + "New actionable comments\nOR unresolved threads?" -> "Done: clean" [label="no"]; +} +``` + +One cycle, in order: + +1. **Capture the watermark, then request.** `SINCE=$(date -u +%Y-%m-%dT%H:%M:%SZ)` *before* `gh pr edit --add-reviewer "@copilot"`. The watermark is what distinguishes the new review from a stale one left by an earlier round. +2. **Confirm Copilot attached.** `gh pr view --json reviewRequests`. If Copilot is **not** in the list, it is unavailable for this repo/plan (or `gh` is too old / this is GHES). Do **not** start waiting — there is no reviewer to produce a review. Instead do one triage pass over any human comments already on the PR (Step 3), state that Copilot review is unavailable, and exit. There is no loop without an automated reviewer to re-request. +3. **Wait in the background.** Launch the wait script with `run_in_background: true` so the session is not held hostage: + + ``` + ${CLAUDE_PLUGIN_ROOT}/skills/review-loop/templates/await-copilot-review.sh "$SINCE" + ``` + + It polls `gh api .../pulls//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: + + - **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. + - **Only if that wait times out with no new review**, re-trigger explicitly with **remove-then-re-add** (`gh pr edit --remove-reviewer "@copilot"` then `--add-reviewer "@copilot"`) — a plain re-add often no-ops once Copilot has already reviewed — and launch the wait once more. + + When a new review lands (from either path), return to Step 4. + + **If the second wait also times out, STOP — do not spin.** Re-requesting is inherently unreliable (GitHub exposes no dependable programmatic re-review; Copilot does not reliably auto-review on push either). Report that automated re-review could not be triggered and point the user at the robust path: enable a **repo ruleset that runs Copilot code review on every push**, which makes the loop reliable, or click the re-request (🔄) icon in the PR's Reviewers menu. Do not loop further against a reviewer that isn't responding. + +## Termination + +Stop and declare clean when **both**: + +- a fresh Copilot review (newer than the latest watermark) returns **no new actionable comments**, AND +- every review thread is **resolved or replied** (a verified-and-declined comment with a stated reason counts as resolved — do not thrash re-litigating it). + +**Safety cap: 3 request→address rounds.** If the loop has not converged by the cap, **stop** and report what remains unresolved. Do not raise the cap to keep going — non-convergence (Copilot surfacing churny or contradictory nits round after round) is a signal to hand back to the user, not to loop harder. Only count a review as "clean" if it arrived *after* your last push; a review from before the push is stale and must not end the loop. + +**Stop early if a re-request can't be triggered.** Separate from the convergence cap: if a round's re-request (Step 6) produces no new review before the wait times out, stop and report rather than retrying blindly. Re-review is not reliably automatable; the durable fix is the on-push Copilot ruleset. Also expect that when a re-review *does* fire, Copilot may repeat comments you already addressed or dismissed — re-verify against the current diff rather than assuming a repeated comment is a new problem. + +## Pushback handling depends on the calling context + +The skill is told its context when invoked. This controls Step 5 pushback only. + +- **Standalone, or the PR-to-main gate (interactive):** pause and surface the pushback to the user with technical reasoning, then act on their direction. +- **Autonomous multi-PR fan-out (sub-PR gate):** do **not** pause interactively — that breaks "autonomous fan-out has no per-sub-PR round-trips". Log the pushback as a bubble-up concern in the state file's `## Bubble-up log`, apply the rest, and let `feature-dev-workflow:reviewing-feature-progress` surface it at the wave checkpoint. + +## 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). + +## What this skill does not do + +- **Does not merge the PR.** It drives the PR to review-clean and hands back. +- **Does not invent comments or run its own static analysis.** Copilot and humans are the reviewers; this skill is the loop and the judgment layer over their output. +- **Does not block on human reviewers.** It sweeps in human comments already present, but only re-requests Copilot. + +## Red flags + +| Thought | Reality | +| --- | --- | +| "I'll `gh api .../requested_reviewers` to add Copilot" | That endpoint rejects Copilot (422). Only `gh pr edit --add-reviewer "@copilot"` works. | +| "I'll `sleep` in a loop until the review lands" | Foreground sleep is blocked and freezes the session. Use the background wait script; the harness wakes you. | +| "The command exited 0, so Copilot is reviewing" | `gh pr edit` can exit 0 without attaching. Confirm via `reviewRequests` before waiting. | +| "Copilot suggested it, so apply it" | Copilot is confidently wrong often. Triage via `receiving-code-review`; verify before applying. | +| "Still finding nits — one more round" | Past the 3-round cap, non-convergence is a signal to hand back, not to loop harder. | +| "A Copilot review exists, so we're clean" | Only a review *after your last push* counts. An earlier-round review is stale. | +| "I re-added Copilot, so a fresh review is coming" | Re-request is unreliable and a plain re-add often no-ops after Copilot already reviewed. Use remove-then-re-add, and confirm a new review by watermark — if none arrives before the wait times out, stop and recommend the on-push ruleset. | +| "I pushed the fixes, now re-request immediately" | If the repo auto-reviews on push, the push already triggers a review; an immediate re-request doubles it. Capture the watermark before pushing, wait for the push-triggered review first, and re-request only if it doesn't arrive. | +| "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. | diff --git a/skills/review-loop/templates/await-copilot-review.sh b/skills/review-loop/templates/await-copilot-review.sh new file mode 100755 index 0000000..da3cd92 --- /dev/null +++ b/skills/review-loop/templates/await-copilot-review.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# Poll an open PR until GitHub Copilot posts a review newer than a watermark, +# then print that review as JSON and exit. Designed to be launched in the +# background so the agent session is never blocked waiting. +# +# Dependencies: gh (with `gh api --jq`, which uses gh's built-in jq engine) and +# shell builtins only. No jq binary, no gh extension. +# +# Usage: +# await-copilot-review.sh [interval-seconds] [timeout-seconds] +# +# is the watermark: capture it BEFORE requesting the review, e.g. +# SINCE=$(date -u +%Y-%m-%dT%H:%M:%SZ) +# so a review left over from a previous round can't be mistaken for the new one. +# +# Exit status: +# 0 a Copilot review newer than the watermark was found (printed to stdout) +# 124 timed out before one appeared +# 2 bad arguments + +set -euo pipefail + +PR="${1:-}" +SINCE="${2:-}" +INTERVAL="${3:-30}" +TIMEOUT="${4:-900}" + +if [ -z "$PR" ] || [ -z "$SINCE" ]; then + echo "usage: await-copilot-review.sh [interval] [timeout]" >&2 + exit 2 +fi + +REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner) +BOT="copilot-pull-request-reviewer[bot]" +DEADLINE=$(( $(date +%s) + TIMEOUT )) + +# jq filter (run by gh's built-in engine): Copilot-authored reviews submitted +# after the watermark. The bot's author login in /reviews is +# copilot-pull-request-reviewer[bot]; the broad test() guards against the +# identity surfacing under a variant login. The watermark is interpolated as a +# jq string literal — safe because an ISO-8601 timestamp has no jq metacharacters. +# +# The filter emits each matching review as a bare object, NOT wrapped in an outer +# array. This is deliberate: gh runs the --jq filter once per page under +# --paginate and concatenates the outputs (and --slurp is rejected together with +# --jq). An array-wrapped filter would emit an empty array for every page with no +# match, so a multi-page no-match would print several empty arrays in a row — +# non-empty output, which the loop would misread as "review found". Emitting bare +# objects means a no-match prints nothing on every page, so the output is +# genuinely empty and the -n test below is correct. +# +# submitted_at > SINCE is a plain string comparison, which is correct here: the +# GitHub REST API always returns UTC ISO-8601 with a Z suffix (fixed-width, so +# lexical order == chronological order). Capture SINCE the same way +# (date -u +%Y-%m-%dT%H:%M:%SZ) and the comparison holds. +filter=".[] + | select((.user.login==\"$BOT\") or (.user.login|test(\"[Cc]opilot\"))) + | select(.submitted_at > \"$SINCE\") + | {id, state, submitted_at, body}" + +while :; do + # Capture output ONLY when gh succeeds. On an HTTP error (404, rate-limit, a + # network blip) gh prints the error body to stdout AND exits non-zero; the + # if-guard discards that body and leaves found empty, so an error is never + # mistaken for a review. This also tolerates transient failures under set -e: + # a failed poll just retries next interval, and only a genuine timeout exits + # 124 — keeping the exit code unambiguous for the caller (0 found / 124 timed). + found="" + if out=$(gh api "repos/$REPO/pulls/$PR/reviews" --paginate --jq "$filter" 2>/dev/null); then + found="$out" + fi + if [ -n "$found" ]; then + printf '%s\n' "$found" + exit 0 + fi + if [ "$(date +%s)" -ge "$DEADLINE" ]; then + echo "timed out after ${TIMEOUT}s waiting for a Copilot review on #$PR" >&2 + exit 124 + fi + sleep "$INTERVAL" +done diff --git a/templates/project-CLAUDE.md b/templates/project-CLAUDE.md index 1094bdd..0817de2 100644 --- a/templates/project-CLAUDE.md +++ b/templates/project-CLAUDE.md @@ -49,6 +49,7 @@ Invoke `feature-dev-workflow:planning-a-feature` at conception. It and the `**RE | Public-facing docs for what the feature changed (once structurally complete) | `feature-dev-workflow:writing-docs` | | Verify-before-done | `superpowers:verification-before-completion` | | Open / flip pull requests | `feature-dev-workflow:opening-a-pull-request` | +| Loop a PR through automated (Copilot) review until clean (opt-in) | `feature-dev-workflow:review-loop` | `superpowers:*` skills come from the [superpowers](https://github.com/obra/superpowers) plugin (a prerequisite, see below).